Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

clang ias, thumb2: proc-v7.S:85:60: error: branch target out of range #1286

Closed
arndb opened this issue Jan 29, 2021 · 7 comments
Closed

clang ias, thumb2: proc-v7.S:85:60: error: branch target out of range #1286

arndb opened this issue Jan 29, 2021 · 7 comments
Assignees
Labels
[ARCH] arm32 This bug impacts ARCH=arm [FIXED][LLVM] 13 This bug was fixed in LLVM 13.x [TOOL] integrated-as The issue is relevant to LLVM integrated assembler

Comments

@arndb
Copy link

arndb commented Jan 29, 2021

Building a thumb2-enabled arm32 kernel with clang ias and CONFIG_SMP_ON_UP, I get a couple of errors like

arch/arm/mm/proc-v7.S:85:60: error: branch target out of range
.pushsection ".alt.smp.init", "a" ; .long 9998b - . ; b.w . + (1f - 9998b) ; .popsection

The underlying code looks like

#ifdef CONFIG_SMP
#define ALT_SMP(instr...)                                       \
9998:   instr
/*
 * Note: if you get assembler errors from ALT_UP() when building with
 * CONFIG_THUMB2_KERNEL, you almost certainly need to use
 * ALT_SMP( W(instr) ... )
 */
#define ALT_UP(instr...)                                        \
        .pushsection ".alt.smp.init", "a"                       ;\
        .long   9998b - .                                       ;\
9997:   instr                                                   ;\
        .if . - 9997b == 2                                      ;\
                nop                                             ;\
        .endif                                                  ;\
        .if . - 9997b != 4                                      ;\
                .error "ALT_UP() content must assemble to exactly 4 bytes";\
        .endif                                                  ;\
        .popsection
#define ALT_UP_B(label)                                 \
        .pushsection ".alt.smp.init", "a"                       ;\
        .long   9998b - .                                       ;\
        W(b)    . + (label - 9998b)                                     ;\
        .popsection
#else
#define ALT_SMP(instr...)
#define ALT_UP(instr...) instr
#define ALT_UP_B(label) b label
#endif

