Update the table for UD0 and UD1 with the latest llvm table #1863
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I usually don't like piecemeal solutions, but this one is small and the more complete fix of automating the x86 synchronization is unlikely to make it into the 5.0 release. If this does get merged, it would be great to make an additional issue here to remind us that we should explicitly test for this when working on the new sync workflow so that we don't lose this kind of information. |
|
Thanks, merged! And yes, we really need to design a better workflow. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The change in this PR is to address the problem regarding UD0 and UD1: i.e., the capstone decoder was not able to process the arguments of these instructions correctly.
According to the X86 instruction-set manual, UD0 and UD1 (Undefined Instruction) take two arguments:
UD0 r32, r/m32
UD1 r32, r/m32
(https://www.felixcloutier.com/x86/ud)
It was observed that capstone does not read the bytes after the opcode correctly, and does not produce the two arguments.
At first, I tried to update all the tables in suite/synctools/tablegen/X86/ with the latest llvm tables, but I was getting various problems.
One of them was that any instructions with zmm regsiters as operands are decoded incorrectly.
After several hours of trial, I ended up giving up updating the whole tables, and instead, I tried updating the specific table in suite/synctools/tablegen/X86/X86InstrSystem.td, which is relevant to the particular problem that I was having with UD0 and UD1, with the llvm table https://github.com/capstone-engine/llvm-capstone/blob/dev/llvm/lib/Target/X86/X86InstrSystem.td#L25.
And I confirmed that this change resolves the observed problem.
Note that this MR does not include the changes in the generated files in arch/X86 because I was having some problem in running the scripts, and had to do some manual editing to make things work.
(Please see GrammaTech#7 if you would like to see the changes in the generated files)
This PR is mainly to bring up the need of updating the tables with the latest llvm tables for X86 as well.
However, if the owner is willing to accept this PR with a partial update in the table, I will try to push the changes in the generated files as well.
As a side note, I am aware of this PR (#1803) as an effort for automatic synchronization, but the PR does not seem to cover X86.
I also noticed that there may be an attempt at synchronizing X86 table: capstone-engine/llvm-capstone#2 (@hainest)
@aeflores @adamjseitz