[PATCH] D91816: [Inline] prevent inlining on stack protector mismatch
Fangrui Song via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Nov 25 14:39:45 PST 2020
MaskRay added inline comments.
================
Comment at: llvm/lib/IR/Attributes.cpp:1942
static void adjustCallerSSPLevel(Function &Caller, const Function &Callee) {
+ assert(!(!Callee.hasStackProtectorFnAttr() &&
+ Caller.hasStackProtectorFnAttr() &&
----------------
Hmm. Perhaps `if (!Callee.hasFnAttribute(Attribute::AlwaysInline)) { assert(); assert(); }` can simplify the conditions a bit.
================
Comment at: llvm/test/CodeGen/X86/stack-protector-2.ll:165
+; Test that the same function body does not get a canary if no ssp fn attrs are
+; set.
----------------
Does the comment apply to bar_sspstrong? I don't get the meaning...
================
Comment at: llvm/test/ThinLTO/X86/nossp.ll:2
+; RUN: opt -module-summary %s -o %t1.bc
+; RUN: opt -module-summary %p/Inputs/nossp.ll -o %t2.bc
+; RUN: llvm-lto2 run %t1.bc %t2.bc -o %t3.bc -save-temps \
----------------
nickdesaulniers wrote:
> rnk wrote:
> > I have recently discovered the new `split-file` utility, and I like it more than adding files to the Inputs directory. Think it's worth giving it a try here? Up to you. It was added mainly for linker test cases, where you want to test linking multiple inputs together.
> @MaskRay suggested that in a previous version of this patch as well. I will look into it. Is the idea that I would concat `llvm/test/ThinLTO/X86/Inputs/nossp.ll` to this, then use `split-file` to split it off?
Right. https://llvm.org/docs/TestingGuide.html#extra-files Inlining the extra file makes it slightly more readable.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D91816/new/
https://reviews.llvm.org/D91816
More information about the llvm-commits
mailing list