Skip to content

[MC] Disassembler does not support x86 instruction prefixes #8081

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
llvmbot opened this issue Jul 25, 2010 · 10 comments
Closed

[MC] Disassembler does not support x86 instruction prefixes #8081

llvmbot opened this issue Jul 25, 2010 · 10 comments
Labels
backend:X86 bugzilla Issues migrated from bugzilla

Comments

@llvmbot
Copy link
Member

llvmbot commented Jul 25, 2010

Bugzilla Link 7709
Resolution FIXED
Resolved on Oct 20, 2017 04:18
Version trunk
OS All
Blocks llvm/llvm-bugzilla-archive#10988
Reporter LLVM Bugzilla Contributor
CC @lattner,@topperc,@RKSimon,@rotateright

Extended Description

Following instructions from http://blog.llvm.org/2010/01/x86-disassembler.html:
echo '0xF3 0xAD' | llvm-mc -disassemble -triple=x86_64-apple-darwin9
outputs:
lodsl
instead of the correct sequence
rep lodsl

@lattner
Copy link
Collaborator

lattner commented Sep 8, 2010

This is still broken.

@llvmbot
Copy link
Member Author

llvmbot commented Jan 12, 2011

disclaimer: my knowledge of llvm internals is almost non-existent.

The issue here might be this:

REP/REPNE/etc are treated as instruction and instruction modifiers (ATTR_XS).

So X86DisassemblerDecoder.c:readPrefixes consumes the prefix byte, and later the prefix is converted into an ATTR flag.

Commenting
case 0xf0: /* LOCK /
case 0xf2: /
REPNE/REPNZ /
case 0xf3: /
REP or REPE/REPZ */
will cause the disassembler not to consume these prefix bytes, and output proper sequence for this test-case:
rep
lodsl
but this will break decoding of all instructions that were using ATTR_XS. (PAUSE VMXON ..)

I don't see any straightforward solution to this conundrum. It seems that, to properly fix this, the readPrefixes would have to check following bytes and if they match one of those instructions that use prefix (ex. XS) attribute, it should consume the prefix. In all other cases it should unconsumeByte.

This solution is very yucky, since it would require X86DisassemblerDecoder.c to know things that only *.td files should know :(

@llvmbot
Copy link
Member Author

llvmbot commented Oct 3, 2011

More test cases for rep prefix from the Intel ISA ref, p 4-316:
0xf3 0x6c #rep ins
0xf3 0x6d #rep ins
0xf3 0xa4 #rep movs
0xf3 0xa5 #rep movs
0xf3 0x6e #rep outs
0xf3 0x6f #rep outs
0xf3 0xac #rep lods
0xf3 0xad #rep lods
0xf3 0xaa #rep stos
0xf3 0xab #rep stos
0xf3 0xa6 #rep cmps
0xf3 0xa7 #repe cmps
0xf3 0xae #repe scas
0xf3 0xaf #repe scas
0xf2 0xa6 #repne cmps
0xf2 0xa7 #repne cmps
0xf2 0xae #repne scas
0xf2 0xaf #repne scas

With r140997:
ins
ins
movsb
movsd
outsb
outsd
lodsb
lodsd
stosb
stosd
cmpsb
cmpsd
scasb
scasd
cmpsb
cmpsd
scasb
scasd

@llvmbot
Copy link
Member Author

llvmbot commented Mar 9, 2012

Kevin Enderby has provided a partial fix for "lock":
http://llvm.org/viewvc/llvm-project?view=rev&revision=152414

Although it looks like "lock" is printed before the instruction rather than as a prefix.

@llvmbot
Copy link
Member Author

llvmbot commented Mar 9, 2012

Yep the problem is that in the translation from the internal form to an MCINST there is no way to represent as part of the MCINST. So my partial fix was to pick off the lock prefix if it is the first prefix and let it get created as its own MCINST which will get it to print.

@llvmbot
Copy link
Member Author

llvmbot commented Jun 24, 2012

Also the segment override prefix gets ignored
0x64 0xa1 0x18 0x00 0x00 0x00
Should be "mov eax, dword ptr fs:[18h]" and not "mov eax, 18h"
Is there any work in progress on this bug? It makes the disassembler completely unusable.

@llvmbot
Copy link
Member Author

llvmbot commented Aug 30, 2017

All these issues were fixed before Revision: 312105.

@rotateright
Copy link
Contributor

Correct me if I'm wrong, but:
https://reviews.llvm.org/rL311882
was reverted at:
https://reviews.llvm.org/rL311987

So we need to reopen this bug.

@llvmbot
Copy link
Member Author

llvmbot commented Sep 21, 2017

You're right: it will be fixed when D37262 is committed.

@llvmbot
Copy link
Member Author

llvmbot commented Oct 20, 2017

Fixed with rL315899.

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 3, 2021
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:X86 bugzilla Issues migrated from bugzilla
Projects
None yet
Development

No branches or pull requests

3 participants