TargetLowering: Replace android triple check with libcall check

[arsenm]

Instead of directly checking if the target is android, check if
__safestack_pointer_address is available and configure android
to have the call.

[arsenm]

• TargetLowering: Replace android triple check with libcall check #148800 👈 (View in Graphite)
• SafeStack: Check if __safestack_pointer_address is available #147917
• SafeStack: Emit __safestack_pointer_address call through RuntimeLibcalls #147916
• SafeStack: Emit call to __stack_chk_fail through RuntimeLibcalls #147915
• RuntimeLibcalls: Add entries for some exception related functions #147914
• StackProtector: Use RuntimeLibcalls to query libcall names #147913
• RuntimeLibcalls: Add some tests for OpenBSD stack protectors #147888
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[llvmbot]

@llvm/pr-subscribers-llvm-ir
@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-backend-nvptx

Author: Matt Arsenault (arsenm)

Changes

Instead of directly checking if the target is android, check if
__safestack_pointer_address is available and configure android
to have the call.

---

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

2 Files Affected:

• (modified) llvm/include/llvm/IR/RuntimeLibcalls.td (+8-3)
• (modified) llvm/lib/CodeGen/TargetLoweringBase.cpp (+12-21)

diff --git a/llvm/include/llvm/IR/RuntimeLibcalls.td b/llvm/include/llvm/IR/RuntimeLibcalls.td
index 65e1adff42d0d..ee273f20e048f 100644
--- a/llvm/include/llvm/IR/RuntimeLibcalls.td
+++ b/llvm/include/llvm/IR/RuntimeLibcalls.td
@@ -24,7 +24,7 @@ def isNotOSMSVCRT : RuntimeLibcallPredicate<"!TT.isOSMSVCRT()">;
 def isPS : RuntimeLibcallPredicate<"TT.isPS()">;
 def isNotOSWindowsOrIsCygwinMinGW
   : RuntimeLibcallPredicate<"!TT.isOSWindows() || TT.isOSCygMing()">;
-
+def isAndroid : RuntimeLibcallPredicate<"TT.isAndroid()">;
 
 def isGNUEnvironment : RuntimeLibcallPredicate<"TT.isGNUEnvironment()">;
 def darwinHasSinCosStret : RuntimeLibcallPredicate<"darwinHasSinCosStret(TT)">;
@@ -1135,6 +1135,8 @@ defvar LibmHasLdexpF80 = LibcallImpls<(add ldexp_f80), isNotOSWindowsOrIsCygwinM
 defvar LibmHasFrexpF128 = LibcallImpls<(add frexp_f128), isNotOSWindowsOrIsCygwinMinGW>;
 defvar LibmHasLdexpF128 = LibcallImpls<(add ldexp_f128), isNotOSWindowsOrIsCygwinMinGW>;
 
+defvar LibcSafestackPointerAddress =
+    LibcallImpls<(add __safestack_pointer_address), isAndroid>;
 
 //===----------------------------------------------------------------------===//
 // Objective-C Runtime Libcalls
@@ -1211,7 +1213,8 @@ def AArch64SystemLibrary : SystemRuntimeLibrary<
        LibcallImpls<(add Int128RTLibcalls), isAArch64_ILP64>,
        LibcallImpls<(add bzero), isOSDarwin>,
        DarwinExp10, DarwinSinCosStret,
-       LibmHasSinCosF32, LibmHasSinCosF64, LibmHasSinCosF128)
+       LibmHasSinCosF32, LibmHasSinCosF64, LibmHasSinCosF128,
+       LibcSafestackPointerAddress)
 >;
 
 // Prepend a # to every name
@@ -1482,6 +1485,7 @@ def ARMSystemLibrary
            AEABIDivRemCalls,
            DarwinSinCosStret, DarwinExp10,
            LibmHasSinCosF32, LibmHasSinCosF64, LibmHasSinCosF128,
+           LibcSafestackPointerAddress,
 
            // Use divmod compiler-rt calls for iOS 5.0 and later.
            LibcallImpls<(add __divmodsi4, __udivmodsi4),
@@ -2105,7 +2109,8 @@ defvar X86CommonLibcalls =
        // FIXME: MSVCRT doesn't have powi. The f128 case is added as a
        // hack for one test relying on it.
        __powitf2_f128,
-       LibcallImpls<(add MostPowI), isNotOSMSVCRT>
+       LibcallImpls<(add MostPowI), isNotOSMSVCRT>,
+       LibcSafestackPointerAddress
 );
 
 defvar Windows32DivRemMulCalls =
diff --git a/llvm/lib/CodeGen/TargetLoweringBase.cpp b/llvm/lib/CodeGen/TargetLoweringBase.cpp
index d4a34555ed820..85a507fca65d7 100644
--- a/llvm/lib/CodeGen/TargetLoweringBase.cpp
+++ b/llvm/lib/CodeGen/TargetLoweringBase.cpp
@@ -1965,27 +1965,18 @@ TargetLoweringBase::getDefaultSafeStackPointerLocation(IRBuilderBase &IRB,
 
 Value *
 TargetLoweringBase::getSafeStackPointerLocation(IRBuilderBase &IRB) const {
-  // FIXME: Can this triple check be replaced with SAFESTACK_POINTER_ADDRESS
-  // being available?
-  if (!TM.getTargetTriple().isAndroid())
-    return getDefaultSafeStackPointerLocation(IRB, true);
-
-  Module *M = IRB.GetInsertBlock()->getParent()->getParent();
-  auto *PtrTy = PointerType::getUnqual(M->getContext());
-
-  const char *SafestackPointerAddressName =
-      getLibcallName(RTLIB::SAFESTACK_POINTER_ADDRESS);
-  if (!SafestackPointerAddressName) {
-    M->getContext().emitError(
-        "no libcall available for safestack pointer address");
-    return PoisonValue::get(PtrTy);
-  }
-
-  // Android provides a libc function to retrieve the address of the current
-  // thread's unsafe stack pointer.
-  FunctionCallee Fn =
-      M->getOrInsertFunction(SafestackPointerAddressName, PtrTy);
-  return IRB.CreateCall(Fn);
+  if (const char *SafestackPointerAddressName =
+          getLibcallName(RTLIB::SAFESTACK_POINTER_ADDRESS)) {
+    // Android provides a libc function to retrieve the address of the current
+    // thread's unsafe stack pointer.
+    Module *M = IRB.GetInsertBlock()->getParent()->getParent();
+    auto *PtrTy = PointerType::getUnqual(M->getContext());
+    FunctionCallee Fn =
+        M->getOrInsertFunction(SafestackPointerAddressName, PtrTy);
+    return IRB.CreateCall(Fn);
+  }
+
+  return getDefaultSafeStackPointerLocation(IRB, true);
 }
 
 //===----------------------------------------------------------------------===//

[dpaoliello]

Looks like there is a -safestack-use-pointer-address option somewhere that can force this behavior and is now causing the tests to fail.

[dpaoliello]

Would prefer if the "Android" part of this comment was moved next to the isAndroid check, and this comment was left as a vague "Some systems provide...."