[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