You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Fix add, sub, adds, subs and cmp encoding in the arm64 assembler ##arch - #27134
add , sub and cmp went through arithmetic () and cmp () , which build the byte swapped word by hand and miss several cases , so these came out as other instructions :
input
before ( decoded with llvm-mc )
after
subs x0, x1, x28
sub x0, x1, x28
20001ceb
sub w0, w1, w17
sub x0, x1, x17
2000114b
add x0, x0, 0x1000
add x0, x0, #0, lsl #12
00044091
add x0, x0, w28, uxtw
add x0, x0, x28
00403c8b
adds x0, x0, x28, lsl 3
adds x0, x0, x28
000c1cab
cmp x0, 0xfff
cmp x1, #63
1ffc3ff1
subs matched the sub prefix and lost its S bit , sub was always 64-bit , immediates above 0xfff ran into the shift bit , and extends and shift amounts were dropped .
addsub () now encodes the three forms from the manual in normal bit order and swaps once at the end , like stp does in #27132 :
immediate : 12 bits , lsl 12 written or implied by a value like 0x1000 , and a negative value turns add into sub and back , as llvm does
shifted register : lsl , lsr , asr with the amount checked against the register size
extended register : uxtb .. sxtx with an amount up to 4 , and used for lsl or no shift when sp is an operand , so add x0, sp, x1 is right too
register 31 is checked to be sp or xzr where each is allowed , so add xzr, x0, 1 and adds sp, x0, 1 are refused
add , adds , sub , subs and cmp ( as subs with xzr ) use it , adds () is removed .
found by assembling the same text with rasm2 -a arm.nz and llvm-mc . over the same 3915 line list , the encodings equal to llvm went from 2451 to 3116 and the wrong ones from 762 to 149 , 68 of those being the ldp / stp of #27132 . nothing that was right before changed , and nothing llvm refuses is accepted now . the 17 new lines in db/asm/arm_64 fail before the change except 3 that already worked , db/asm and db/tools/rasm2 have no failures .
Please rebase on master and resolve the conflicts first.
The shared encoder makes sense. I checked the PR head (5c262eb) with a small driver linked to r_util: all 17 new assembly cases match, versus 3 on the base. But there are two range checks to fix before merging:
imm = -imm overflows for 0x8000000000000000. UBSan catches it. add x0, x1, 0x8000000000000000 is then accepted as sub x0, x1, 0 (200000d1). Keep the magnitude unsigned, negate with unsigned arithmetic, then check the range. This also affects subs and cmp.
The extended-register path copies the parser's ut64 shift_amount into an int before checking it. add x0, x1, w2, uxtw 0xffffffff becomes a negative amount, passes amount <= 4, and triggers a left shift of a negative value. It emits 20fcffff. Keep amount as ut64 until after the 0..4 check. Please add rejection tests for these inputs, alongside the valid boundary of 4.
One nearby followup:
The shifted-register path still accepts SP as Rm. add x0, x1, sp emits add x0, x1, xzr; cmp x0, sp has the same problem. This already happens on the base. A small !(rm->reg_type & ARM_SP) check in that path would close the gap. Cover WSP too, and keep XZR/WZR working.
No obvious performance blocker here. The encoding work and the small bounded cmp operand move are constant time. These fixes should fit in the current helper without another abstraction.
the immediate stays a ut64 and is negated as 0 - imm , so 0x8000000000000000 stays too big and is rejected for add , sub , subs and cmp .
the extend amount stays ut64 until the <= 4 check , so uxtw 0xffffffff is rejected . uxtw 4 is in the asm tests as the valid boundary , and uxtw 5 is rejected .
the shifted register path rejects sp and wsp as rm now , so add x0, x1, sp , cmp x0, sp and cmp w0, wsp fail . xzr and wzr still work , and there are asm tests for both .
the rejected inputs are one test in db/cmd/cmd_pa that uses pa , so nothing forks . it fails on the old head and passes with the fix . db/asm and cmd_pa have no failures . the new encodings match llvm-mc .
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
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.
Description
add,subandcmpwent througharithmetic ()andcmp (), which build the byte swapped word by hand and miss several cases , so these came out as other instructions :subs x0, x1, x28sub x0, x1, x2820001cebsub w0, w1, w17sub x0, x1, x172000114badd x0, x0, 0x1000add x0, x0, #0, lsl #1200044091add x0, x0, w28, uxtwadd x0, x0, x2800403c8badds x0, x0, x28, lsl 3adds x0, x0, x28000c1cabcmp x0, 0xfffcmp x1, #631ffc3ff1subsmatched thesubprefix and lost its S bit ,subwas always 64-bit , immediates above0xfffran into the shift bit , and extends and shift amounts were dropped .addsub ()now encodes the three forms from the manual in normal bit order and swaps once at the end , likestpdoes in #27132 :lsl 12written or implied by a value like0x1000, and a negative value turnsaddintosuband back , as llvm doeslsl,lsr,asrwith the amount checked against the register sizeuxtb..sxtxwith an amount up to 4 , and used forlslor no shift whenspis an operand , soadd x0, sp, x1is right toosporxzrwhere each is allowed , soadd xzr, x0, 1andadds sp, x0, 1are refusedadd,adds,sub,subsandcmp( assubswithxzr) use it ,adds ()is removed .found by assembling the same text with
rasm2 -a arm.nzand llvm-mc . over the same 3915 line list , the encodings equal to llvm went from 2451 to 3116 and the wrong ones from 762 to 149 , 68 of those being theldp/stpof #27132 . nothing that was right before changed , and nothing llvm refuses is accepted now . the 17 new lines indb/asm/arm_64fail before the change except 3 that already worked ,db/asmanddb/tools/rasm2have no failures .