Immediates that need shifts are misassembled for ARM #23374

jsonn · 2015-03-23T22:53:25Z


Bugzilla Link	23000
Resolution	FIXED
Resolved on	Feb 21, 2018 14:27
Version	trunk
OS	Linux
Blocks	#20796
CC	@compnerd,@DimitryAndric,@john-brawn-arm,@kbeyls,@nico,@ostannard,@rengolin,@smithp35,@TNorthover

Extended Description

Try to assemble:

AES_Te:
.word 1,2,3,4,5,6
.word 1,2,3,4,5,6
.word 1,2,3,4,5,6
.word 1,2,3,4,5,6
.word 1,2,3,4,5,6
.word 1,2,3,4,5,6
.word 1,2,3,4,5,6
.word 1,2,3,4,5,6
.word 1,2,3,4,5,6
.word 1,2,3,4,5,6
.word 1,2,3,4,5,6
AES_encrypt:
sub r10,r3,#(AES_encrypt-AES_Te) @ Te

and disassemble. The immediate for the sub is misencoded. This is observed in the OpenSSL assembly.

jsonn · 2015-03-23T22:53:25Z

assigned to @rengolin

john-brawn-arm · 2015-03-25T16:13:19Z

Adjusting the example slightly to:
AES_Te:
.word 1,2,3,4,5,6
.word 1,2,3,4,5,6
.word 1,2,3,4,5,6
.word 1,2,3,4,5,6
.word 1,2,3,4,5,6
.word 1,2,3,4,5,6
.word 1,2,3,4,5,6
.word 1,2,3,4,5,6
.word 1,2,3,4,5,6
.word 1,2,3,4,5,6
.word 1,2,3,4,5,6
AES_encrypt:
sub r10,r3,#(AES_encrypt-AES_Te) @ Te
sub r10,r3,#(6114)
.word (AES_encrypt-AES_Te)
.word (6114)

Then assembling with clang and disassembling with llvm-dis I see
AES_encrypt:
108: 08 a1 43 e2 sub r10, r3, #8, #2
10c: 42 af 43 e2 sub r10, r3, #264

$d.2:
110: 08 01 00 00 andeq r0, r0, r8, lsl #2
114: 08 01 00 00 andeq r0, r0, r8, lsl #2

It's only the sub with an immediate that's a label difference that's been misassembled.

compnerd · 2015-03-26T04:13:47Z

Yes, that makes sense. Ive not had enough time to fully isolate the issue. The adjustment is being evaluated correctly. It seems that the issue comes in really late (near the the of the fragment layout phase).

jsonn · 2015-03-26T14:29:44Z

The core of the issue is that add and sub take a 12bit immediate with optional shifting, but we use a 16bit fix up when we can't evaluate the immediate. A new ARM-specific fix up is needed for this,

rengolin · 2016-07-03T14:36:26Z

Blocking Chromium meta, since this seems to be the last of the Chromium bugs with IAS.

nico · 2016-07-06T00:02:11Z

(It looks like the last needed for boringssl in chromium, but not the last for all of chromium I think)

rengolin · 2016-07-06T00:58:28Z

Oh, sorry. I stand corrected. :-)

rengolin · 2016-07-26T16:20:18Z

This seems fixed on trunk:

...
100: 05 00 00 00 andeq r0, r0, r5
104: 06 00 00 00 andeq r0, r0, r6

AES_encrypt:
108: 42 af 43 e2 sub r10, r3, #264
10c: 42 af 43 e2 sub r10, r3, #264

$d:
110: 08 01 00 00 andeq r0, r0, r8, lsl #2
114: 08 01 00 00 andeq r0, r0, r8, lsl #2

Saleem, did you fix this? I can't find the commit...

This could also have been "accidentally" fixed by another commit... Peter was working around add/sub fixups and making some refactoring around it.

compnerd · 2016-07-27T02:23:52Z

No, I hadn't had a chance to work on it. I would assume that it was Peter's work. However, I think that we should at least check in some tests for the future.

rengolin · 2016-07-27T02:40:25Z

Good point. I can do that.

rengolin · 2016-07-27T14:16:09Z

Tests in r276858.

nico · 2016-08-08T20:31:30Z

This isn't fixed for boringssl in a chromium bug (which is probably the same source that the openssl bit is reduced from). Maybe it's due to thumb:

thakis@thakis:~/src/chrome/src/out/gnand$ cat ../../third_party/boringssl/linux-arm/crypto/aes/bsaes-armv7.S 
.arch	armv7-a
.fpu	neon

.text
.syntax	unified 	@ ARMv7-capable assembler is expected to handle this
.thumb

.type	_bsaes_decrypt8,%function
.align	4
_bsaes_decrypt8:
	adr	r6,_bsaes_decrypt8
	vldmia	r4!, {q9}		@ round 0 key
	add	r6,r6,#.LM0ISR-_bsaes_decrypt8

