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

Bill Wendling via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Oct 20 20:12:13 PDT 2020


void added inline comments.


================
Comment at: llvm/docs/LangRef.rst:1829
+
+    If a function with the ``nossp`` attribute calls a callee function that has
+    a stack protector function attribute, such as ``ssp``, ``sspreq``, or
----------------
Do we care about the opposite situation where a function requiring a stack protector doesn't inline a function with the `nossp` attribute?


================
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.
----------------
nickdesaulniers wrote:
> This should be an anomalous situation due to the added verifier check. Should I make it an assert instead?
If it should never happen, then please make it an assert.


================
Comment at: llvm/lib/IR/Verifier.cpp:1986
+    Assert(N < 2,
+           "nossp, ssp, sspreq, sspstrong fn attrs should be mutually "
+           "exclusive",
----------------
Could use stronger language here. :-)

"nssp, ssp, sspreq, and sspstrong fn attrs are mutually exclusive."


================
Comment at: llvm/lib/IR/Verifier.cpp:1977-1984
+    if (Attrs.hasFnAttribute(Attribute::NoStackProtect))
+      ++N;
+    if (Attrs.hasFnAttribute(Attribute::StackProtect))
+      ++N;
+    if (Attrs.hasFnAttribute(Attribute::StackProtectReq))
+      ++N;
+    if (Attrs.hasFnAttribute(Attribute::StackProtectStrong))
----------------
nickdesaulniers wrote:
> I'd love to cast these booleans to `unsigned` and add them, but I find that all of the `static_casts` don't really make this more concise.  Thoughts?
I think what you have is fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87956



More information about the cfe-commits mailing list