[HWASan] [GlobalISel] Add +tagged-globals backend feature for GlobalISel
authorMitch Phillips <31459023+hctim@users.noreply.github.com>
Mon, 3 Aug 2020 20:55:27 +0000 (13:55 -0700)
committerMitch Phillips <31459023+hctim@users.noreply.github.com>
Mon, 3 Aug 2020 21:28:44 +0000 (14:28 -0700)
GlobalISel is the default ISel for aarch64 at -O0. Prior to D78465, GlobalISel
didn't have support for dealing with address-of-global lowerings, so it fell
back to SelectionDAGISel.

HWASan Globals require special handling, as they contain the pointer tag in the
top 16-bits, and are thus outside the code model. We need to generate a `movk`
in the instruction sequence with a G3 relocation to ensure the bits are
relocated properly. This is implemented in SelectionDAGISel, this patch does
the same for GlobalISel.

GlobalISel and SelectionDAGISel differ in their lowering sequence, so there are
differences in the final instruction sequence, explained in
`tagged-globals.ll`. Both of these implementations are correct, but GlobalISel
is slightly larger code size / slightly slower (by a couple of arithmetic
instructions). I don't see this as a problem for now as GlobalISel is only on
by default at `-O0`.

Reviewed By: aemerson, arsenm

Differential Revision: https://reviews.llvm.org/D82615

compiler-rt/test/hwasan/TestCases/exported-tagged-global.c [new file with mode: 0644]
llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp
llvm/test/CodeGen/AArch64/tagged-globals.ll

diff --git a/compiler-rt/test/hwasan/TestCases/exported-tagged-global.c b/compiler-rt/test/hwasan/TestCases/exported-tagged-global.c
new file mode 100644 (file)
index 0000000..198d878
--- /dev/null
@@ -0,0 +1,16 @@
+// RUN: %clang_hwasan %s -o %t
+// RUN: %run %t
+// RUN: %clang_hwasan -O1 %s -o %t
+// RUN: %run %t
+// RUN: %clang_hwasan -O1 -mllvm --aarch64-enable-global-isel-at-O=1 %s -o %t
+// RUN: %run %t
+
+static int global;
+
+__attribute__((optnone)) int *address_of_global() { return &global; }
+
+int main(int argc, char **argv) {
+  int *global_address = address_of_global();
+  *global_address = 13;
+  return 0;
+}
index 6e5563a..063c451 100644 (file)
@@ -351,7 +351,7 @@ AArch64LegalizerInfo::AArch64LegalizerInfo(const AArch64Subtarget &ST)
 
     if (DstSize == 128 && !Query.Types[0].isVector())
       return false; // Extending to a scalar s128 needs narrowing.
-    
+
     // Make sure that we have something that will fit in a register, and
     // make sure it's a power of 2.
     if (DstSize < 8 || DstSize > 128 || !isPowerOf2_32(DstSize))
@@ -676,6 +676,27 @@ bool AArch64LegalizerInfo::legalizeSmallCMGlobalValue(MachineInstr &MI,
   // Set the regclass on the dest reg too.
   MRI.setRegClass(ADRP.getReg(0), &AArch64::GPR64RegClass);
 
+  // MO_TAGGED on the page indicates a tagged address. Set the tag now. We do so
+  // by creating a MOVK that sets bits 48-63 of the register to (global address
+  // + 0x100000000 - PC) >> 48. The additional 0x100000000 offset here is to
+  // prevent an incorrect tag being generated during relocation when the the
+  // global appears before the code section. Without the offset, a global at
+  // `0x0f00'0000'0000'1000` (i.e. at `0x1000` with tag `0xf`) that's referenced
+  // by code at `0x2000` would result in `0x0f00'0000'0000'1000 - 0x2000 =
+  // 0x0eff'ffff'ffff'f000`, meaning the tag would be incorrectly set to `0xe`
+  // instead of `0xf`.
+  // This assumes that we're in the small code model so we can assume a binary
+  // size of <= 4GB, which makes the untagged PC relative offset positive. The
+  // binary must also be loaded into address range [0, 2^48). Both of these
+  // properties need to be ensured at runtime when using tagged addresses.
+  if (OpFlags & AArch64II::MO_TAGGED) {
+    ADRP = MIRBuilder.buildInstr(AArch64::MOVKXi, {LLT::pointer(0, 64)}, {ADRP})
+               .addGlobalAddress(GV, 0x100000000,
+                                 AArch64II::MO_PREL | AArch64II::MO_G3)
+               .addImm(48);
+    MRI.setRegClass(ADRP.getReg(0), &AArch64::GPR64RegClass);
+  }
+
   MIRBuilder.buildInstr(AArch64::G_ADD_LOW, {DstReg}, {ADRP})
       .addGlobalAddress(GV, 0,
                         OpFlags | AArch64II::MO_PAGEOFF | AArch64II::MO_NC);
index b0cf882..cdca6c7 100644 (file)
-; RUN: llc < %s | FileCheck %s
+; RUN: llc --relocation-model=static < %s \
+; RUN:   | FileCheck %s --check-prefixes=CHECK-STATIC,CHECK-SELECTIONDAGISEL
+; RUN: llc --relocation-model=pic < %s \
+; RUN:   | FileCheck %s --check-prefix=CHECK-PIC
+
+; Ensure that GlobalISel lowers correctly. GlobalISel is the default ISel for
+; -O0 on aarch64. GlobalISel lowers the instruction sequence in the static
+; relocation model different to SelectionDAGISel. GlobalISel does the lowering
+; of AddLow *after* legalization, and thus doesn't differentiate between
+; address-taken-only vs. address-taken-for-loadstore. Hence, we generate a movk
+; instruction for load/store instructions as well with GlobalISel. GlobalISel
+; also doesn't have the scaffolding to correctly check the bounds of the global
+; offset, and cannot fold the lo12 bits into the load/store. Neither of these
+; things are a problem as GlobalISel is only used by default at -O0, so we don't
+; mind the code size and performance increase.
+
+; RUN: llc --aarch64-enable-global-isel-at-O=0 -O0 < %s \
+; RUN:   | FileCheck %s --check-prefixes=CHECK-STATIC,CHECK-GLOBALISEL
+; RUN: llc --aarch64-enable-global-isel-at-O=0 -O0 --relocation-model=pic < %s \
+; RUN:   | FileCheck %s --check-prefix=CHECK-PIC
 
 target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
 target triple = "aarch64-unknown-linux-android"
 
-@global = external hidden global i32
+@global = external global i32
 declare void @func()
 
 define i32* @global_addr() #0 {
-  ; CHECK: global_addr:
-  ; CHECK: adrp x0, :pg_hi21_nc:global
-  ; CHECK: movk x0, #:prel_g3:global+4294967296
-  ; CHECK: add x0, x0, :lo12:global
+  ; Static relocation model has common codegen between SelectionDAGISel and
+  ; GlobalISel when the address-taken of a global isn't folded into a load or
+  ; store instruction.
+  ; CHECK-STATIC: global_addr:
+  ; CHECK-STATIC: adrp [[REG:x[0-9]+]], :pg_hi21_nc:global
+  ; CHECK-STATIC: movk [[REG]], #:prel_g3:global+4294967296
+  ; CHECK-STATIC: add x0, [[REG]], :lo12:global
+  ; CHECK-STATIC: ret
+
+  ; CHECK-PIC: global_addr:
+  ; CHECK-PIC: adrp [[REG:x[0-9]+]], :got:global
+  ; CHECK-PIC: ldr x0, {{\[}}[[REG]], :got_lo12:global]
+  ; CHECK-PIC: ret
+
   ret i32* @global
 }
 
 define i32 @global_load() #0 {
