Skip to content

synctools: generalize asmwriter.py's AArch64-only alias/mnemonic logic - #3053

Closed
StilesCrisis wants to merge 1 commit into
capstone-engine:v5from
StilesCrisis:fix-asmwriter-arch-generalization
Closed

synctools: generalize asmwriter.py's AArch64-only alias/mnemonic logic#3053
StilesCrisis wants to merge 1 commit into
capstone-engine:v5from
StilesCrisis:fix-asmwriter-arch-generalization

Conversation

@StilesCrisis

@StilesCrisis StilesCrisis commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Problem

suite/synctools/asmwriter.py (the script that converts raw llvm-tblgen -gen-asm-writer output into capstone's C style) has AArch64-only logic hardcoded into several places, dating to when the script was written for AArch64 in 2022 and never exercised against another target since. Running it against a non-AArch64 target's raw tblgen output produced by a reasonably modern LLVM breaks in several ways - found while regenerating PowerPC's asm-writer output for #3052.

Bugs fixed

  • getRegisterName/getMnemonic's "const char *AArch64InstPrinter::" skip rule was a literal string match, so for any other target it also matched (and silently swallowed) that target's own real getRegisterName definition line - e.g. PPC's "const char *PPCInstPrinter::getRegisterName(...)" matches the same substring pattern. Result: a getRegisterName body with no function signature at all. Now excludes getRegisterName explicitly, and matches the target's own class name instead of a hardcoded AArch64InstPrinter.
  • getMnemonic had no translation for newer LLVM's early-out guard (if (Bits == 0) return {nullptr, Bits};), which doesn't exist in the older LLVM output this script was originally written against.
  • Newer LLVM wraps register handles in MCRegister, so getRegisterName's raw definition now includes a RegNo = Reg.id(); redeclaration line that conflicts with this script's hardcoded plain-unsigned-parameter signature.
  • The injected printAliasInstr alias-matching block hardcoded AArch64InstPrinterValidateMCOperand and AArch64's SME tile-slice-indexing bracket syntax (za[w8, 0]) unconditionally. Validator name is now parameterized by target; SME bracket handling now only applies for AArch64/ARM64, with a plain set_mem_access()-only fallback for every other target (none of which have SME syntax).

Each fix was confirmed necessary by an actual compiler/parse error while iterating toward a from-scratch PPC rebuild, not speculative.

Scope note

This is a synctools script fix, not a PPC-specific change - it generalizes logic that only worked for AArch64 to work for any target regenerated against a comparably modern LLVM. It's independent of #3052, which uses an older (LLVM 7.0.1-vintage) toolchain that doesn't hit any of these code paths at all.

One related note for anyone regenerating a target against modern LLVM specifically: if that target has zero K_Custom AliasPatternConds (true for PPC, as of writing), the injected <Target>InstPrinterValidateMCOperand call in the alias-matching block is unreachable but still needs a stub of that name in the target's InstPrinter.c to link - this script doesn't generate that stub itself, since it's C source, not template output.

@StilesCrisis
synctools: generalize asmwriter.py's AArch64-only alias/mnemonic logic
d88585d 
While attempting to regenerate PPC's AsmWriter output end-to-end (see
previous commit), hit several real bugs in this script - all in logic
that was hand-written specifically for AArch64 in 2022 and never
exercised against any other target since:

- getMnemonic/getRegisterName's "const char *AArch64InstPrinter::"
  skip rule was hardcoded to the literal string "AArch64InstPrinter::",
  so it silently swallowed PPC's real getRegisterName definition line
  too (both match "const char *PPCInstPrinter::getRegisterName(...)"),
  producing a getRegisterName body with no function signature at all.
  Now excludes getRegisterName explicitly.

- getMnemonic's std::pair-returning raw signature had no matching
  early-out translation for the "if (Bits == 0) return {nullptr,
  Bits};" guard that newer LLVM adds - fixed to fold into the same
  uint64_t-returning convention already used for the main return path.

- Newer LLVM wraps register handles in MCRegister, so getRegisterName's
  raw definition now reads "getRegisterName(MCRegister Reg) { unsigned
  RegNo = Reg.id(); ... }" - this leftover redeclaration line conflicted
  with our hardcoded plain-unsigned-parameter signature. Now stripped.

- The injected alias-matching block's AArch64InstPrinterValidateMCOperand
  call and its isSME/set_sme_index handling (AArch64 SME tile-slice
  indexing syntax, e.g. "za[w8, 0]") were both hardcoded literals.
  Parameterized the validator name by target; SME handling now only
  applies for AArch64/ARM64, with a plain always-set_mem_access fallback
  for every other target (which never has SME bracket syntax).

Each fix was confirmed necessary by direct compiler error (not
speculative) while iterating toward a from-scratch PPC rebuild. See the
previous commit for what remains unverified about that full rebuild.
@Rot127

Rot127 commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

These scripts are no longer in use.
We use AutoSync now as updater on the next branch: https://github.com/capstone-engine/capstone/blob/next/suite/auto-sync/README.md

We still backport important fixes to v5 or do very small additions, but it is no longer under active development.

I suggest to switch to the latest Alpha release.
Even though it is called Alpha. It is way better tested and more correct than the v5 branch.

