[PATCH] D19852: [safestack] Use non-thread-local unsafe stack pointer for Contiki OS

[Peter Collingbourne via llvm-commits]

pcc added inline comments.

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:2025
@@ -2024,1 +2024,3 @@
 Value *X86TargetLowering::getSafeStackPointerLocation(IRBuilder<> &IRB) const {
+  if (Subtarget.isTargetContiki()) {
+    const char *Name = "__safestack_unsafe_stack_ptr";
----------------
mlemay-intel wrote:
> pcc wrote:
> > mlemay-intel wrote:
> > > pcc wrote:
> > > > You might also consider moving the SafeStack::getOrCreateUnsafeStackPtr implementation into a utility function in TargetLoweringBase that takes a bool UseTLS. Then guarantee that getSafeStackPointerLocation always returns non-null and you can remove SafeStack::getOrCreateUnsafeStackPtr altogehter.
> > > That would be a nice way to eliminate this redundancy.  However, the SafeStack pass is written to support TL being null.  How would your suggested approach handle that case?
> > What happens if you `assert(TL);`? Do any tests fail?
> I added `assert(TL)` right after the initialization of `TL` in `runOnFunction` and ran check-llvm and check-clang.  There were no unexpected failures.
In that case, I would suggest that you change the pass to require TL to be available. There's precedent for this in the CodeGen pipeline -- see the StackProtector pass.


https://reviews.llvm.org/D19852