[safestack] Add option for non-TLS unsafe stack pointer

[mlemay-intel]

Summary

This patch adds an option, -safe-stack-no-tls, for using normal
storage instead of thread-local storage for the unsafe stack pointer.
This can be useful when SafeStack is applied to an operating system
kernel.

[eugenis]

lib/Transforms/Instrumentation/SafeStack.cpp

50

Could you invert the meaning of this option? Negative language makes the code less readable. I.e. make the option _enable_ TLS, with "true" as the default value.

[mlemay-intel]

Name the option UseTLS instead of NoTLS to improve the readability of the code that checks it.

[eugenis]

LGTM

[mlemay-intel]

I don't have commit access yet, so can you please commit this for me, @eugenis? Thank you.

[eugenis]

Yes.
But on the second thought, please add a test.

[mlemay-intel]

I added a test. I also revised the code to use an enumerated command line option instead of a simple Boolean, in case additional types of storage are necessary in the future.

[eugenis]

Landed as r256221.

lib/Transforms/Instrumentation/SafeStack.cpp

 #include "llvm/Target/TargetSubtargetInfo.h"
 #include "llvm/Transforms/Utils/Local.h"
 #include "llvm/Transforms/Utils/ModuleUtils.h"
 
 using namespace llvm;
 
 #define DEBUG_TYPE "safestack"
 
+static cl::opt<bool> UseTLS("safe-stack-tls",
+    cl::Hidden, cl::init(true),
+    cl::desc("Use thread-local storage for the unsafe stack pointer"));
+
 namespace llvm {
 
 STATISTIC(NumFunctions, "Total number of functions");
 STATISTIC(NumUnsafeStackFunctions, "Number of functions with unsafe stack");
 STATISTIC(NumUnsafeStackRestorePointsFunctions,
           "Number of functions that use setjmp or exceptions");
 
 STATISTIC(NumAllocas, "Total number of allocas");
Value *SafeStack::getOrCreateUnsafeStackPtr(IRBuilder<> &IRB, Function &F) {
   // Otherwise, assume the target links with compiler-rt, which provides a
   // thread-local variable with a magic name.
   Module &M = *F.getParent();
   const char *UnsafeStackPtrVar = "__safestack_unsafe_stack_ptr";
   auto UnsafeStackPtr =
       dyn_cast_or_null<GlobalVariable>(M.getNamedValue(UnsafeStackPtrVar));
 
   if (!UnsafeStackPtr) {
+    auto TLSModel = UseTLS ?
+        GlobalValue::InitialExecTLSModel :
+        GlobalValue::NotThreadLocal;
     // The global variable is not defined yet, define it ourselves.
     // We use the initial-exec TLS model because we do not support the
     // variable living anywhere other than in the main executable.
     UnsafeStackPtr = new GlobalVariable(
         M, StackPtrTy, false, GlobalValue::ExternalLinkage, nullptr,
-        UnsafeStackPtrVar, nullptr, GlobalValue::InitialExecTLSModel);
+        UnsafeStackPtrVar, nullptr, TLSModel);
   } else {
     // The variable exists, check its type and attributes.
     if (UnsafeStackPtr->getValueType() != StackPtrTy)
       report_fatal_error(Twine(UnsafeStackPtrVar) + " must have void* type");
-    if (!UnsafeStackPtr->isThreadLocal())
-      report_fatal_error(Twine(UnsafeStackPtrVar) + " must be thread-local");
+    if (UseTLS != UnsafeStackPtr->isThreadLocal())
+      report_fatal_error(Twine(UnsafeStackPtrVar) + " must " +
+                         (UseTLS ? "" : "not ") + "be thread-local");
   }
   return UnsafeStackPtr;
 }
 
 void SafeStack::findInsts(Function &F,
                           SmallVectorImpl<AllocaInst *> &StaticAllocas,
                           SmallVectorImpl<AllocaInst *> &DynamicAllocas,
                           SmallVectorImpl<Argument *> &ByValArguments,