Skip to content

capstone2llvmir/arm64: handle ARM64_VAS_2D vector arrangement - #1235

Open
takano32 wants to merge 1 commit into
avast:masterfrom
takano32:fix/arm64-vas-2d
Open

takano32 wants to merge 1 commit into
avast:masterfrom
takano32:fix/arm64-vas-2d

Conversation

@takano32

Copy link
Copy Markdown
Contributor

Fixes #1225.

Problem

Capstone2LlvmIrTranslatorArm64_impl::extractVectorValue() switches on op.vas and handles every Capstone v5 VESS arrangement (16B/8B/4B/1B, 8H/4H/2H/1H, 4S/2S/1S, 1D, 1Q, INVALID) except ARM64_VAS_2D, which falls into the default: branch and throws GenericError("Arm64: extractVectorValue(): Unknown VESS type").

extractVectorValue() is invoked from loadOp() for every vector register operand, and the exception is caught by Decoder::runCatcher(), which logs the error and calls exit(1). So decompiling any AArch64 binary that contains a .2d-arranged SIMD operand — e.g. add v0.2d, v1.2d, v2.2d, which compilers emit routinely (inlined memcpy/memset, vectorized FP loops) — aborts the entire decompilation. This is the exact failure reported in #1225.

Fix

Treat 2D like the existing 1D case (64-bit elements), the same way the other multi-element arrangements (16B/8H/4S) already share their element-size branch with their single-element forms. One added line.

Testing

  • Added a regression test (add v0.2d, v1.2d, v2.2d). Without the fix it fails with the Unknown VESS type error from the issue; with the fix it passes.
  • Full retdec-tests-capstone2llvmir suite passes (3927 tests).

🤖 Generated with Claude Code

https://claude.ai/code/session_014nNMVbAuffaPPQuYdLZgD1

extractVectorValue() handled every Capstone v5 VESS arrangement except
ARM64_VAS_2D, so translating any instruction with a .2d vector operand
(e.g. `add v0.2d, v1.2d, v2.2d`) threw "Unknown VESS type", aborting
the whole decompilation of common AArch64 binaries.

Treat 2D like the existing 1D case (64-bit elements), the same way the
other multi-element arrangements (16B/8H/4S) already share their
element-size branch.

Fixes avast#1225

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014nNMVbAuffaPPQuYdLZgD1

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f628201201

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

val = irb.CreateLShr(val, llvm::ConstantInt::get(val->getType(), 32 * op.vector_index));
val = irb.CreateZExtOrTrunc(val, llvm::IntegerType::getInt32Ty(_module->getContext()));
return irb.CreateBitCast(val, llvm::Type::getFloatTy(_module->getContext()));
case ARM64_VAS_2D:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Keep .2d vectors out of scalar extraction

For whole-vector operands like the new regression case add v0.2d, v1.2d, v2.2d, routing ARM64_VAS_2D through this scalar lane extractor only loads one 64-bit element, bitcasts it through double, and translateAdd() then stores that single i64 result back into the 128-bit V register. Any non-zero upper 64-bit lane is therefore ignored or overwritten instead of being added lane-wise, so this turns the previous crash into silent wrong IR for common .2d SIMD arithmetic; vector operands should be handled as 128-bit/pseudo vector operations rather than extracted as a scalar element.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The observation is correct in isolation — for a whole-register .2d operand this path models only a single 64-bit lane. But that is exactly how this translator already models every multi-element arrangement: 16B/8B/4B, 8H/4H/2H, and 4S/2S go through the very same scalar extractor with vector_index == -1 (e.g. add v0.4s, v1.4s, v2.4s lifts today through the 4S branch a few lines above with the identical single-lane approximation, and storeOp() likewise stores a single value into the i128 V register for all of them). capstone2llvmir has no lane-wise vector semantics anywhere — V registers are plain i128 globals — so ARM64_VAS_2D being the one enumerator missing from this switch was an oversight of the Capstone v5 upgrade rather than a deliberately stricter path.

So the trade-off here is not "correct SIMD lift vs. approximate lift" — it is "the translator's established approximation vs. exit(1) for the whole binary" (#1225): today a single compiler-inlined memcpy aborts the entire decompilation. This PR intentionally makes 2D consistent with the existing behavior of all other arrangements and nothing more.

Proper lane-wise SIMD lifting would require redesigning the vector operand load/store path for all arrangements and every vector-capable instruction — far beyond the scope of this crash fix. Happy to open a follow-up issue to track that if the maintainers want one.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unknown VESS type

1 participant