-  ; CHECK: global_load:
-  ; CHECK: adrp x8, :pg_hi21_nc:global
-  ; CHECK: ldr w0, [x8, :lo12:global]
+  ; CHECK-SELECTIONDAGISEL: global_load:
+  ; CHECK-SELECTIONDAGISEL: adrp [[REG:x[0-9]+]], :pg_hi21_nc:global
+  ; CHECK-SELECTIONDAGISEL: ldr w0, {{\[}}[[REG]], :lo12:global{{\]}}
+  ; CHECK-SELECTIONDAGISEL: ret
+
+  ; CHECK-GLOBALISEL: global_load:
+  ; CHECK-GLOBALISEL: adrp [[REG:x[0-9]+]], :pg_hi21_nc:global
+  ; CHECK-GLOBALISEL: movk [[REG]], #:prel_g3:global+4294967296
+  ; CHECK-GLOBALISEL: add [[REG]], [[REG]], :lo12:global
+  ; CHECK-GLOBALISEL: ldr w0, {{\[}}[[REG]]{{\]}}
+  ; CHECK-GLOBALISEL: ret
+
+  ; CHECK-PIC: global_load:
+  ; CHECK-PIC: adrp [[REG:x[0-9]+]], :got:global
+  ; CHECK-PIC: ldr  [[REG]], {{\[}}[[REG]], :got_lo12:global]
+  ; CHECK-PIC: ldr w0, {{\[}}[[REG]]{{\]}}
+  ; CHECK-PIC: ret
+
   %load = load i32, i32* @global
   ret i32 %load
 }
 
+define void @global_store() #0 {
+  ; CHECK-SELECTIONDAGISEL: global_store:
+  ; CHECK-SELECTIONDAGISEL: adrp [[REG:x[0-9]+]], :pg_hi21_nc:global
+  ; CHECK-SELECTIONDAGISEL: str wzr, {{\[}}[[REG]], :lo12:global{{\]}}
+  ; CHECK-SELECTIONDAGISEL: ret
+
+  ; CHECK-GLOBALISEL: global_store:
+  ; CHECK-GLOBALISEL: adrp [[REG:x[0-9]+]], :pg_hi21_nc:global
+  ; CHECK-GLOBALISEL: movk [[REG]], #:prel_g3:global+4294967296
+  ; CHECK-GLOBALISEL: add [[REG]], [[REG]], :lo12:global
+  ; CHECK-GLOBALISEL: str wzr, {{\[}}[[REG]]{{\]}}
+  ; CHECK-GLOBALISEL: ret
+
+  ; CHECK-PIC: global_store:
+  ; CHECK-PIC: adrp [[REG:x[0-9]+]], :got:global
+  ; CHECK-PIC: ldr  [[REG]], {{\[}}[[REG]], :got_lo12:global]
+  ; CHECK-PIC: str wzr, {{\[}}[[REG]]{{\]}}
+  ; CHECK-PIC: ret
+
+  store i32 0, i32* @global
+  ret void
+}
+
 define void ()* @func_addr() #0 {
-  ; CHECK: func_addr:
-  ; CHECK: adrp x0, func
-  ; CHECK: add x0, x0, :lo12:func
+  ; CHECK-STATIC: func_addr:
+  ; CHECK-STATIC: adrp [[REG:x[0-9]+]], func
+  ; CHECK-STATIC: add x0, [[REG]], :lo12:func
+  ; CHECK-STATIC: ret
+
+  ; CHECK-PIC: func_addr:
+  ; CHECK-PIC: adrp [[REG:x[0-9]+]], :got:func
+  ; CHECK-PIC: ldr  x0, {{\[}}[[REG]], :got_lo12:func]
+  ; CHECK-PIC: ret
+
   ret void ()* @func
 }