Re: [PATCH] tools/tests: Add EIP check to test_x86_emulator.c

[Razvan Cojocaru <rcojocaru@bitdefender.com>]

On 08/07/2014 11:23 AM, Jürgen Groß wrote:
> On 08/07/2014 10:16 AM, Razvan Cojocaru wrote:
>> The test now also checks that EIP was modified after emulating
>> instructions after (and including) the "movq %mm3,(%ecx)..."
>> code block.
>>
>> Signed-off-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
>> ---
>>   tools/tests/x86_emulator/test_x86_emulator.c |   30
>> +++++++++++++++++---------
>>   1 file changed, 20 insertions(+), 10 deletions(-)
>>
>> diff --git a/tools/tests/x86_emulator/test_x86_emulator.c
>> b/tools/tests/x86_emulator/test_x86_emulator.c
>> index 0a00d5a..dd4e986 100644
>> --- a/tools/tests/x86_emulator/test_x86_emulator.c
>> +++ b/tools/tests/x86_emulator/test_x86_emulator.c
>> @@ -615,7 +615,8 @@ int main(int argc, char **argv)
>>           regs.eip    = (unsigned long)&instr[0];
>>           regs.ecx    = (unsigned long)res;
>>           rc = x86_emulate(&ctxt, &emulops);
>> -        if ( (rc != X86EMUL_OKAY) || memcmp(res, res + 8, 32) )
>> +        if ( (rc != X86EMUL_OKAY) || memcmp(res, res + 8, 32) ||
>> +             (regs.eip == (unsigned long)&instr[0]) )
> 
> Is really any modification okay? I think the test should check for
> the correct EIP, not just for one of 2^64 - 1 incorrect values.

While you are, of course, perfectly right that not any modification is
okay, this does at least catch the bigger issue of EIP not being updated
at all. I'll look into it.


Thanks,
Razvan Cojocaru