[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"
 
+enum UnsafeStackPtrStorageVal { ThreadLocalUSP, SingleThreadUSP };
+
+static cl::opt<UnsafeStackPtrStorageVal> USPStorage("safe-stack-usp-storage",
+    cl::Hidden, cl::init(ThreadLocalUSP),
+    cl::desc("Type of storage for the unsafe stack pointer"),
+    cl::values(clEnumValN(ThreadLocalUSP, "thread-local",
+                          "Thread-local storage"),
+               clEnumValN(SingleThreadUSP, "single-thread",
+                          "Non-thread-local storage"),
+               clEnumValEnd));
+
 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));
 
+  bool UseTLS = USPStorage == ThreadLocalUSP;
+
   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,

test/Transforms/SafeStack/array.ll

 ; RUN: opt -safe-stack -S -mtriple=i386-pc-linux-gnu < %s -o - | FileCheck %s
+; RUN: opt -safe-stack -safe-stack-usp-storage=single-thread -S -mtriple=i386-pc-linux-gnu < %s -o - | FileCheck -check-prefix=SINGLE-THREAD %s
 ; RUN: opt -safe-stack -S -mtriple=x86_64-pc-linux-gnu < %s -o - | FileCheck %s
+; RUN: opt -safe-stack -safe-stack-usp-storage=single-thread -S -mtriple=x86_64-pc-linux-gnu < %s -o - | FileCheck -check-prefix=SINGLE-THREAD %s
 
 ; array [4 x i8]
 ; Requires protector.
 
+; CHECK: @__safestack_unsafe_stack_ptr = external thread_local(initialexec) global i8*
+; SINGLE-THREAD: @__safestack_unsafe_stack_ptr = external global i8*
+
 define void @foo(i8* %a) nounwind uwtable safestack {
 entry:
   ; CHECK: %[[USP:.*]] = load i8*, i8** @__safestack_unsafe_stack_ptr
 
   ; CHECK: %[[USST:.*]] = getelementptr i8, i8* %[[USP]], i32 -16
 
   ; CHECK: store i8* %[[USST]], i8** @__safestack_unsafe_stack_ptr