[ARM] Do not use LOAD_STACK_GUARD with ROPI/RWPI

[pzheng]

Summary

ROPI/RWPI are not supported with LOAD_STACK_GUARD currently.

[nickdesaulniers]

Thanks for the patch! Please consider adding a test case for this.

[pzheng]

Add a test

[pzheng]

Thanks for reviewing the patch, @nickdesaulniers. I have added a test for the change.

[rengolin]

llvm/test/CodeGen/ARM/stack-guard-rwpi.ll

2

Perhaps add a second RUN line with another relocation model showing that you do see the stack guard?

Maybe even a third, for ROPI, just to make sure neither of them are removed by accident in the future.

6

I'm not fully aware how the stack guard works in practice, is this an example of it not lowered?

[pzheng]

llvm/test/CodeGen/ARM/stack-guard-rwpi.ll

2

Makes sense to me, will add two more RUN lines as you suggested.

6

Yes, this is an example of how the stack guard is lowered when LOAD_STACK_GUARD is not used. If LOAD_STACK_GUARD is used for rwpi/ropi (which is what this patch is trying to avoid), there is an assertion coming from lvm/lib/Target/ARM/ARMBaseInstrInfo.cpp.

void llvm::ARMBaseInstrInfo::expandLoadStackGuardBase(MachineBasicBlock::iterator, unsigned int, unsigned int) const: Assertion `!Subtarget.isROPI() && !Subtarget.isRWPI() && "ROPI/RWPI not currently supported with stack guard"' failed.

This patch basically prevents the assertion from happening since we already know ropi/rwpi are not supported with LOAD_STACK_GUARD yet .

[pzheng]

Update test based on review feedback

[rengolin]

llvm/test/CodeGen/ARM/stack-guard-rwpi.ll

6

Ack, makes sense, just checking, thanks!

llvm/lib/Target/ARM/ARMISelLowering.cpp

ARMTargetLowering::shouldExpandAtomicCmpXchgInIR(AtomicCmpXchgInst *AI) const {
   return AtomicExpansionKind::None;
 }
 
 bool ARMTargetLowering::shouldInsertFencesForAtomic(
     const Instruction *I) const {
   return InsertFencesForAtomic;
 }
 
-bool ARMTargetLowering::useLoadStackGuardNode() const { return true; }
+bool ARMTargetLowering::useLoadStackGuardNode() const {
+  // ROPI/RWPI are not supported currently.
+  return !Subtarget->isROPI() && !Subtarget->isRWPI();
+}
 
 void ARMTargetLowering::insertSSPDeclarations(Module &M) const {
   if (!Subtarget->getTargetTriple().isWindowsMSVCEnvironment())
     return TargetLowering::insertSSPDeclarations(M);
 
   // MSVC CRT has a global variable holding security cookie.
   M.getOrInsertGlobal("__security_cookie",
                       Type::getInt8PtrTy(M.getContext()));

llvm/test/CodeGen/ARM/stack-guard-rwpi.ll

+; RUN: llc -mtriple=arm-- --relocation-model=rwpi %s -o - | \
+; RUN: FileCheck %s --check-prefixes=RWPI
+; RUN: llc -mtriple=arm-- --relocation-model=ropi %s -o - | \
+; RUN: FileCheck %s --check-prefixes=ROPI
+; RUN: llc -mtriple=arm-- --relocation-model=pic %s -o - | \
+; RUN: FileCheck %s --check-prefixes=PIC
+
+; RWPI:        ldr     {{r[0-9]+}}, .LCPI0_0
+; RWPI:        .LCPI0_0:
+; RWPI-NEXT:           .long   __stack_chk_guard(sbrel)
+
+; ROPI:        ldr     {{r[0-9]+}}, .LCPI0_0
+; ROPI:        .LCPI0_0:
+; ROPI-NEXT:           .long   __stack_chk_guard
+
+; PIC:         ldr     {{r[0-9]+}}, .LCPI0_0
+; PIC:         .LCPI0_0:
+; PIC-NEXT:    .Ltmp0:
+; PIC-NEXT:            .long   __stack_chk_guard(GOT_PREL)-((.LPC0_0+8)-.Ltmp0)
+
+define dso_local i32 @foo(i32 %t) nounwind sspstrong {
+entry:
+  %vla = alloca i32, i32 %t
+  %call = call i32 @baz(i32* %vla)
+  ret i32 %call
+}
+
+declare dso_local i32 @baz(i32*)