.type	_bsaes_const,%object
.align	6
_bsaes_const:
.LM0ISR:@ InvShiftRows constants
.quad	0x0a0e0206070b0f03, 0x0004080c0d010509
thakis@thakis:~/src/chrome/src/out/gnand$ ../../third_party/llvm-build/Release+Asserts/bin/clang -B../../third_party/android_tools/ndk/toolchains/arm-linux-androideabi-4.9/prebuilt/linux-x86_64/bin --target=arm-linux-androideabi -march=armv7-a -mfloat-abi=softfp -mthumb -mtune=generic-armv7-a -mfpu=neon -g2 --sysroot=../../third_party/android_tools/ndk/platforms/android-16/arch-arm -c ../../third_party/boringssl/linux-arm/crypto/aes/bsaes-armv7.S 
../../third_party/boringssl/linux-arm/crypto/aes/bsaes-armv7.S:13:12: error: invalid operand for instruction
 add r6,r6,#.LM0ISR-_bsaes_decrypt8
           ^

nico · 2016-08-08T20:31:53Z

(that was a somewhat reduced repro, not the original file, of course)

rengolin · 2016-08-15T17:31:31Z

Hi Nico,

Thanks for the new snippet, I can now reproduce it on ARM and Thumb. The old snippet still doesn't fail, so I'll abandon that investigation and look at the new reduced case.

cheers,
--renato

nico · 2017-06-10T23:17:43Z

This is still an issue, even though the diagnostic changed slightly. With the same repro as above:

../../third_party/llvm-build/Release+Asserts/bin/clang -B../../third_party/android_tools/ndk/toolchains/arm-linux-androideabi-4.9/prebuilt/linux-x86_64/bin --target=arm-linux-androideabi -march=armv7-a -mfloat-abi=softfp -mthumb -mtune=generic-armv7-a -mfpu=neon -g2 --sysroot=../../third_party/android_tools/ndk/platforms/android-16/arch-arm -c foo.S
-bash: ../../third_party/llvm-build/Release+Asserts/bin/clang: No such file or directory
Nicos-MacBook-Pro:src thakis$ cd out/gn
Nicos-MacBook-Pro:gn thakis$ ../../third_party/llvm-build/Release+Asserts/bin/clang -B../../third_party/android_tools/ndk/toolchains/arm-linux-androideabi-4.9/prebuilt/linux-x86_64/bin --target=arm-linux-androideabi -march=armv7-a -mfloat-abi=softfp -mthumb -mtune=generic-armv7-a -mfpu=neon -g2 --sysroot=../../third_party/android_tools/ndk/platforms/android-16/arch-arm -c ../../foo.S
../../foo.S:13:12: error: immediate operand must be in the range [0,4095]
 add r6,r6,#.LM0ISR-_bsaes_decrypt8
           ^

rengolin · 2017-06-11T11:00:20Z

The new message is due to a change by Oliver to clean up the asm messages. It shouldn't change the initial bug.

smithp35 · 2017-08-23T13:01:23Z

I think that I managed to fix this as part of pr28647 Support of thumb2's modified immediate assembly syntax is incomplete (committed Mon Jun 5 2017)

With trunk llvm-mc or clang I can assemble the latest reproducer without error and produce a file that disassembles to:

00000000 <_bsaes_decrypt8>:
0: f2af 0604 subw r6, pc, #4
4: ecf4 2b04 vldmia r4!, {d18-d19}
8: f106 0640 add.w r6, r6, #64 ; 0x40
c: bf00 nop
e: bf00 nop
10: bf00 nop
12: bf00 nop
14: bf00 nop
16: bf00 nop
18: bf00 nop
1a: bf00 nop
1c: bf00 nop
1e: bf00 nop
20: bf00 nop
22: bf00 nop
24: bf00 nop
26: bf00 nop
28: bf00 nop
2a: bf00 nop
2c: bf00 nop
2e: bf00 nop
30: bf00 nop
32: bf00 nop
34: bf00 nop
36: bf00 nop
38: bf00 nop
3a: bf00 nop
3c: bf00 nop
3e: bf00 nop

00000040 <_bsaes_const>:
40: 0f03 070b 0206 0a0e 0509 0d01 080c 0004

Can you take another look to see if there is still a problem? If not we should be able to resolve this pr.

rengolin · 2018-02-21T22:27:55Z

Marking this as resolved, since it was fixed months ago.

nico mentioned this issue Jul 24, 2014

[Meta] Chromium building with -integrated-as on ARM #20796

Closed

llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 9, 2021

This issue was closed.

Immediates that need shifts are misassembled for ARM #23374

Immediates that need shifts are misassembled for ARM #23374

Comments

jsonn commented Mar 23, 2015

Extended Description

jsonn commented Mar 23, 2015

Uh oh!

john-brawn-arm commented Mar 25, 2015

Uh oh!

compnerd commented Mar 26, 2015

Uh oh!

jsonn commented Mar 26, 2015

Uh oh!

rengolin commented Jul 3, 2016

Uh oh!

nico commented Jul 6, 2016

Uh oh!

rengolin commented Jul 6, 2016

Uh oh!

rengolin commented Jul 26, 2016

Uh oh!

compnerd commented Jul 27, 2016

Uh oh!

rengolin commented Jul 27, 2016

Uh oh!

rengolin commented Jul 27, 2016

Uh oh!

nico commented Aug 8, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nico commented Aug 8, 2016

Uh oh!

rengolin commented Aug 15, 2016

Uh oh!

nico commented Jun 10, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rengolin commented Jun 11, 2017

Uh oh!

smithp35 commented Aug 23, 2017

Uh oh!

rengolin commented Feb 21, 2018

Uh oh!

nico commented Aug 8, 2016 •

edited

Loading

nico commented Jun 10, 2017 •

edited

Loading