[PATCH] D87956: [WIP][IR] add fn attr for no_stack_protector; prevent inlining ssp into no-ssp

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.


================
Comment at: clang/test/CodeGen/stack-protector.c:39
 // SAFESTACK-NOSSP: attributes #[[A]] = {{.*}} safestack
-// SAFESTACK-NOSSP-NOT: ssp
+// SAFESTACK-NOSSP-NOT: attribute #[[A]] = {{.*}} ssp
 
----------------
should be `attributes` plural.


================
Comment at: llvm/include/llvm/Bitcode/LLVMBitCodes.h:610
+  // TODO: reorder
+  ATTR_KIND_NO_STACK_PROTECT = 70,
   ATTR_KIND_STACK_PROTECT = 26,
----------------
any comments from reviewers before I go and do a tedious reordering of these enum values?


================
Comment at: llvm/lib/IR/Attributes.cpp:1901-1902
+  // caller was explicitly annotated as nossp.
+  if (Caller.hasFnAttribute(Attribute::NoStackProtect))
+    return;
   // If upgrading the SSP attribute, clear out the old SSP Attributes first.
----------------
This should be an anomalous situation due to the added verifier check. Should I make it an assert instead?


================
Comment at: llvm/lib/Transforms/Utils/InlineFunction.cpp:1687
+      return InlineResult::failure(
+          "non-stack-protected caller would inline stack-protected callee");
+
----------------
Is there a better way to emit an OptimizationRemark? Having this feedback in `-Rpass=inline` might be nice.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87956/new/

https://reviews.llvm.org/D87956