Skip to content
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

Avoid random access values for operands in not-yet-specified instructions. #2259

Merged
merged 1 commit into from
Mar 21, 2024

Conversation

mur47x111
Copy link
Contributor

See description in #2258

@github-actions github-actions bot added the X86 Arch label Jan 23, 2024
@kabeor
Copy link
Member

kabeor commented Jan 24, 2024

Thank you, can you plz also add a minor PoC in suite/cstest/issues.cs?

@@ -16915,7 +16915,7 @@

{ /* X86_VCMPSSZrr_Int, X86_INS_VCMP: vcmp */
0,
{ 0 }
{ CS_AC_WRITE, CS_AC_READ, CS_AC_READ, 0 }
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added the regression test along with this change, which only fixes the rr variant of vcmpunordss. Let me know if these access values can also be generated by tablegen.

Copy link
Collaborator

@Rot127 Rot127 Jan 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the access details can be generated. But x86 is not "officially" supported by auto-sync currently.
But the access information should theoretically generate fine.

You need to build our llvm-tblgen (see docs) and run

llvm-tblgen --gen-asm-matcher --printerLang=CCS -I ./llvm/lib/Target/X86/ -I ./llvm/include/ ./llvm/lib/Target/X86/X86.td

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you still want to generate these things, please do it in another PR. So it is nicely distinct from the changes here.

Copy link
Collaborator

@Rot127 Rot127 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please rebase ontop of next.

arch/X86/X86IntelInstPrinter.c Outdated Show resolved Hide resolved
arch/X86/X86ATTInstPrinter.c Outdated Show resolved Hide resolved
@Rot127 Rot127 added this to the v5.0.2 milestone Mar 20, 2024
@mur47x111
Copy link
Contributor Author

Please rebase ontop of next.

done

@kabeor
Copy link
Member

kabeor commented Mar 21, 2024

Thank you! Merged.

@kabeor kabeor merged commit 5d9942d into capstone-engine:next Mar 21, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants