[safestack] Support safestack in stack size diagnostics

Current stack size diagnostics ignore the size of the unsafe stack.
This patch attaches the size of the static portion of the unsafe stack
to the function as metadata, which can be used by the backend to emit
diagnostics regarding stack usage.

Reviewed By: phosek, mcgrathr

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

diff --git a/clang/test/Frontend/stack-usage-safestack.c b/clang/test/Frontend/stack-usage-safestack.c
new file mode 100644 (file)
index 0000000..888d40c
--- /dev/null
+++ b/clang/test/Frontend/stack-usage-safestack.c
@@ -0,0 +1,20 @@
+/// Check that stack frame size warnings behave the same when safe stack is enabled
+
+// RUN: %clang_cc1 %s -fwarn-stack-size=48 -S -o - -triple=i386-apple-darwin 2>&1 | FileCheck --check-prefix=REGULAR %s
+// RUN: %clang_cc1 %s -fwarn-stack-size=1060 -S -o - -triple=i386-apple-darwin 2>&1 | FileCheck --check-prefix=IGNORE %s
+
+// RUN: %clang_cc1 %s -fsanitize=safe-stack -fwarn-stack-size=48 -S -o - -triple=i386-apple-darwin 2>&1 | FileCheck --check-prefix=SAFESTACK %s
+// RUN: %clang_cc1 %s -fsanitize=safe-stack -fwarn-stack-size=1060 -S -o - -triple=i386-apple-darwin 2>&1 | FileCheck --check-prefix=IGNORE %s
+
+extern void init(char *buf, int size);
+extern int use_buf(char *buf, int size);
+
+// REGULAR: warning: stack frame size ([[#]]) exceeds limit ([[#]]) in 'stackSizeWarning'
+// SAFESTACK: warning: stack frame size ([[#]]) exceeds limit ([[#]]) in 'stackSizeWarning'
+// IGNORE-NOT: stack frame size ([[#]]) exceeds limit ([[#]]) in 'stackSizeWarning'
+void stackSizeWarning() {
+  char buf[1024];
+  init(buf, sizeof(buf));
+  if (buf[0] < 42)
+    use_buf(buf, sizeof(buf));
+}

diff --git a/llvm/include/llvm/CodeGen/MachineFrameInfo.h b/llvm/include/llvm/CodeGen/MachineFrameInfo.h
index 2d018aa..dfedf14 100644 (file)
--- a/llvm/include/llvm/CodeGen/MachineFrameInfo.h
+++ b/llvm/include/llvm/CodeGen/MachineFrameInfo.h
@@ -334,6 +334,9 @@ private:
   /// Not null, if shrink-wrapping found a better place for the epilogue.
   MachineBasicBlock *Restore = nullptr;
 
+  /// Size of the UnsafeStack Frame
+  uint64_t UnsafeStackSize = 0;
+
 public:
   explicit MachineFrameInfo(unsigned StackAlignment, bool StackRealignable,
                             bool ForcedRealign)
@@ -787,6 +790,9 @@ public:
   MachineBasicBlock *getRestorePoint() const { return Restore; }
   void setRestorePoint(MachineBasicBlock *NewRestore) { Restore = NewRestore; }
 
+  uint64_t getUnsafeStackSize() const { return UnsafeStackSize; }
+  void setUnsafeStackSize(uint64_t Size) { UnsafeStackSize = Size; }
+
   /// Return a set of physical registers that are pristine.
   ///
   /// Pristine registers hold a value that is useless to the current function,

