xtensa: sign-extend scaled offsets in decodeOffset_* helpers#2997
Open
Samin061 wants to merge 1 commit into
Open
xtensa: sign-extend scaled offsets in decodeOffset_* helpers#2997Samin061 wants to merge 1 commit into
Samin061 wants to merge 1 commit into
Conversation
Collaborator
|
@b1llow Please take a look. |
Rot127
requested changes
Jul 18, 2026
Collaborator
There was a problem hiding this comment.
Revert this. The unit test below covers it already.
Contributor
Author
There was a problem hiding this comment.
Done, reverted. The two cases in issues.yaml cover both paths (the 64_16 sign extension and the 128_2 scaling), so test_poc.c was redundant.
Samin061
force-pushed
the
xtensa-offset-sign-extend
branch
from
July 19, 2026 06:27
f13d675 to
06f9808
Compare
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.
Your checklist for this pull request
Detailed description
The
decodeOffset_*helpers inarch/Xtensa/XtensaDisassembler.cdecode the scaled memory offset of the ESP32-S3ee.*instructions. Each one scales the field only when it is not already aligned, and never sign extends it:These offsets are signed, and two generated artifacts in the tree pin down the exact shape. The decoder tables give each field's width, and the printer gives the range it accepts.
decodeOffset_64_16Operandreads a 6 bit field andprintOffset_64_16_AsmOperandaccepts[-512,496]in steps of 16, which is exactly the 64 values ofSignExtend64(Imm << 4, 10). The rest of the family lines up the same way: 4 bit against[-128,112], and three 8 bit fields against[-1024,1016],[-2048,2032]and[-512,508].decodeOffset_128_1OperandanddecodeOffset_128_2Operandare the unsigned ones ([0,127]and[0,254]), so they only need the scale.Two things follow from the missing sign extension. When the decoded value lands outside the printer's range,
CS_ASSERT_RETdrops the operand, so release builds print the instruction with an operand missing and assert builds abort in the printer. When it happens to land inside the range, it is silently wrong.cstool esp32 2f71014ebefore:and after:
The 6 bit field here is 49, so the offset is
SignExtend64(49 << 4, 10), or -240. The old code produced 784 and the printer rejected it.I also tightened each
CS_ASSERT_RET_VALto the field width the tables actually pass.Immis the raw unsigned field, so the previousisIntN(8, Imm)/isIntN(16, Imm)guards were true for every input. This does touch the line from #2986: the swapped-argument fix there was right, the guard just never matched the field.Test plan
Added both cases to
tests/issues/issues.yaml(the64_16sign extension and the128_2scaling) andtest_xtensa_offset_sign_extensiontotests/integration/test_poc.c. On an ASAN/UBSan build test_poc aborts before the fix atXtensaInstPrinter.c:643withInvalid argument, value must be in range [-512,496], and exits clean with the fix.To check the change does not move anything it should not, I diffed cstool over 12000 random 4 byte words in
esp32andesp32s2against an unpatched build. 574 outputs differ: 506 recover an operand the printer had dropped, and 68 correct a silently wrong offset, for exampleee.vldbc.16.ip q7, a7, 0x60becoming0xc0andee.vst.l.64.ip q7, a3, 0x50becoming0x280. None of them change a value that was already correct. The existing Xtensa cases undertests/decode identically before and after, including the#2986word, whose offset field is 0.Closing issues