Skip to content

Commit

Permalink
[ARM] [ELF] Fix ARMMaterializeGV for Indirect calls
Browse files Browse the repository at this point in the history
Recent shouldAssumeDSOLocal changes (introduced by 961f31d8ad14c66)
do not take in consideration the relocation model anymore.  The ARM
fast-isel pass uses the function return to set whether a global symbol
is loaded indirectly or not, and without the expected information
llvm now generates an extra load for following code:

```
$ cat test.ll
@__asan_option_detect_stack_use_after_return = external global i32
define dso_local i32 @main(i32 %argc, i8** %argv) #0 {
entry:
  %0 = load i32, i32* @__asan_option_detect_stack_use_after_return,
align 4
  %1 = icmp ne i32 %0, 0
  br i1 %1, label %2, label %3

2:
  ret i32 0

3:
  ret i32 1
}

attributes #0 = { noinline optnone }

$ lcc test.ll -o -
[...]
main:
        .fnstart
[...]
        movw    r0, :lower16:__asan_option_detect_stack_use_after_return
        movt    r0, :upper16:__asan_option_detect_stack_use_after_return
        ldr     r0, [r0]
        ldr     r0, [r0]
        cmp     r0, #0
[...]
```

And without 'optnone' it produces:
```
[...]
main:
        .fnstart
[...]
        movw    r0, :lower16:__asan_option_detect_stack_use_after_return
        movt    r0, :upper16:__asan_option_detect_stack_use_after_return
        ldr     r0, [r0]
        clz     r0, r0
        lsr     r0, r0, #5
        bx      lr

[...]
```

This triggered a lot of invalid memory access in sanitizers for
arm-linux-gnueabihf.  I checked this patch both a stage1 built with
gcc and a stage2 bootstrap and it fixes all the Linux sanitizers
issues.

Reviewed By: MaskRay

Differential Revision: https://reviews.llvm.org/D95379
  • Loading branch information
zatrazz authored and memfrob committed Oct 4, 2022
1 parent 3140f6e commit 08518a8
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 4 deletions.
4 changes: 3 additions & 1 deletion llvm/lib/Target/ARM/ARMFastISel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -606,7 +606,9 @@ unsigned ARMFastISel::ARMMaterializeGV(const GlobalValue *GV, MVT VT) {
}
}

if (IsIndirect) {
if ((Subtarget->isTargetELF() && Subtarget->isGVInGOT(GV)) ||
(Subtarget->isTargetMachO() && IsIndirect) ||
Subtarget->genLongCalls()) {
MachineInstrBuilder MIB;
unsigned NewDestReg = createResultReg(TLI.getRegClassFor(VT));
if (isThumb2)
Expand Down
6 changes: 3 additions & 3 deletions llvm/test/CodeGen/ARM/fast-isel-intrinsic.ll
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@ define void @t2() nounwind ssp {

; ARM-ELF: movw [[REG1:r[0-9]+]], :lower16:temp
; ARM-ELF: movt [[REG1]], :upper16:temp
; ARM-ELF: add [[REG1]], r1, #4
; ARM-ELF-NEXT: add r1, r1, #16
; ARM-ELF: add r0, [[REG1]], #4
; ARM-ELF-NEXT: add r1, [[REG1]], #16

; ARM: movw r2, #17
; ARM: bl {{_?}}memcpy
Expand Down Expand Up @@ -106,7 +106,7 @@ define void @t3() nounwind ssp {

; ARM-ELF: movw [[REG0:r[0-9]+]], :lower16:temp
; ARM-ELF: movt [[REG0]], :upper16:temp
; ARM-ELF: add [[REG0]], r1, #4
; ARM-ELF: add r0, [[REG0]], #4
; ARM-ELF-NEXT: add r1, r1, #16

; ARM: movw r2, #10
Expand Down

0 comments on commit 08518a8

Please sign in to comment.