gdabah / distorm

Powerful Disassembler Library For x86/AMD64
Other
1.26k stars 238 forks source link

Byte 0x63 is incorrectly dissassembled as ARPL instead of invalid in 16-bit mode #23

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Decode byte 0x63 with Decode16Bits.

What is the expected output? What do you see instead?
You should see DB 0x63.
You will see ARPL <mem>, <reg>.

What version of the product are you using? On what operating system?
Not sure. Win32.

Please provide any additional information below.
Around line 300 of instructions.c there is a check for ARPL vs. MOVSXD. The 
check is for 32-bit and 64-bit, but not for 16-bit.

The line:

return ci->dt == Decode64Bits ? (_InstInfo*)&II_movsxd : &II_arpl;

should be changed to:

return ci->dt == Decode64Bits ? (_InstInfo*)&II_movsxd : ci->dt == Decode32Bits 
? &II_arpl : NULL;

which fixes the problem.

Original issue reported on code.google.com by matthew....@gmail.com on 29 Apr 2011 at 6:26

GoogleCodeExporter commented 9 years ago
Found in r165, although I'm 99% sure it hasn't been fixed since.

Original comment by matthew....@gmail.com on 29 Apr 2011 at 6:29

GoogleCodeExporter commented 9 years ago
As you can see all 16 bits instructions are enabled for both 32 and 64 bits. 
There's such an if statement to do that 
http://code.google.com/p/distorm/source/browse/trunk/src/decoder.c#141

I do this on purpose. And same for the other issue you opened. The problem is 
that the DB in disOps, you're welcome to take a look at x86sets.py, doesn't 
have the info for each instruction to indicate the processor.
However, there's the instruction set class, which might give some extra info 
about the instruction and its class type, but maybe that's not too informative 
in your case.
And honestly, I'm not sure how many people out there use diStorm for 16 bits 
mode.

Original comment by distorm@gmail.com on 30 Apr 2011 at 9:43

GoogleCodeExporter commented 9 years ago
So, before I go any further with this...

1. What is the purpose of allowing the disassembler to disassemble 32-bit 
instructions in 16-bit mode?

2. Is there a good reason why the opcode information would be better suited to 
being categorized by 16/32/64-bit as opposed to categorized by processor?

Original comment by matthew....@gmail.com on 30 Apr 2011 at 6:16

GoogleCodeExporter commented 9 years ago
1. in cases where 16 bit uses 32 bit instruction, some real mode quirks 
probably.
2. I never said it was better. It was a kind of standard in disassemblers. And 
to be honest, making it categorized by processor will take a lot of work, and 
nobody cares about it much, AFAIK.

Original comment by distorm@gmail.com on 30 Apr 2011 at 9:06

GoogleCodeExporter commented 9 years ago
It shouldn't take a lot of work. You already have 3 processors: 80186, 80486, 
and 80686 (or something similar).  Just rename the DecodeType to Processor, and 
rename Decode16Bits to 80186, etc.  Viola.  You now have 3 processors, and can 
later add more.  Otherwise, one would never be able to extend distorm to 
anything else.  For what it's worth, IDA uses processor types.

In any case, if it's not going to be changed, then I guess that's the end of 
this.  I was testing distorm out for a project of mine, but if it can't 
disassemble 8086 then oh well.

Original comment by matthew....@gmail.com on 30 Apr 2011 at 11:47

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago

Original comment by distorm@gmail.com on 31 May 2011 at 10:01

GoogleCodeExporter commented 9 years ago
According to docs it should be ARPL in 16 bits mode. There's no problem, unless 
I learn otherwise.

Original comment by distorm@gmail.com on 7 Jun 2011 at 9:32