[CodeGen] Restore accessing __stack_chk_guard via a .refptr stub on mingw after 2518433f861fcb87

[mstorsjo]

Summary

Add tests for this particular detail for x86 and arm (similar tests already existed for x86_64 and aarch64).

The libssp implementation may be located in a separate DLL, and in those cases, the references need to be in a .refptr stub, to avoid needing to touch up code in the text section at runtime (which is supported but inefficient for x86, and unsupported for arm).

There were half a dozen of commits at least on top of this change (so it's not trivial to revert back to working condition, to add more comprehensive tests for the original state), and none of them seem to have an associated phabricator review thread to comment on, so commenting here instead...

[MaskRay]

Sorry, I did not know Windows has ssp...

The if (getTargetMachine().getRelocationModel() == Reloc::Static) change was added to keep the current ELF behavior in static relocation model (which may cause a copy relocation if the variable is defined in a shared object (libc.so))... Restricting this to Windows looks find

The long-term plan is:

• Add -fdirect-access-external-data to supersede -mpie-copy-relocations D92633
• Make -fdirect-access-external-data work with -fno-pic D92714
• Make -fdirect-access-external-data set a module flag (I don't have a good name yet): control whether default visibility declarations (both variables and functions) should default to dso_local
• Rewrite if (isa<llvm::Function>(GV) && !CGOpts.NoPLT && RM == llvm::Reloc::Static) (https://github.com/llvm/llvm-project/blob/master/clang/lib/CodeGen/CodeGenModule.cpp#L983 ) to use the module flag (the option is called ***data, but I guess it makes sense for dso_local specifiers on function declarations as well since the dso_local specifier controls the behavior of "taking the address of an undefined function" (which is used similar to a data symbol)
• Let Module::getOrInsertGlobal() check the module flag and make variable declarations dso_local

Actually, marking undefined function/variable declarations dso_local in ELF -fno-pic code (allow R_X86_64_64/R_386_32) has a low priority (lots of users have switched to -fpie + users don't care 32-bit (performance) much + GOTPCRELX can retrieve back most of the performance cost on 64-bit code). I just had not fixed an x86-64 FastISel bug 687b83ceabafe81970cd4639e7f0c89036402081 when I thought we should mark some declarations dso_local

[mstorsjo]

Committed in 78a57069b53a08d5aef98a8472fcfa73dbbc8771.

llvm/lib/CodeGen/TargetLoweringBase.cpp

 
 // Currently only support "standard" __stack_chk_guard.
 // TODO: add LOAD_STACK_GUARD support.
 void TargetLoweringBase::insertSSPDeclarations(Module &M) const {
   if (!M.getNamedValue("__stack_chk_guard")) {
     auto *GV = new GlobalVariable(M, Type::getInt8PtrTy(M.getContext()), false,
                                   GlobalVariable::ExternalLinkage, nullptr,
                                   "__stack_chk_guard");
-    if (getTargetMachine().getRelocationModel() == Reloc::Static)
+    if (TM.getRelocationModel() == Reloc::Static &&
+        !TM.getTargetTriple().isWindowsGNUEnvironment())
       GV->setDSOLocal(true);
   }
 }
 
 // Currently only support "standard" __stack_chk_guard.
 // TODO: add LOAD_STACK_GUARD support.
 Value *TargetLoweringBase::getSDagStackGuard(const Module &M) const {
   return M.getNamedValue("__stack_chk_guard");

llvm/test/CodeGen/ARM/win32-ssp.ll

+; RUN: llc -mtriple=thumbv7-w64-mingw32 < %s -o - | FileCheck --check-prefix=MINGW %s
+
+declare void @llvm.lifetime.start.p0i8(i64, i8* nocapture)
+declare dso_local void @other(i8*)
+declare void @llvm.lifetime.end.p0i8(i64, i8* nocapture)
+
+define dso_local void @func() sspstrong {
+entry:
+; MINGW-LABEL: func:
+; MINGW: movw [[REG:r[0-9]+]], :lower16:.refptr.__stack_chk_guard
+; MINGW: movt [[REG]], :upper16:.refptr.__stack_chk_guard
+; MINGW: ldr [[REG2:r[0-9]+]], {{\[}}[[REG]]]
+; MINGW: ldr {{r[0-9]+}}, {{\[}}[[REG2]]]
+; MINGW: bl other
+; MINGW: ldr {{r[0-9]+}}, {{\[}}[[REG2]]]
+; MINGW: bl __stack_chk_fail
+
+  %c = alloca i8, align 1
+  call void @llvm.lifetime.start.p0i8(i64 1, i8* nonnull %c)
+  call void @other(i8* nonnull %c)
+  call void @llvm.lifetime.end.p0i8(i64 1, i8* nonnull %c)
+  ret void
+}

llvm/test/CodeGen/X86/win32-ssp.ll

 ; RUN: llc -mtriple=x86_64-w64-mingw32        < %s -o - | FileCheck --check-prefix=MINGW %s
 ; RUN: llc -mtriple=x86_64-pc-windows-itanium < %s -o - | FileCheck --check-prefix=MSVC  %s
 ; RUN: llc -mtriple=x86_64-pc-windows-msvc    < %s -o - | FileCheck --check-prefix=MSVC  %s
+; RUN: llc -mtriple=i686-w64-mingw32          < %s -o - | FileCheck --check-prefix=MINGW %s
 
 declare void @llvm.lifetime.start.p0i8(i64, i8* nocapture)
 declare dso_local void @other(i8*)
 declare void @llvm.lifetime.end.p0i8(i64, i8* nocapture)
 
 define dso_local void @func() sspstrong {
 entry:
 ; MINGW-LABEL: func:
-; MINGW: mov{{l|q}}  .refptr.__stack_chk_guard(%rip), [[REG:%[a-z]+]]
+; MINGW: mov{{l|q}}  .refptr.[[PREFIX:_?]]__stack_chk_guard{{(\(%rip\))?}}, [[REG:%[a-z]+]]
 ; MINGW: mov{{l|q}}  ([[REG]])
-; MINGW: callq other
+; MINGW: call{{l|q}} [[PREFIX]]other
 ; MINGW: mov{{l|q}}  ([[REG]])
-; MINGW: callq __stack_chk_fail
-; MINGW: .seh_endproc
+; MINGW: call{{l|q}} [[PREFIX]]__stack_chk_fail
 
 ; MSVC-LABEL: func:
 ; MSVC: mov{{l|q}} __security_cookie
 ; MSVC: callq other
 ; MSVC: callq __security_check_cookie
 ; MSVC: .seh_endproc
 
   %c = alloca i8, align 1
   call void @llvm.lifetime.start.p0i8(i64 1, i8* nonnull %c)
   call void @other(i8* nonnull %c)
   call void @llvm.lifetime.end.p0i8(i64 1, i8* nonnull %c)
   ret void
 }