[LTO] A static relocation model can override the PIC level wrt treating external address as directly accessible

[wolfy1961]

As described in issue #64999, commit e018cbf7208 caused the symbol __stack_chk_guard to not become dso_local when PIC is enabled in a module. However, during LTO we can force a static relocation model, which overrides the PIC level. In this case __stack_chk_guard should become dso_local.
For this purpose we're adding a boolean to the interface of getDirectAccessExternalData() to indicate the relocation model.

Fixes #64999

[llvmbot]

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-llvm-ir

Changes

As described in issue #64999, commit e018cbf7208 caused the symbol __stack_chk_guard to not become dso_local when PIC is enabled in a module. However, during LTO we can force a static relocation model, which overrides the PIC level. In this case __stack_chk_guard should become dso_local.
For this purpose we're adding a boolean to the interface of getDirectAccessExternalData() to indicate the relocation model.

Fixes #64999

---

Full diff: https://github.com/llvm/llvm-project/pull/65512.diff

6 Files Affected:

• (modified) clang/lib/CodeGen/CodeGenModule.cpp (+3-2)
• (modified) llvm/include/llvm/IR/Module.h (+1-1)
• (modified) llvm/lib/CodeGen/TargetLoweringBase.cpp (+2-1)
• (modified) llvm/lib/IR/Module.cpp (+2-2)
• (modified) llvm/lib/Target/X86/X86ISelLoweringCall.cpp (+2-1)
• (modified) llvm/test/LTO/ARM/ssp-static-reloc.ll (+3-5)

diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index 8b0c9340775cbe9..c8756d0b5f56a57 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -1127,7 +1127,8 @@ void CodeGenModule::Release() {
   if (LangOpts.HLSL)
     getHLSLRuntime().finishCodeGen();
 
-  if (uint32_t PLevel = Context.getLangOpts().PICLevel) {
+  uint32_t PLevel = Context.getLangOpts().PICLevel;
+  if (PLevel) {
     assert(PLevel < 3 && "Invalid PIC Level");
     getModule().setPICLevel(static_cast<llvm::PICLevel::Level>(PLevel));
     if (Context.getLangOpts().PIE)
@@ -1152,7 +1153,7 @@ void CodeGenModule::Release() {
     getModule().setRtLibUseGOT();
   if (getTriple().isOSBinFormatELF() &&
       CodeGenOpts.DirectAccessExternalData !=
-          getModule().getDirectAccessExternalData()) {
+          getModule().getDirectAccessExternalData(PLevel == 0)) {
     getModule().setDirectAccessExternalData(
         CodeGenOpts.DirectAccessExternalData);
   }
diff --git a/llvm/include/llvm/IR/Module.h b/llvm/include/llvm/IR/Module.h
index 70beddddc1c1615..cade1fccd4c5c17 100644
--- a/llvm/include/llvm/IR/Module.h
+++ b/llvm/include/llvm/IR/Module.h
@@ -958,7 +958,7 @@ class LLVM_EXTERNAL_VISIBILITY Module {
 
   /// Get/set whether referencing global variables can use direct access
   /// relocations on ELF targets.
-  bool getDirectAccessExternalData() const;
+  bool getDirectAccessExternalData(bool IsStaticRelocModel) const;
   void setDirectAccessExternalData(bool Value);
 
   /// Get/set whether synthesized functions should get the uwtable attribute.
diff --git a/llvm/lib/CodeGen/TargetLoweringBase.cpp b/llvm/lib/CodeGen/TargetLoweringBase.cpp
index 3e4bff5ddce1264..e1f439fd32dad1a 100644
--- a/llvm/lib/CodeGen/TargetLoweringBase.cpp
+++ b/llvm/lib/CodeGen/TargetLoweringBase.cpp
@@ -2016,7 +2016,8 @@ void TargetLoweringBase::insertSSPDeclarations(Module &M) const {
                                   "__stack_chk_guard");
 
     // FreeBSD has "__stack_chk_guard" defined externally on libc.so
-    if (M.getDirectAccessExternalData() &&
+    if (M.getDirectAccessExternalData(TM.getRelocationModel() ==
+                                      Reloc::Static) &&
         !TM.getTargetTriple().isWindowsGNUEnvironment() &&
         !TM.getTargetTriple().isOSFreeBSD() &&
         !TM.getTargetTriple().isOSDarwin())
diff --git a/llvm/lib/IR/Module.cpp b/llvm/lib/IR/Module.cpp
index dba660bbe5bafd3..db90bc0c9cae44b 100644
--- a/llvm/lib/IR/Module.cpp
+++ b/llvm/lib/IR/Module.cpp
@@ -687,12 +687,12 @@ void Module::setRtLibUseGOT() {
   addModuleFlag(ModFlagBehavior::Max, "RtLibUseGOT", 1);
 }
 
-bool Module::getDirectAccessExternalData() const {
+bool Module::getDirectAccessExternalData(bool IsStaticRelocModel) const {
   auto *Val = cast_or_null<ConstantAsMetadata>(
       getModuleFlag("direct-access-external-data"));
   if (Val)
     return cast<ConstantInt>(Val->getValue())->getZExtValue() > 0;
-  return getPICLevel() == PICLevel::NotPIC;
+  return getPICLevel() == PICLevel::NotPIC || IsStaticRelocModel;
 }
 
 void Module::setDirectAccessExternalData(bool Value) {
diff --git a/llvm/lib/Target/X86/X86ISelLoweringCall.cpp b/llvm/lib/Target/X86/X86ISelLoweringCall.cpp
index 754d2042105e57d..9eeb9bf682b1f41 100644
--- a/llvm/lib/Target/X86/X86ISelLoweringCall.cpp
+++ b/llvm/lib/Target/X86/X86ISelLoweringCall.cpp
@@ -606,7 +606,8 @@ Value *X86TargetLowering::getIRStackGuard(IRBuilderBase &IRB) const {
                                 nullptr, GuardSymb, nullptr,
                                 GlobalValue::NotThreadLocal, AddressSpace);
         if (!Subtarget.isTargetDarwin())
-          GV->setDSOLocal(M->getDirectAccessExternalData());
+          GV->setDSOLocal(M->getDirectAccessExternalData(
+              getTargetMachine().getRelocationModel() == Reloc::Static));
       }
       return GV;
     }
diff --git a/llvm/test/LTO/ARM/ssp-static-reloc.ll b/llvm/test/LTO/ARM/ssp-static-reloc.ll
index c8825c2aae0fbb6..bee3639e19d8222 100644
--- a/llvm/test/LTO/ARM/ssp-static-reloc.ll
+++ b/llvm/test/LTO/ARM/ssp-static-reloc.ll
@@ -2,9 +2,8 @@
 ; RUN: llvm-lto -O0 -relocation-model=static -o %t.o %t.bc
 ; RUN: llvm-objdump -d -r %t.o | FileCheck %s
 
-; Confirm that we do generate one too many indirections accessing the stack guard
-; variable, when the relocation model is static and the PIC level is not 0..
-; This is preparation for the fix.
+; Confirm that we do not generate one too many indirections accessing the stack guard
+; variable, when the relocation model is static and the PIC level is not 0.
 ;
 target triple = "armv4t-unknown-unknown"
 
@@ -20,8 +19,7 @@ entry:
 ; CHECK:      <foo>:
 ; CHECK:      [[#%x,CURPC:]]:{{.*}} ldr r[[REG1:[0-9]+]], [pc, #0x[[#%x,OFFSET:]]]
 ; CHECK-NEXT: ldr r[[REG2:[0-9]+]], [r[[REG1]]]
-; CHECK-NEXT: ldr r[[REG3:[0-9]+]], [r[[REG2]]]
-; CHECK-NEXT: str r[[REG3]],
+; CHECK-NEXT: str r[[REG2]],
 ; CHECK:      [[#CURPC + OFFSET + 8]]:{{.*}}.word
 ; CHECK-NEXT: R_ARM_ABS32 __stack_chk_guard
 

[wolfy1961]

Ping ...

[wolfy1961]

Ping ...

[wolfy1961]

Ping...
Any takers?

[teresajohnson]

@MaskRay @nickdesaulniers since they authored and reviewed e018cbf7208, respectively

[wolfy1961]

Superseded by PR 70014.