and this was recently changed with commit 450abd3 ("ARM: kernel: use relative
references for UP/SMP alternatives").

@nickdesaulniers nickdesaulniers added [ARCH] arm32 This bug impacts ARCH=arm [TOOL] integrated-as The issue is relevant to LLVM integrated assembler labels Jan 29, 2021
@nickdesaulniers
Copy link
Member

nickdesaulniers commented Feb 10, 2021

cc @jcai19

This is the issue observed in ALT_UP_B in arch/arm/include/asm/assembler.h with CONFIG_THUMB2_KERNEL.

@nickdesaulniers
Copy link
Member

nickdesaulniers commented Feb 10, 2021

Smaller test case (requires https://reviews.llvm.org/D95872)

.syntax unified
9998:
        nop.w @ don't use adr_l inside ALT_SMP()
.pushsection ".alt.smp.init", "a"
.long 9998b - .
        b.w . + (1f - 9998b)
.popsection
1:
$ clang --target=arm-linux-gnueabi -Wa,-mthumb -Wa,-march=armv7-a foo.s
foo.s:6:5: error: branch target out of range
b.w . + (1f - 9998b)
    ^

vs

$ arm-linux-gnueabi-as -mthumb -march=armv7-a foo.s
$ arm-linux-gnueabi-objdump -Dr a.out
...
Disassembly of section .text:

00000000 <.text>:
   0:   f3af 8000       nop.w

Disassembly of section .alt.smp.init:

00000000 <.alt.smp.init>:
   0:   00000000        andeq   r0, r0, r0
                        0: R_ARM_REL32  .text
   4:   f000 b800       b.w     8 <.alt.smp.init+0x8>
...

@jcai19 jcai19 self-assigned this Feb 10, 2021
@nickdesaulniers
Copy link
Member

nickdesaulniers commented Feb 24, 2021

seems like target specific parsing, since

9998:
  jmp . + (1f - 9998b + 2)
1:
  nop

works for x86, but

.syntax unified

9998:
  b.w . + (1f - 9998b + 2)
1:
  nop

fails for clang --target=arm-linux-gnueabi -Wa,-mthumb -Wa,-march=armv7-a.

Looks like calls to parseExpression in parseOperand are failing. I wonder why? cc @DavidSpickett Perhaps following how X86AsmParser resolves this might shed some more light.

@jcai19
Copy link
Member

jcai19 commented Feb 24, 2021

Thanks for the reduced test case. I also reproduced the issue with llvm-mc.

$ cat foo.s
.syntax unified

9998:
  b.w . + (1f - 9998b + 2)
1:
  nop

$ llvm-mc -triple=thumbv7a-linux-gnueabi -filetype=obj foo.s -o clang.o
foo.s:4:7: error: branch target out of range
  b.w . + (1f - 9998b + 2)

@jcai19
Copy link
Member

jcai19 commented Feb 24, 2021

Further reduced test case:

$ cat foo.s
.syntax unified

9998:
  b.w . + 2

$ llvm-mc -triple=thumbv7a-linux-gnueabi -filetype=obj foo.s -o clang.o
foo.s:4:7: error: branch target out of range
  b.w . + 2

But if I further reduce operand of the jump instruction then the test case works.

$ cat foo.s
.syntax unified

9998:
  b.w .

$ llvm-mc -triple=thumbv7a-linux-gnueabi -filetype=obj foo.s -o clang.o
$ echo $?
0
$ arm-linux-gnueabihf-objdump -d clang.o
clang.o:     file format elf32-littlearm


Disassembly of section .text:

00000000 <.text>:
   0:	f7ff bffe 	b.w	0 <.text>

The difference is caused by one particular line (https://llvm.org/doxygen/ARMAsmParser_8cpp_source.html#l01072) of code in ARMOperand::isSignedOffset, which returns opposite boolean values for the two cases. Will do more investigation and see if we can handle the former casse by fixing this line in LLVM.

@jcai19
Copy link
Member

jcai19 commented Feb 25, 2021

So the fundamental issue here is that LLVM ARM backend tries to validate the range of branch targets before symbolic values are resolved (i.e. before the layout of fragments are finalized). LLVM also validates the range after the values are known, so disabling the first check for symbolic expressions (which they already do if the expression is a single symbol) seems to work for me. I have a patch locally but it seems LLVM Phabricator is down for the moment. Will send it for review once it comes back.

@jcai19
Copy link
Member

jcai19 commented Feb 25, 2021

LLVM patch sent: https://reviews.llvm.org/D97501

@jcai19 jcai19 added the [PATCH] Submitted A patch has been submitted for review label Feb 25, 2021
nickdesaulniers added a commit to ClangBuiltLinux/continuous-integration2 that referenced this issue Feb 26, 2021
arm32_v6 isn't booting with IAS:
ClangBuiltLinux/linux#1313

arm32_v7 is blocked on one last build issue:
ClangBuiltLinux/linux#1286

arm32_all{mod|yes}config is blocked on some patches that will probably
land in the 5.12 merge window:
https://www.armlinux.org.uk/developer/patches/viewpatch.php?id=9061/1
https://www.armlinux.org.uk/developer/patches/viewpatch.php?id=9062/1

I need to go chase backports back to 4.19.
jcai19 added a commit to llvm/llvm-project that referenced this issue Mar 2, 2021
Currently ARM backend validates the range of branch targets before the
layout of fragments is finalized. This causes build failure if symbolic
expressions are used, with the exception of a single symbolic value.
For example, "b.w ." works but "b.w . + 2" currently fails to
assemble. This fixes the issue by delaying this check (in
ARMAsmParser::validateInstruction) of b.w instructions until the symbol
expressions are resolved (in ARMAsmBackend::adjustFixupValue).

Link:
ClangBuiltLinux/linux#1286

Reviewed By: MaskRay

Differential Revision: https://reviews.llvm.org/D97568
@nickdesaulniers nickdesaulniers added [FIXED][LLVM] 13 This bug was fixed in LLVM 13.x and removed [PATCH] Submitted A patch has been submitted for review labels Mar 2, 2021
morehouse pushed a commit to morehouse/llvm-project that referenced this issue Mar 4, 2021
Currently ARM backend validates the range of branch targets before the
layout of fragments is finalized. This causes build failure if symbolic
expressions are used, with the exception of a single symbolic value.
For example, "b.w ." works but "b.w . + 2" currently fails to
assemble. This fixes the issue by delaying this check (in
ARMAsmParser::validateInstruction) of b.w instructions until the symbol
expressions are resolved (in ARMAsmBackend::adjustFixupValue).

Link:
ClangBuiltLinux/linux#1286

Reviewed By: MaskRay

Differential Revision: https://reviews.llvm.org/D97568
mem-frob pushed a commit to draperlaboratory/hope-llvm-project that referenced this issue Oct 7, 2022
Currently ARM backend validates the range of branch targets before the
layout of fragments is finalized. This causes build failure if symbolic
expressions are used, with the exception of a single symbolic value.
For example, "b.w ." works but "b.w . + 2" currently fails to
assemble. This fixes the issue by delaying this check (in
ARMAsmParser::validateInstruction) of b.w instructions until the symbol
expressions are resolved (in ARMAsmBackend::adjustFixupValue).

Link:
ClangBuiltLinux/linux#1286

Reviewed By: MaskRay

Differential Revision: https://reviews.llvm.org/D97568
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[ARCH] arm32 This bug impacts ARCH=arm [FIXED][LLVM] 13 This bug was fixed in LLVM 13.x [TOOL] integrated-as The issue is relevant to LLVM integrated assembler
Projects
None yet
Development

No branches or pull requests

3 participants