@Rot127 Rot127 closed this Sep 2, 2026
@StilesCrisis

StilesCrisis commented Sep 2, 2026 via email

Copy link
Copy Markdown
Contributor Author

@StilesCrisis

StilesCrisis commented Sep 2, 2026 via email

Copy link
Copy Markdown
Contributor Author

@Rot127

Rot127 commented Sep 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Also to clarify: the PR is large solely because it was made properly
through llvm-tblgen.

I see. Thanks for pointing that out. Still, the v5 branch has not the same amount of testing. So I don't want to merge a ~5000 loc change. Since we worked on the AutoSync update we really try to improve the stability and reliability of Capstone, and that would go against it. Also because these old generating scripts are not tested.

If v6 is still labeled as alpha, I don't think decomp-toolkit would accept a PR switching to it, even if it's better in this instance.

I can recommend to open an issue about it and ask. Many other big projects switched already.
Including QEMU and linux perf tools.

PPC in particular was so much improved compared to the v5 branch, I think for most people it is worth the effort.

@StilesCrisis

StilesCrisis commented Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

Many other big projects switched already.
Including QEMU and linux perf tools.

That's valuable to know. I will see if it's easy to switch dtk over to v6, and if so, I will push the PR and use this as the rationale. If it's complex for some reason, or it causes regressions, I will be back here.

@StilesCrisis

StilesCrisis commented Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

Just noticed we're chatting on the "wrong" PR (this was fixes to the v5 py setup script, not the meaty PR with all the table changes) but NBD

@Rot127

Rot127 commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

If it's complex for some reason, or it causes regressions, I will be back here.

Don't forget to check out the release guide: https://github.com/capstone-engine/capstone/blob/next/docs/cs_v6_release_guide.md

I tried my best to document any breaking change and new additions there.

@StilesCrisis

StilesCrisis commented Sep 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Thank you very much for the guidance. capstone v6 is pretty much a win on every axis and resolves my issue. I had Claude exercise the before-and-after on my project:

  • The fcmpo/fcmpu bug (capstone#3052) is genuinely fixed — v5 failed to decode all 2,650 occurrences in retail's .text; v6 decodes every one correctly, verified register-for-register against our old manual workaround formula.
  • Had to add CS_MODE_BOOKE alongside CS_MODE_PS — plain v6 regressed rfi decoding (used in our OS exception handlers) without it; with it, rfi decodes correctly again.
  • Full-segment sweep: 325,552 instructions, 99.99939% decoded — the only 2 failures are a deprecated mcrxr encoding that v5 also failed on (not a v6 regression).
  • One cosmetic-only difference: v6 prints branch mnemonics in LLVM-style aliases (bt eq,/bf eq,) instead of v5's GNU-as style (beq/bne) — same instructions/operands, just different assembler alias naming, and no capstone option was found to restore the old style.

I asked for more info on mcrxr since that op was unfamiliar to me. It looks like very old versions of CodeWarrior would emit it.

mcrxr (Move to Condition Register from XER) is a legacy PowerPC instruction that copies the top 4 bits of the XER register — SO (summary overflow), OV (overflow), CA (carry), and one reserved bit — into a chosen condition-register field (crf), then clears those bits in XER. It's a one-instruction shortcut for testing carry/overflow via branches (PowerPC has no direct "branch on carry" — you normally need mfxer + shifting/masking to get at those bits, or this fused move). It was part of the original POWER/PowerPC architecture but got dropped from later revisions of the ISA (Book I, from PowerPC ISA 2.03 onward) — many later implementations either don't wire it up in hardware, trap it, or run it slowly via microcode, so compilers stopped emitting it and disassemblers (including capstone, in both v5 and v6) generally don't bother decoding it.

The word the agent found, 0x7C000400, decodes as mcrxr cr0 — the classic textbook encoding for it. It shows up twice in [project]'s .text at 0x8011fb3c/0x8011fcd4, almost certainly inside old runtime/OS-level code

@Rot127

Rot127 commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

One cosmetic-only difference: v6 prints branch mnemonics in LLVM-style aliases

That is on purpose. One target of v6 is, that it outputs as closely to LLVM as possible.
Because LLVM is our ground truth we want to be as similar as possible. So we can compare bugs in Capstone against LLVM.

I asked for more info on mcrxr

You can add it to our LLVM fork and generate the inc files again with AutoSync.
The target description of LLVM is a little hard to get into. But if you check out our docs it shouldn't be too complicated with an LLM.

@Rot127

Rot127 commented Sep 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Check out this PR: capstone-engine/llvm-capstone#78

There I added some proprietary Apple AArch64 instructions. Your case will likely be simpler.

@StilesCrisis

StilesCrisis commented Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

The checked-in PowerPC tables in capstone are pinned to an "LLVM 18"-era snapshot from Dec 2024, so a straight regen pulls in a huge amount of unrelated upstream churn alongside my one-instruction change. I'm going to try and find a period-appropriate snapshot.

@Rot127

Rot127 commented Sep 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

huge amount of unrelated upstream churn alongside my one-instruction change.

You mean in the v5 branch? That won't work indeed. It is way too outdated.
If you want to add it the v5 branch, I think the only sensible way is to add an if () case in PPCDisassemble().

What meant above is, to add the instruction to the next branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@StilesCrisis @Rot127