diff --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
index 01f6334..ea8a86b 100644 (file)
--- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -1354,7 +1354,8 @@ void AsmPrinter::emitStackSizeSection(const MachineFunction &MF) {
   OutStreamer->SwitchSection(StackSizeSection);
 
   const MCSymbol *FunctionSymbol = getFunctionBegin();
-  uint64_t StackSize = FrameInfo.getStackSize();
+  uint64_t StackSize =
+      FrameInfo.getStackSize() + FrameInfo.getUnsafeStackSize();
   OutStreamer->emitSymbolValue(FunctionSymbol, TM.getProgramPointerSize());
   OutStreamer->emitULEB128IntValue(StackSize);
 
@@ -1369,7 +1370,8 @@ void AsmPrinter::emitStackUsage(const MachineFunction &MF) {
     return;
 
   const MachineFrameInfo &FrameInfo = MF.getFrameInfo();
-  uint64_t StackSize = FrameInfo.getStackSize();
+  uint64_t StackSize =
+      FrameInfo.getStackSize() + FrameInfo.getUnsafeStackSize();
 
   if (StackUsageStream == nullptr) {
     std::error_code EC;

diff --git a/llvm/lib/CodeGen/MachineFunction.cpp b/llvm/lib/CodeGen/MachineFunction.cpp
index db91088..06830e8 100644 (file)
--- a/llvm/lib/CodeGen/MachineFunction.cpp
+++ b/llvm/lib/CodeGen/MachineFunction.cpp
@@ -107,6 +107,27 @@ static const char *getPropertyName(MachineFunctionProperties::Property Prop) {
   llvm_unreachable("Invalid machine function property");
 }
 
+void setUnsafeStackSize(const Function &F, MachineFrameInfo &FrameInfo) {
+  if (!F.hasFnAttribute(Attribute::SafeStack))
+    return;
+
+  auto *Existing =
+      dyn_cast_or_null<MDTuple>(F.getMetadata(LLVMContext::MD_annotation));
+
+  if (!Existing || Existing->getNumOperands() != 2)
+    return;
+
+  auto *MetadataName = "unsafe-stack-size";
+  if (auto &N = Existing->getOperand(0)) {
+    if (cast<MDString>(N.get())->getString() == MetadataName) {
+      if (auto &Op = Existing->getOperand(1)) {
+        auto Val = mdconst::extract<ConstantInt>(Op)->getZExtValue();
+        FrameInfo.setUnsafeStackSize(Val);
+      }
+    }
+  }
+}
+
 // Pin the vtable to this file.
 void MachineFunction::Delegate::anchor() {}
 
@@ -175,6 +196,8 @@ void MachineFunction::init() {
       /*ForcedRealign=*/CanRealignSP &&
           F.hasFnAttribute(Attribute::StackAlignment));
 
+  setUnsafeStackSize(F, *FrameInfo);
+
   if (F.hasFnAttribute(Attribute::StackAlignment))
     FrameInfo->ensureMaxAlignment(*F.getFnStackAlign());
 

diff --git a/llvm/lib/CodeGen/PrologEpilogInserter.cpp b/llvm/lib/CodeGen/PrologEpilogInserter.cpp
index ee22cbd..bc45450 100644 (file)
--- a/llvm/lib/CodeGen/PrologEpilogInserter.cpp
+++ b/llvm/lib/CodeGen/PrologEpilogInserter.cpp
@@ -283,6 +283,9 @@ bool PEI::runOnMachineFunction(MachineFunction &MF) {
     assert(!Failed && "Invalid warn-stack-size fn attr value");
     (void)Failed;
   }
+  if (MF.getFunction().hasFnAttribute(Attribute::SafeStack)) {
+    StackSize += MFI.getUnsafeStackSize();
+  }
   if (StackSize > Threshold) {
     DiagnosticInfoStackSize DiagStackSize(F, StackSize, Threshold, DS_Warning);
     F.getContext().diagnose(DiagStackSize);

diff --git a/llvm/lib/CodeGen/SafeStack.cpp b/llvm/lib/CodeGen/SafeStack.cpp
index 1f7eac4..2c16c19 100644 (file)
--- a/llvm/lib/CodeGen/SafeStack.cpp
+++ b/llvm/lib/CodeGen/SafeStack.cpp
@@ -48,6 +48,7 @@
 #include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/MDBuilder.h"
+#include "llvm/IR/Metadata.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IR/Type.h"
 #include "llvm/IR/Use.h"
@@ -633,6 +634,13 @@ Value *SafeStack::moveStaticAllocasToUnsafeStack(
   // FIXME: no need to update BasePointer in leaf functions.
   unsigned FrameSize = alignTo(SSL.getFrameSize(), StackAlignment);
 
+  MDBuilder MDB(F.getContext());
+  SmallVector<Metadata *, 2> Data;
+  Data.push_back(MDB.createString("unsafe-stack-size"));
+  Data.push_back(MDB.createConstant(ConstantInt::get(Int32Ty, FrameSize)));
+  MDNode *MD = MDTuple::get(F.getContext(), Data);
+  F.setMetadata(LLVMContext::MD_annotation, MD);
+
   // Update shadow stack pointer in the function epilogue.
   IRB.SetInsertPoint(BasePointer->getNextNode());
 

diff --git a/llvm/test/CodeGen/X86/warn-stack.ll b/llvm/test/CodeGen/X86/warn-stack.ll
index fc59c6e..2e985f3 100644 (file)
--- a/llvm/test/CodeGen/X86/warn-stack.ll
+++ b/llvm/test/CodeGen/X86/warn-stack.ll
@@ -21,4 +21,17 @@ entry:
   ret void
 }
 
+; Ensure that warn-stack-size also considers the size of the unsafe stack.
+; With safestack enabled the machine stack size is well below 80, but the
+; combined stack size of the machine stack and unsafe stack will exceed the
+; warning threshold
+
+; CHECK: warning: stack frame size (120) exceeds limit (80) in function 'warn_safestack'
+define void @warn_safestack() nounwind ssp safestack "warn-stack-size"="80" {
+entry:
+  %buffer = alloca [80 x i8], align 1
+  %arraydecay = getelementptr inbounds [80 x i8], [80 x i8]* %buffer, i64 0, i64 0
+  call void @doit(i8* %arraydecay) nounwind
+  ret void
+}
 declare void @doit(i8*)

diff --git a/llvm/test/Transforms/SafeStack/ARM/debug.ll b/llvm/test/Transforms/SafeStack/ARM/debug.ll
index af132ae..2bcfeb8 100644 (file)
--- a/llvm/test/Transforms/SafeStack/ARM/debug.ll
+++ b/llvm/test/Transforms/SafeStack/ARM/debug.ll
@@ -10,8 +10,8 @@ target triple = "armv7-pc-linux-android"
 ; void Capture(char*x);
 ; void f() { char c[16]; Capture(c); }
 
-; CHECK: !35 = !DILocation(line: 3, column: 11, scope: !17, inlinedAt: !36)
-; CHECK: !36 = distinct !DILocation(line: 6, scope: !27)
+; CHECK: !36 = !DILocation(line: 3, column: 11, scope: !17, inlinedAt: !37)
+; CHECK: !37 = distinct !DILocation(line: 6, scope: !27)
 
 @addr = common local_unnamed_addr global i8*** null, align 4, !dbg !0