Repository navigation
Conversation
SLLI encodes its shift as 32 - shift in a 5-bit field, so field 0 means a shift of 32. decodeShimm1_31Operand already decodes this correctly, but printShimm0_31_AsmOperand only accepted 0-31, so the shift was dropped from the text (and assertion builds aborted). Accept 0-32 in the printer. This operand range is only used by SLLI. Example (esp32): 00 e8 01 before: slli a14, a8, after: slli a14, a8, 0x20 Add a test case to tests/details/xtensa.yaml.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## next #3130 +/- ##
=======================================
Coverage ? 61.29%
=======================================
Files ? 243
Lines ? 56814
Branches ? 10075
=======================================
Hits ? 34822
Misses ? 16794
Partials ? 5198 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This branch has not been deployed
This file contains hidden or 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
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.
Summary
For Xtensa
slliwith a shift amount of 32, the shift was missing from theprinted text, and builds with assertions enabled aborted.
Reproducer
Arch
CS_ARCH_XTENSA, mode esp32 (also esp32s2 and esp8266), bytes00 e8 01:slli a14, a8,with the shift missing, although the detail operandcorrectly holds
imm 0x20. With assertions enabled, it aborts inprintShimm0_31_AsmOperand(arch/Xtensa/XtensaInstPrinter.c:271).slli a14, a8, 0x20Found by running random bytes through
cs_disasmon every architecture underASan and UBSan.
Cause
SLLI stores
32 - shiftin a 5-bit field. The decoder(
decodeShimm1_31Operand) correctly turns field 0 into a shift of 32, but theprinter only accepted 0–31, so its range check failed and the operand text was
never printed.
32 is the correct value: GNU binutils' Xtensa tables (
bfd/xtensa-modules.c,operand
msalp32) also decode field 0 as 32. The 0–31 range comes fromEspressif's LLVM fork, which the Xtensa module is generated from. It declares
SLLI's operand as
shimm0_31but decodes it as 1–32. Upstream LLVM main usesshimm1_31for SLLI and has the same mismatch.Fix
The printer now accepts 0–32, with a one-line comment
(
XtensaInstPrinter.c:269). This operand range is used only by SLLI. This isa hand edit to a translated file, which auto-sync allows and preserves on
regeneration.
I kept 0 in the range (rather than 1–32, since the decoder never produces 0)
so the check still holds if the decoder is later regenerated to map field 0
to 0. Happy to change it to 1–32 if you prefer.
Testing
tests/details/xtensa.yaml, checking both the textand the operand detail. It aborts without the fix and passes with it.
new case also passes in a Release build.
clang-formatmakes no changes;git diff --checkis clean.Not included
The same fuzzing found a RISC-V issue:
cv.bitrev(db 16 18 dbon riscv32).The generated table reads its 2-bit immediate as 5 bits, and upstream LLVM has
the same bug. A proper fix means changing LLVM's instruction definitions and
regenerating the tables, so I'll leave that for a separate PR or issue.
AI disclosure
none