arm64: integrated assembler fails with "error: expected assembly-time absolute expression"

[samitolvanen]

Upstream commit f7b93d4 ("arm64/alternatives: use subsections for replacement sequences") broke the integrated assembler, which is also needed for LTO:

...
In file included from arch/arm64/kernel/debug-monitors.c:10:
In file included from ./include/linux/cpu.h:17:
In file included from ./include/linux/node.h:18:
In file included from ./include/linux/device.h:15:
In file included from ./include/linux/dev_printk.h:16:
In file included from ./include/linux/ratelimit.h:6:
In file included from ./include/linux/sched.h:15:
In file included from ./include/linux/sem.h:5:
In file included from ./include/uapi/linux/sem.h:5:
In file included from ./include/linux/ipc.h:7:
In file included from ./include/linux/rhashtable-types.h:15:
In file included from ./include/linux/workqueue.h:9:
In file included from ./include/linux/timer.h:6:
In file included from ./include/linux/ktime.h:24:
In file included from ./include/linux/time.h:74:
In file included from ./include/linux/time32.h:13:
In file included from ./include/linux/timex.h:65:
In file included from ./arch/arm64/include/asm/timex.h:8:
In file included from ./arch/arm64/include/asm/arch_timer.h:18:
In file included from ./include/linux/smp.h:82:
In file included from ./arch/arm64/include/asm/smp.h:28:
./arch/arm64/include/asm/percpu.h:115:1: error: expected assembly-time absolute expression
PERCPU_RET_OP(add, add, ldadd)
^
./arch/arm64/include/asm/percpu.h:105:2: note: expanded from macro 'PERCPU_RET_OP'
        __PERCPU_RET_OP_CASE(w,  , name, 32, op_llsc, op_lse)           \
        ^
