ClangBuiltLinux / linux

Linux kernel source tree
Other
241 stars 14 forks source link

error: too many operands for instruction: tst r3,#1,0 #1337

Open nickdesaulniers opened 3 years ago

nickdesaulniers commented 3 years ago

via: https://github.com/ClangBuiltLinux/continuous-integration2/runs/2223604174?check_suite_focus=true

/builds/linux/arch/arm/kernel/relocate_kernel.S:30:12: error: too many operands for instruction
 tst r3,#1,0
           ^
/builds/linux/arch/arm/kernel/relocate_kernel.S:36:12: error: too many operands for instruction
 tst r3,#2,0
           ^
/builds/linux/arch/arm/kernel/relocate_kernel.S:43:12: error: too many operands for instruction
 tst r3,#4,0
           ^
/builds/linux/arch/arm/kernel/relocate_kernel.S:49:12: error: too many operands for instruction
 tst r3,#8,0
           ^

so this is a CONFIG_THUMB2_KERNEL build of ARCH=arm of android12-5.4. So code that was likely removed from newer versions of the kernel (5.10 and mainline didn't have this failure).

nickdesaulniers commented 3 years ago

https://github.com/ClangBuiltLinux/linux/issues/1337

oWo

nickdesaulniers commented 3 years ago

assembling+disassembling this with GNU binutils produces:

   0:   e3130001        tst     r3, #1

This case should be F5.1.262 TST (immediate) A1 encoding. The section

F1.7.7 Modified immediate constants in T32 and A32 instructions

has a further addendum about "Modified immediate constants in A32 instructions" which states:

An alternative syntax is available for a modified immediate constant that permits the programmer to specify the encoding directly. In this syntax, # is instead written as #, #, where: \ Is the numeric value of abcdefgh, in the range 0-255. \ Is twice the numeric value of rotation, an even number in the range 0-30

Table F1-3 Encoding of modified immediates in A32 processing instructions

is needed to understand what is meant by abcdefgh. It seems that this is what's being used in the kernel (for no good reason, IMO, since a <rot> of 0 does nothing).

cc @DavidSpickett

nickdesaulniers commented 3 years ago

I'm not sure if there's an existing AsmOperandClass for this? We might need one for "rotated immediates" (or something other than rot_imm).

jcai19 commented 3 years ago

It seems this would fail in both IAS and GAS?

$ cat foo.s 
.thumb
tst r3,#1,0

$ llvm-mc -triple=armv7a -filetype=obj foo.s -o ias.o
foo.s:2:11: error: too many operands for instruction
tst r3,#1,0

$ arm-linux-gnueabihf-as -march=armv7-a foo.s -o gas.o
foo.s: Assembler messages:
foo.s:2: Error: unshifted register required -- `tst r3,#1,0'
DavidSpickett commented 3 years ago

The kernel change was made in https://github.com/ClangBuiltLinux/linux/issues/770 / https://github.com/torvalds/linux/commit/91274f962e942c23c97ba7e4c7fa04277c533a45

I think this is related to the implementation of unified syntax.

Assembler Thumb Arm Unified Thumb Unified Arm
LLVM Not allowed Allowed Not allowed Allowed
GAS Not allowed Allowed Allowed Allowed

This file:

.syntax unified
.arm
tst r3, #1, 0
.thumb
tst r3, #1, 0

With GAS produces:

00000000 <.text>:
   0:   e3130001        tst     r3, #1
   4:   f013 0f01       tst.w   r3, #1

Note that it allows the syntax for convenience but still selects the thumb encoding for the second instruction.

I'll see if I can find another instruction in llvm that has done this. Could be a case by case thing and I'm sure we've missed some.

Edit: I couldn't find any apart from the Arm mode versions that accept both formats as you'd expect. Surprised there's not more code to handle unified syntax in general.