[SSP] [3/3] cmpxchg and addrspacecast instructions can now

trigger stack protectors.  Fixes PR42238.

Add test coverage for llvm.memset, as proxy for all llvm.mem*
intrinsics. There are two issues here: (1) they could be lowered to a
libc call, which could be intercepted, and do Bad Stuff; (2) with a
non-constant size, they could overwrite the current stack frame.

The test was mostly written by Matt Arsenault in r363169, which was
later reverted; I tweaked what he had and added the llvm.memset part.

Differential Revision: https://reviews.llvm.org/D67845

llvm-svn: 373220

1 files changed, 21 insertions, 2 deletions

diff --git a/llvm/lib/CodeGen/StackProtector.cpp b/llvm/lib/CodeGen/StackProtector.cpp
index 7cce1c597ff..5683d1db473 100644
--- a/llvm/lib/CodeGen/StackProtector.cpp
+++ b/llvm/lib/CodeGen/StackProtector.cpp
@@ -164,12 +164,19 @@ bool StackProtector::HasAddressTaken(const Instruction *AI) {
       if (AI == cast<StoreInst>(I)->getValueOperand())
         return true;
       break;
+    case Instruction::AtomicCmpXchg:
+      // cmpxchg conceptually includes both a load and store from the same
+      // location. So, like store, the value being stored is what matters.
+      if (AI == cast<AtomicCmpXchgInst>(I)->getNewValOperand())
+        return true;
+      break;
     case Instruction::PtrToInt:
       if (AI == cast<PtrToIntInst>(I)->getOperand(0))
         return true;
       break;
     case Instruction::Call: {
-      // Ignore intrinsics that are not calls. TODO: Use isLoweredToCall().
+      // Ignore intrinsics that do not become real instructions.
+      // TODO: Narrow this to intrinsics that have store-like effects.
       const auto *CI = cast<CallInst>(I);
       if (!isa<DbgInfoIntrinsic>(CI) && !CI->isLifetimeStartOrEnd())
         return true;
@@ -180,6 +187,7 @@ bool StackProtector::HasAddressTaken(const Instruction *AI) {
     case Instruction::BitCast:
     case Instruction::GetElementPtr:
     case Instruction::Select:
+    case Instruction::AddrSpaceCast:
       if (HasAddressTaken(I))
         return true;
       break;
@@ -192,8 +200,19 @@ bool StackProtector::HasAddressTaken(const Instruction *AI) {
           return true;
       break;
     }
-    default:
+    case Instruction::Load:
+    case Instruction::AtomicRMW:
+    case Instruction::Ret:
+      // These instructions take an address operand, but have load-like or
+      // other innocuous behavior that should not trigger a stack protector.
+      // atomicrmw conceptually has both load and store semantics, but the
+      // value being stored must be integer; so if a pointer is being stored,
+      // we'll catch it in the PtrToInt case above.
       break;
+    default:
+      // Conservatively return true for any instruction that takes an address
+      // operand, but is not handled above.
+      return true;
     }
   }
   return false;