./arch/arm64/include/asm/percpu.h:79:16: note: expanded from macro '__PERCPU_RET_OP_CASE'
        asm volatile (ARM64_LSE_ATOMIC_INSN(                            \
                      ^
./arch/arm64/include/asm/lse.h:37:2: note: expanded from macro 'ARM64_LSE_ATOMIC_INSN'
        ALTERNATIVE(llsc, __LSE_PREAMBLE lse, ARM64_HAS_LSE_ATOMICS)
        ^
./arch/arm64/include/asm/alternative.h:296:2: note: expanded from macro 'ALTERNATIVE'
        _ALTERNATIVE_CFG(oldinstr, newinstr, __VA_ARGS__, 1)
        ^
./arch/arm64/include/asm/alternative.h:98:2: note: expanded from macro '_ALTERNATIVE_CFG'
        __ALTERNATIVE_CFG(oldinstr, newinstr, feature, IS_ENABLED(cfg))
        ^
./arch/arm64/include/asm/alternative.h:80:14: note: expanded from macro '__ALTERNATIVE_CFG'
        ".previous\n\t"                                                 \
                    ^
<inline asm>:26:7: note: instantiated into assembly here
        .org    . - (664b-663b) + (662b-661b)
                ^

[nickdesaulniers]

cc @jcai19

[samitolvanen]

Edit: See below. This produces the same error, but for a different reason.

Stand-alone reproducer:

$ cat test.c 
void a(void) {
        asm(
        ".subsection 1  \n"
        "1:             \n"
        "nop            \n"
        "2:             \n"
        ".previous      \n"
        ".org . + 2b-1b \n"
        );
}
$ clang --target=aarch64-linux-gnu -integrated-as -c test.c
test.c:8:3: error: expected assembly-time absolute expression
        ".org . + 2b-1b \n"
         ^
<inline asm>:6:6: note: instantiated into assembly here
.org . + 2b-1b  
     ^
test.c:8:3: error: expected assembly-time absolute expression
        ".org . + 2b-1b \n"
         ^
<inline asm>:6:6: note: instantiated into assembly here
.org . + 2b-1b  
     ^
2 errors generated.

Not entirely sure why it prints out seemingly the same error twice though.

[jcai19]

Thanks for the reproducer! A slightly simplified example:

$ cat foo.s
1:
2:
.org . + 2b-1b

$ llvm-mc -filetype=obj -triple=arm64 foo.s -o foo.o
foo.s:3:6: error: expected assembly-time absolute expression
.org . + 2b-1b
^
foo.s:3:6: error: expected assembly-time absolute expression
.org . + 2b-1b
^
foo.s:3:6: error: expected assembly-time absolute expression
.org . + 2b-1b
^

if I put parentheses around 2b and 1b respectively as follows, then it assembled.
$ cat foo.s
1:
2:
.org . + (2b-1b)

Seems like IAS evaluates the operands in order and therefore fails to evaluate the first two operands. Will see if I can come up with a quick fix.

[samitolvanen]

Adding parenthesis fixes the reproducer, but the kernel has them already, so I wonder if creduce found a different problem...:

#define __ALTERNATIVE_CFG(oldinstr, newinstr, feature, cfg_enabled)     \
        ".if "__stringify(cfg_enabled)" == 1\n"                         \
        "661:\n\t"                                                      \
        oldinstr "\n"                                                   \
        "662:\n"                                                        \
        ".pushsection .altinstructions,\"a\"\n"                         \
        ALTINSTR_ENTRY(feature)                                         \
        ".popsection\n"                                                 \
        ".subsection 1\n"                                               \
        "663:\n\t"                                                      \
        newinstr "\n"                                                   \
        "664:\n\t"                                                      \
        ".previous\n\t"                                                 \
        ".org   . - (664b-663b) + (662b-661b)\n\t"                      \
        ".org   . - (662b-661b) + (664b-663b)\n"                        \
        ".endif\n"

[jcai19]

I think kernel has parentheses around each operand in this case, i.e. .org - (Expr1) + (Expr2). I haven't verified but adding one more pair of parentheses should fix the problem, .org - ((Expr1) + (Expr2)).

[samitolvanen]

I tested this change:

diff --git a/arch/arm64/include/asm/alternative.h b/arch/arm64/include/asm/alternative.h
index 12f0eb56a1cc..94c859e95f01 100644
--- a/arch/arm64/include/asm/alternative.h
+++ b/arch/arm64/include/asm/alternative.h
@@ -78,8 +78,8 @@ static inline void apply_alternatives_module(void *start, size_t length) { }
        newinstr "\n"                                                   \
        "664:\n\t"                                                      \
        ".previous\n\t"                                                 \
-       ".org   . - (664b-663b) + (662b-661b)\n\t"                      \
-       ".org   . - (662b-661b) + (664b-663b)\n"                        \
+       ".org   . - ((664b-663b) + (662b-661b))\n\t"                    \
+       ".org   . - ((662b-661b) + (664b-663b))\n"                      \
        ".endif\n"
 
 #define __ALTERNATIVE_CFG_CB(oldinstr, feature, cfg_enabled, cb)       \

And now I'm getting different errors:

In file included from init/main.c:17:
In file included from ./include/linux/module.h:13:
In file included from ./include/linux/stat.h:6:
In file included from ./arch/arm64/include/asm/stat.h:12:
In file included from ./include/linux/time.h:6:
In file included from ./include/linux/seqlock.h:36:
In file included from ./include/linux/spinlock.h:54:
In file included from ./include/linux/irqflags.h:16:
./arch/arm64/include/asm/irqflags.h:56:15: error: invalid .org offset '3608' (at offset '3616')
        asm volatile(ALTERNATIVE(
                     ^
./arch/arm64/include/asm/alternative.h:296:2: note: expanded from macro 'ALTERNATIVE'
        _ALTERNATIVE_CFG(oldinstr, newinstr, __VA_ARGS__, 1)
        ^
./arch/arm64/include/asm/alternative.h:98:2: note: expanded from macro '_ALTERNATIVE_CFG'
        __ALTERNATIVE_CFG(oldinstr, newinstr, feature, IS_ENABLED(cfg))
        ^
./arch/arm64/include/asm/alternative.h:80:14: note: expanded from macro '__ALTERNATIVE_CFG'
        ".previous\n\t"                                                 \
                    ^
<inline asm>:26:7: note: instantiated into assembly here
        .org    . - ((664b-663b) + (662b-661b))
                ^

[jcai19]

Thanks for the verification. Based on IAS's code the extra parentheses did work and made the assembling proceeded a little bit further. Will take a look at the new error.

[samitolvanen]

Note that the commit that tripped this bug didn't actually touch the .org statements, it just changed .pushsection / .popsection to .subsection / .previous.

[nickdesaulniers]

Right, so is .subsection handled differently than .pushsection in LLVM IAS? What's the difference between those directives?

[jcai19]

So if I replace .subsection and .previous with .pushsection and .popsection respectively in the reproducer, the issue still happens. I guess what I do not understand is how come the same issue was not exposed before the change? Did the change alter program semantics?

$ cat foo.s
#.subsection 1
.pushsection .altinstr_replacement, "a"
1:
nop
2:
#.previous
.popsection
.org . + 2b-1b

$ llvm-mc -filetype=obj -triple=arm64 foo.s -o clang.o
foo.s:8:6: error: expected assembly-time absolute expression
.org . + 2b-1b
^
foo.s:8:6: error: expected assembly-time absolute expression
.org . + 2b-1b
^
foo.s:8:6: error: expected assembly-time absolute expression
.org . + 2b-1b

So if understand correctly, the original code placed the code within .pushsection/.popsection pair into a new section (.altinstr_replacement), while the newer version puts the same code into a new subsection of the current section. So it appears that somewhere IAS failed to update the offset within the section and caused the assertion failure. It however still does not explain why the old code worked. Anyway, will keep investigating.

[samitolvanen]

My initial interestingness test for creduce was too simple. The reproducer fails with the same error, but for a different reason. Here's the full preprocessed arch/arm64/kernel/debug-monitors.c, which is one of the files that fails to build.

[jcai19]

Thanks for the full reproducer. I just noticed the change should be

diff --git a/arch/arm64/include/asm/alternative.h b/arch/arm64/include/asm/alternative.h
index 12f0eb56a1cc..94c859e95f01 100644
--- a/arch/arm64/include/asm/alternative.h
+++ b/arch/arm64/include/asm/alternative.h
@@ -78,8 +78,8 @@ static inline void apply_alternatives_module(void *start, size_t length) { }
        newinstr "\n"                                                   \
        "664:\n\t"                                                      \
        ".previous\n\t"                                                 \
-       ".org   . - (664b-663b) + (662b-661b)\n\t"                      \
-       ".org   . - (662b-661b) + (664b-663b)\n"                        \
+       ".org   . - ((664b-663b) - (662b-661b))\n\t"                    \
+       ".org   . - ((662b-661b) - (664b-663b))\n"                      \
        ".endif\n"
 
 #define __ALTERNATIVE_CFG_CB(oldinstr, feature, cfg_enabled, cb)       \

This probably explains the new error you saw, although it still failed and went back to the reported error. Still investigating.

[samitolvanen]

Ah, good catch!

I think this might be related to the .arch_extension lse directive. Here's another attempt at a reproducer:

$ cat test.c
void a(void) {
        asm(
        ".subsection 1                  \n"
        "1:                             \n"
        "nop                            \n"
        "2:                             \n"
        ".previous                      \n"
        "3:                             \n"
        "nop                            \n"
        "4:                             \n"
        ".subsection 1                  \n"
        "5:                             \n"
        ".arch_extension lse            \n"
        "nop                            \n"
        "6:                             \n"
        ".previous                      \n"
        ".org . - (6b-5b) + (4b-3b)     \n"
        );
}
$ clang --target=aarch64-linux-gnu -integrated-as -c test.c
test.c:17:10: error: expected assembly-time absolute expression
        ".org . - (6b-5b) + (4b-3b)     \n"
         ^
<inline asm>:15:6: note: instantiated into assembly here
.org . - (6b-5b) + (4b-3b)      
     ^
1 error generated.

This builds if I remove .arch_extension lse, or replace .subsection/previous with .pushsection/popsection:

$ cat test1.c 
void a(void) {
        asm(
        ".pushsection .mysection,\"a\"  \n"
        "1:                             \n"
        "nop                            \n"
        "2:                             \n"
        ".popsection                    \n"
        "3:                             \n"
        "nop                            \n"
        "4:                             \n"
        ".pushsection .mysection,\"a\"  \n"
        "5:                             \n"
        ".arch_extension lse            \n"
        "nop                            \n"
        "6:                             \n"
        ".popsection                    \n"
        ".org . - (6b-5b) + (4b-3b)     \n"
        );
}
$ clang --target=aarch64-linux-gnu -integrated-as -c test1.c
$ llvm-objdump -D test1.o
...
Disassembly of section .text:

0000000000000000 <a>:
       0: 1f 20 03 d5   nop
       4: c0 03 5f d6   ret

Disassembly of section .mysection:

0000000000000000 <$x.0>:
       0: 1f 20 03 d5   nop

0000000000000004 <$x.1>:
       4: 1f 20 03 d5   nop
...
$ cat test2.c
void a(void) {
        asm(
        ".subsection 1                  \n"
        "1:                             \n"
        "nop                            \n"
        "2:                             \n"
        ".previous                      \n"
        "3:                             \n"
        "nop                            \n"
        "4:                             \n"
        ".subsection 1                  \n"
        "5:                             \n"
        "nop                            \n"
        "6:                             \n"
        ".previous                      \n"
        ".org . - (6b-5b) + (4b-3b)     \n"
        );
}
$ clang --target=aarch64-linux-gnu -integrated-as -c test2.c
$ llvm-objdump -D test2.o
...
Disassembly of section .text:

0000000000000000 <a>:
       0: 1f 20 03 d5   nop
       4: c0 03 5f d6   ret

0000000000000008 <$x.0>:
       8: 1f 20 03 d5   nop
       c: 1f 20 03 d5   nop
...

[jcai19]

Awesome. Thanks for reducing the reproducer. Just want to mention here for future references that I played with IAS a bit more and realized it had nothing to do with parentheses. The existing ones should be enough.

[jcai19]

This actually reminders me of #742. IAS implements each section as a list of data blocks called fragments. Currently it can only resolve the differences of labels in the same fragment. .arch_extension creates a new fragment, and therefore causing IAS to fail. I have a LLVM patch ( https://reviews.llvm.org/D69411) from a while ago to add support to resolve the difference of labels if the fragments they belong to are separated by fix-sized fragments only (the size of some fragments may be changed, such as those including jump instructions). There were concerns that the list of fragments may be changed after the differences are resolved, so the patch was never submitted. I guess I will have to revisit the patch and see if there is anything we can do. Meanwhile, a possible workaround could be to move .arch_extension to the beginning of the code, if that's possible.

[samitolvanen]

Meanwhile, a possible workaround could be to move .arch_extension to the beginning of the code, if that's possible.

Moving .arch_extension lse to the beginning of the assembly block fixes the reproducer, but the kernel still fails to build. It looks like something is broken when we have multiple functions with the same assembly construct:

$ cat test3.c
void a(void)
{ 
        asm(
        ".arch_extension lse            \n"
        "1:                             \n"
        "nop                            \n"
        "2:                             \n"
        ".subsection 1                  \n"
        "3:                             \n"
        "nop                            \n"
        "4:                             \n"
        ".previous                      \n"
        ".org . - (4b-3b) + (2b-1b)     \n"
        );
}
/* identical to a() */
void b(void)
{
        asm(
        ".arch_extension lse            \n"
        "1:                             \n"
        "nop                            \n"
        "2:                             \n"
        ".subsection 1                  \n"
        "3:                             \n"
        "nop                            \n"
        "4:                             \n"
        ".previous                      \n"
        ".org . - (4b-3b) + (2b-1b)     \n"
        );
}
$ clang --target=aarch64-linux-gnu -integrated-as -c test3.c
test3.c:29:3: error: expected assembly-time absolute expression
        ".org . - (4b-3b) + (2b-1b)     \n"
         ^
<inline asm>:10:6: note: instantiated into assembly here
.org . - (4b-3b) + (2b-1b)      
     ^
1 error generated.

Removing one of the functions fixes the issue, and so does removing the .arch_extension directives. I assume this is the same bug, but I'm not sure how we can work around this in the kernel.

[jcai19]

Thanks. The given example assembles if I first compile it into an assembly file and then use llvm-mc to do the assembling. Don't know if the assembly was generated correctly, but if the answer is yes, then it seems the handling of inline assembly is problematic in this case.

$ clang --target=aarch64-linux-gnu -c test3.c -o test3.o
test3.c:29:10: error: expected assembly-time absolute expression
        ".org . - (14b-13b) + (12b-11b)     \n"
         ^
<inline asm>:10:6: note: instantiated into assembly here
.org . - (14b-13b) + (12b-11b)     
     ^

$ clang --target=aarch64-linux-gnu -S test3.c -o test3.s
$ cat test3.s 
	.text
	.file	"test3.c"
	.globl	a                       // -- Begin function a
	.p2align	2
	.type	a,@function
a:                                      // @a
// %bb.0:                               // %entry
	//APP
.Ltmp0:
	nop
.Ltmp1:
	.text	1
.Ltmp2:
	nop
.Ltmp3:
	.text

.Ltmp4:
.org (.Ltmp4-(.Ltmp3-.Ltmp2))+(.Ltmp1-.Ltmp0), 0

	//NO_APP
	ret
.Lfunc_end0:
	.size	a, .Lfunc_end0-a
                                        // -- End function
	.globl	b                       // -- Begin function b
	.p2align	2
	.type	b,@function
b:                                      // @b
// %bb.0:                               // %entry
	//APP
.Ltmp5:
	nop
.Ltmp6:
	.text	1
.Ltmp7:
	nop
.Ltmp8:
	.text

.Ltmp9:
.org (.Ltmp9-(.Ltmp8-.Ltmp7))+(.Ltmp6-.Ltmp5), 0

	//NO_APP
	ret
.Lfunc_end1:
	.size	b, .Lfunc_end1-b
                                        // -- End function
	.ident	"clang version 11.0.0 (https://github.com/llvm/llvm-project.git b925ca37a8f28851e110b8f0cd6e7f9a36a15d65)"
	.section	".note.GNU-stack","",@progbits
	.addrsig
$ llvm-mc -filetype=obj -triple=arm64 test3.s -o test3.o
$ llvm-objdump -d test3.o

test3.o:	file format elf64-littleaarch64


Disassembly of section .text:

0000000000000000 <a>:
       0: 1f 20 03 d5  	nop
       4: c0 03 5f d6  	ret

0000000000000008 <b>:
       8: 1f 20 03 d5  	nop
       c: c0 03 5f d6  	ret
      10: 1f 20 03 d5  	nop
      14: 1f 20 03 d5  	nop