Skip to content

[finding - write your own PR title] loongarch: vstelm/xvstelm detail uses the offset immediate as the memory base register - #3150

Closed
Samin061 wants to merge 1 commit into
capstone-engine:nextfrom
Samin061:loongarch-stelm-mem-operand
Closed

Samin061 wants to merge 1 commit into
capstone-engine:nextfrom
Samin061:loongarch-stelm-mem-operand

Conversation

@Samin061

@Samin061 Samin061 commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

NOTE FOR THE ACCOUNT OWNER (do not publish this text as-is): capstone-engine/capstone forbids AI-written PR descriptions and commit messages (CONTRIBUTING.md, "AI guidelines"; the PR template repeats it). Before publishing, (1) rewrite the commit subject in your own words with git commit --amend (current placeholder subject: "loongarch: fix vstelm/xvstelm memory operand detail"), and (2) write the PR body yourself using the repository template (.github/PULL_REQUEST_TEMPLATE.md: checklist, Detailed description, Test plan, Closing issues). Keep it short; the maintainer (Rot127) has asked for brief descriptions. The AI guidelines allow AI for the code itself, so disclose AI assistance in one sentence in your own words. Facts to draw from:

Where: LoongArch_rewrite_memory_operand() in arch/LoongArch/LoongArchMapping.c. The vstelm.{b,h,w,d} and xvstelm.{b,h,w,d} instructions (forms FMT2RI8I*_VRII and FMT2RI8I*_XRII) have four operands: vd/xd, rj, si8 offset, element index. The function grouped them with the three-operand base+offset loads/stores and rewrote operand[-2], which for these instructions is the si8 immediate, into the LOONGARCH_OP_MEM operand. The immediate value therefore became mem.base and the element index became mem.disp.

Effect: cs_detail reports an invalid register id as the memory base (e.g. 214 for xvstelm.h $xr26, $t4, 0xd6, 1, bytes 1a ae 45 33, CS_MODE_LOONGARCH64) and lists it in regs_read via cs_regs_access. cs_reg_name() returns NULL for it in release builds; in Debug builds the getRegisterName() assert aborts (cstool and any consumer calling cs_reg_name on the reported id). Reproduce with: cstool -d loongarch64 "1a ae 45 33" (Debug build aborts with "RegNo && RegNo < 177"; release build prints mem.base = (null)).

Fix: handle the eight *stelm forms in their own case: rj (operand[-3]) becomes the memory operand with disp = si8, the element index moves down one slot, op_count is decremented. A sweep of the generated mapping shows these eight forms are the only four-operand memory forms that took the generic path (sc.q, llacq, screl were already special-cased).

Tests: regression cases for xvstelm.h and vstelm.b added to tests/details/loongarch.yaml (fail before the fix: regs_read_count 3 != 2; pass after). Locally run: cstest tests/details (DetailTests), tests/MC/LoongArch (1207 cases), ctest MCTests/IssueTests/FeaturesTests/NegativeTests/unit_/integration_ all pass on a Debug build; clang-format clean; git diff --check clean.

@github-actions github-actions Bot added the LoongArch Arch label Nov 2, 2026
@codecov

codecov Bot commented Nov 2, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (next@8b95af3). Learn more about missing BASE report.

Additional details and impacted files
@@           Coverage Diff           @@
##             next    #3150   +/-   ##
=======================================
  Coverage        ?   61.45%           
=======================================
  Files           ?      243           
  Lines           ?    56829           
  Branches        ?    10077           
=======================================
  Hits            ?    34925           
  Misses          ?    16708           
  Partials        ?     5196           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@amaanq

amaanq commented Nov 2, 2026

Copy link
Copy Markdown

This clearly violates the AI guidelines

@Rot127

Rot127 commented Nov 2, 2026

Copy link
Copy Markdown
Collaborator

Indeed, it does.

@Rot127 Rot127 closed this Nov 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants