subject:"\[PATCH\] D87956\: \[IR\] add fn attr for no_stack_protector; prevent inlining on mismatch"

[PATCH] D87956: [IR] add fn attr for no_stack_protector; prevent inlining on mismatch

2020-11-19 Thread Nick Desaulniers via Phabricator via cfe-commits

nickdesaulniers added a comment.

Simpler approach: https://reviews.llvm.org/D91816


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87956

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87956: [IR] add fn attr for no_stack_protector; prevent inlining on mismatch

2020-11-17 Thread Nick Desaulniers via Phabricator via cfe-commits

nickdesaulniers added inline comments.



Comment at: llvm/test/Transforms/Inline/inline_nossp.ll:3
+; RUN: opt -passes='cgscc(inline)' %s -S | FileCheck %s
+; RUN: opt -always-inline -o - -S %s | FileCheck %s
+

nickdesaulniers wrote:
> aeubanks wrote:
> > nickdesaulniers wrote:
> > > aeubanks wrote:
> > > > This test fails with the NPM,
> > > > `opt -passes=always-inline ...`
> > > > 
> > > > Does `llvm::isInlineViable()` need to be updated?
> > > Thanks for the report.  Is there a Cmake configuration I need to 
> > > explicitly set to reproduce? `LLVM_USE_NEWPM`?
> > Just a new RUN line: `RUN: opt -passes=always-inline -o - -S %s | FileCheck 
> > %s`
> I think I'm going to back out this patch and the fix ups, and pursue a much 
> simpler approach: 
> https://github.com/ClangBuiltLinux/llvm-project/commit/501847c5239be52bbe32a9c5bd0723e4d0ad2990.
reverted in f4c6080ab820219c5bf78b0c2143e7fa194da296


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87956

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87956: [IR] add fn attr for no_stack_protector; prevent inlining on mismatch

2020-11-17 Thread Nick Desaulniers via Phabricator via cfe-commits

nickdesaulniers added inline comments.



Comment at: llvm/test/Transforms/Inline/inline_nossp.ll:3
+; RUN: opt -passes='cgscc(inline)' %s -S | FileCheck %s
+; RUN: opt -always-inline -o - -S %s | FileCheck %s
+

aeubanks wrote:
> nickdesaulniers wrote:
> > aeubanks wrote:
> > > This test fails with the NPM,
> > > `opt -passes=always-inline ...`
> > > 
> > > Does `llvm::isInlineViable()` need to be updated?
> > Thanks for the report.  Is there a Cmake configuration I need to explicitly 
> > set to reproduce? `LLVM_USE_NEWPM`?
> Just a new RUN line: `RUN: opt -passes=always-inline -o - -S %s | FileCheck 
> %s`
I think I'm going to back out this patch and the fix ups, and pursue a much 
simpler approach: 
https://github.com/ClangBuiltLinux/llvm-project/commit/501847c5239be52bbe32a9c5bd0723e4d0ad2990.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87956

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87956: [IR] add fn attr for no_stack_protector; prevent inlining on mismatch

2020-11-16 Thread Arthur Eubanks via Phabricator via cfe-commits

aeubanks added inline comments.



Comment at: llvm/test/Transforms/Inline/inline_nossp.ll:3
+; RUN: opt -passes='cgscc(inline)' %s -S | FileCheck %s
+; RUN: opt -always-inline -o - -S %s | FileCheck %s
+

nickdesaulniers wrote:
> aeubanks wrote:
> > This test fails with the NPM,
> > `opt -passes=always-inline ...`
> > 
> > Does `llvm::isInlineViable()` need to be updated?
> Thanks for the report.  Is there a Cmake configuration I need to explicitly 
> set to reproduce? `LLVM_USE_NEWPM`?
Just a new RUN line: `RUN: opt -passes=always-inline -o - -S %s | FileCheck %s`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87956

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87956: [IR] add fn attr for no_stack_protector; prevent inlining on mismatch

2020-11-16 Thread Nick Desaulniers via Phabricator via cfe-commits

nickdesaulniers added inline comments.



Comment at: llvm/test/Transforms/Inline/inline_nossp.ll:3
+; RUN: opt -passes='cgscc(inline)' %s -S | FileCheck %s
+; RUN: opt -always-inline -o - -S %s | FileCheck %s
+

aeubanks wrote:
> This test fails with the NPM,
> `opt -passes=always-inline ...`
> 
> Does `llvm::isInlineViable()` need to be updated?
Thanks for the report.  Is there a Cmake configuration I need to explicitly set 
to reproduce? `LLVM_USE_NEWPM`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87956

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87956: [IR] add fn attr for no_stack_protector; prevent inlining on mismatch

2020-11-14 Thread Arthur Eubanks via Phabricator via cfe-commits

aeubanks added inline comments.



Comment at: llvm/test/Transforms/Inline/inline_nossp.ll:3
+; RUN: opt -passes='cgscc(inline)' %s -S | FileCheck %s
+; RUN: opt -always-inline -o - -S %s | FileCheck %s
+

This test fails with the NPM,
`opt -passes=always-inline ...`

Does `llvm::isInlineViable()` need to be updated?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87956

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87956: [IR] add fn attr for no_stack_protector; prevent inlining on mismatch

2020-11-04 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay added inline comments.
Herald added a subscriber: pengfei.



Comment at: 
clang/test/Frontend/optimization-remark-missed-inline-stack-protectors.c:1
+// RUN: %clang_cc1 -stack-protector 2 -Rpass-missed=inline -O2 -verify %s 
-emit-llvm-only
+

Emm. I am a bit unsure that we need the clang/test/Frontend test. The inlining 
decision logic is coded into Utils/InlineFunction.cpp and tested by 
Transforms/Inline/inline_nossp.ll

clang is at a higher layer and does not need to understand the inlining 
decision.



Comment at: llvm/docs/LangRef.rst:1827
 
+``nossp``
+This attribute indicates the function should not emit a stack smashing

Would this be confusing now that both nossp and ssp exist? 

Is there an alternative design which can make this extensible? @jdoerfert 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87956

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87956: [IR] add fn attr for no_stack_protector; prevent inlining on mismatch

2020-10-23 Thread Nick Desaulniers via Phabricator via cfe-commits

nickdesaulniers added a comment.

I missed adding to llvm/lib/Bitcode/Reader/BitcodeReader.cpp, so LTO is broken 
with this fn attribute...todo...fix

  diff --git a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp 
b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
  index f8069b93103f..a99d6baa8d9d 100644
  --- a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
  +++ b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
  @@ -1537,6 +1537,8 @@ static Attribute::AttrKind getAttrFromCode(uint64_t 
Code) {
   return Attribute::ByRef;
 case bitc::ATTR_KIND_MUSTPROGRESS:
   return Attribute::MustProgress;
  +  case bitc::ATTR_KIND_NO_STACK_PROTECT:
  +return Attribute::NoStackProtect;
 }
   }


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87956

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87956: [IR] add fn attr for no_stack_protector; prevent inlining on mismatch

2020-10-23 Thread Nick Desaulniers via Phabricator via cfe-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb7926ce6d7a8: [IR] add fn attr for no_stack_protector; 
prevent inlining on mismatch (authored by nickdesaulniers).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87956

Files:
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/stack-protector.c
  clang/test/Frontend/optimization-remark-missed-inline-stack-protectors.c
  llvm/bindings/go/llvm/ir_test.go
  llvm/bindings/ocaml/llvm/llvm.ml
  llvm/docs/BitCodeFormat.rst
  llvm/docs/LangRef.rst
  llvm/include/llvm/Bitcode/LLVMBitCodes.h
  llvm/include/llvm/IR/Attributes.td
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/AsmParser/LLToken.h
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/CodeGen/StackProtector.cpp
  llvm/lib/IR/Attributes.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/lib/Transforms/IPO/ForceFunctionAttrs.cpp
  llvm/lib/Transforms/Utils/CodeExtractor.cpp
  llvm/lib/Transforms/Utils/InlineFunction.cpp
  llvm/test/CodeGen/X86/stack-protector-2.ll
  llvm/test/Transforms/CodeExtractor/PartialInlineAttributes.ll
  llvm/test/Transforms/Inline/inline_nossp.ll
  llvm/test/Transforms/Inline/inline_ssp.ll
  llvm/test/Verifier/function-attribute-nossp-ssp-sspreq-sspstrong.ll
  llvm/utils/emacs/llvm-mode.el
  llvm/utils/kate/llvm.xml
  llvm/utils/llvm.grm
  llvm/utils/vim/syntax/llvm.vim
  llvm/utils/vscode/llvm/syntaxes/ll.tmLanguage.yaml

Index: llvm/utils/vscode/llvm/syntaxes/ll.tmLanguage.yaml
===
--- llvm/utils/vscode/llvm/syntaxes/ll.tmLanguage.yaml
+++ llvm/utils/vscode/llvm/syntaxes/ll.tmLanguage.yaml
@@ -245,6 +245,7 @@
 \\bspir_func\\b|\
 \\bspir_kernel\\b|\
 \\bsret\\b|\
+\\bnossp\\b|\
 \\bssp\\b|\
 \\bsspreq\\b|\
 \\bsspstrong\\b|\
Index: llvm/utils/vim/syntax/llvm.vim
===
--- llvm/utils/vim/syntax/llvm.vim
+++ llvm/utils/vim/syntax/llvm.vim
@@ -148,6 +148,7 @@
   \ spir_func
   \ spir_kernel
   \ sret
+  \ nossp
   \ ssp
   \ sspreq
   \ sspstrong
Index: llvm/utils/llvm.grm
===
--- llvm/utils/llvm.grm
+++ llvm/utils/llvm.grm
@@ -168,6 +168,7 @@
  | noinline
  | alwaysinline
  | optsize
+ | nossp
  | ssp
  | sspreq
  | returns_twice
Index: llvm/utils/kate/llvm.xml
===
--- llvm/utils/kate/llvm.xml
+++ llvm/utils/kate/llvm.xml
@@ -93,6 +93,7 @@
optsize 
readnone 
readonly 
+   nossp 
ssp 
sspreq 
sspstrong 
Index: llvm/utils/emacs/llvm-mode.el
===
--- llvm/utils/emacs/llvm-mode.el
+++ llvm/utils/emacs/llvm-mode.el
@@ -26,7 +26,7 @@
  "inaccessiblemem_or_argmemonly" "inlinehint" "jumptable" "minsize" "mustprogress" "naked" "nobuiltin"
  "noduplicate" "nofree" "noimplicitfloat" "noinline" "nonlazybind" "noredzone" "noreturn"
  "norecurse" "noundef" "nounwind" "optnone" "optsize" "readnone" "readonly" "returns_twice"
- "speculatable" "ssp" "sspreq" "sspstrong" "safestack" "sanitize_address" "sanitize_hwaddress" "sanitize_memtag"
+ "speculatable" "nossp" "ssp" "sspreq" "sspstrong" "safestack" "sanitize_address" "sanitize_hwaddress" "sanitize_memtag"
  "sanitize_thread" "sanitize_memory" "strictfp" "uwtable" "willreturn" "writeonly" "immarg") 'symbols) . font-lock-constant-face)
;; Variables
'("%[-a-zA-Z$._][-a-zA-Z$._0-9]*" . font-lock-variable-name-face)
Index: llvm/test/Verifier/function-attribute-nossp-ssp-sspreq-sspstrong.ll
===
--- /dev/null
+++ llvm/test/Verifier/function-attribute-nossp-ssp-sspreq-sspstrong.ll
@@ -0,0 +1,58 @@
+; RUN: not opt -verify -o - %s 2>&1 | FileCheck %s
+define void @test_1 () #1 { ret void }
+define void @test_2 () #2 { ret void }
+define void @test_3 () #3 { ret void }
+define void @test_4 () #4 { ret void }
+define void @test_5 () #5 { ret void }
+define void @test_6 () #6 { ret void }
+define void @test_7 () #7 { ret void }
+define void @test_8 () #8 { ret void }
+define void @test_9 () #9 { ret void }
+define void @test_10 () #10 { ret void }
+define void @test_11 () #10 { ret void }
+define void @test_12 () #10 { ret void }
+define void @test_13 () #10 { ret void }
+define void @test_14 () #10 { ret void }
+
+attributes #0 = { nossp }
+attributes #1 = { ssp }
+attributes #2 = { sspreq }
+attributes #3 = { sspstrong }
+
+attributes #4 = { nossp ssp }
+attributes #5 = { nossp sspreq }
+attributes #6 = { nossp

[PATCH] D87956: [IR] add fn attr for no_stack_protector; prevent inlining on mismatch

2020-10-22 Thread Nick Desaulniers via Phabricator via cfe-commits

nickdesaulniers added a comment.

As a follow up, I probably want to make `-fno-stack-protector` sprinkle this 
new fn attr `nossp` all over everything; that and this should fix our LTO bug 
with no changes to the source, otherwise we can add 
`__attribute__((no_stack_protector))` to the source (though GCC only landed 
support for that TODAY, so portability would be an issue there)..


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87956

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87956: [IR] add fn attr for no_stack_protector; prevent inlining on mismatch

2020-10-22 Thread Nick Desaulniers via Phabricator via cfe-commits

nickdesaulniers added a comment.

In D87956#2348446 , @void wrote:

> Why not just use "expected-error" and the like?

Ah, brilliant! Thank you!  Will wait to submit until tomorrow morning.




Comment at: llvm/docs/BitCodeFormat.rst:1072
+* code 69: ``byref``
+* code 70: ``mustprogress``
+* code 71: ``nossp``

void wrote:
> I'm tempted to say that these two adds should be a separate commit, but it's 
> a nit and I'll leave it up to you.
Done in rG4abaf0ec0a3c


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87956

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87956: [IR] add fn attr for no_stack_protector; prevent inlining on mismatch

2020-10-22 Thread Nick Desaulniers via Phabricator via cfe-commits

nickdesaulniers updated this revision to Diff 300124.
nickdesaulniers marked 3 inline comments as done.
nickdesaulniers added a comment.
This revision is now accepted and ready to land.

- rebase, add alwaysinline tests, use expected-remark, improve remarks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87956

Files:
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/stack-protector.c
  clang/test/Frontend/optimization-remark-missed-inline-stack-protectors.c
  llvm/bindings/go/llvm/ir_test.go
  llvm/bindings/ocaml/llvm/llvm.ml
  llvm/docs/BitCodeFormat.rst
  llvm/docs/LangRef.rst
  llvm/include/llvm/Bitcode/LLVMBitCodes.h
  llvm/include/llvm/IR/Attributes.td
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/AsmParser/LLToken.h
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/CodeGen/StackProtector.cpp
  llvm/lib/IR/Attributes.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/lib/Transforms/IPO/ForceFunctionAttrs.cpp
  llvm/lib/Transforms/Utils/CodeExtractor.cpp
  llvm/lib/Transforms/Utils/InlineFunction.cpp
  llvm/test/CodeGen/X86/stack-protector-2.ll
  llvm/test/Transforms/CodeExtractor/PartialInlineAttributes.ll
  llvm/test/Transforms/Inline/inline_nossp.ll
  llvm/test/Transforms/Inline/inline_ssp.ll
  llvm/test/Verifier/function-attribute-nossp-ssp-sspreq-sspstrong.ll
  llvm/utils/emacs/llvm-mode.el
  llvm/utils/kate/llvm.xml
  llvm/utils/llvm.grm
  llvm/utils/vim/syntax/llvm.vim
  llvm/utils/vscode/llvm/syntaxes/ll.tmLanguage.yaml

Index: llvm/utils/vscode/llvm/syntaxes/ll.tmLanguage.yaml
===
--- llvm/utils/vscode/llvm/syntaxes/ll.tmLanguage.yaml
+++ llvm/utils/vscode/llvm/syntaxes/ll.tmLanguage.yaml
@@ -245,6 +245,7 @@
 \\bspir_func\\b|\
 \\bspir_kernel\\b|\
 \\bsret\\b|\
+\\bnossp\\b|\
 \\bssp\\b|\
 \\bsspreq\\b|\
 \\bsspstrong\\b|\
Index: llvm/utils/vim/syntax/llvm.vim
===
--- llvm/utils/vim/syntax/llvm.vim
+++ llvm/utils/vim/syntax/llvm.vim
@@ -148,6 +148,7 @@
   \ spir_func
   \ spir_kernel
   \ sret
+  \ nossp
   \ ssp
   \ sspreq
   \ sspstrong
Index: llvm/utils/llvm.grm
===
--- llvm/utils/llvm.grm
+++ llvm/utils/llvm.grm
@@ -168,6 +168,7 @@
  | noinline
  | alwaysinline
  | optsize
+ | nossp
  | ssp
  | sspreq
  | returns_twice
Index: llvm/utils/kate/llvm.xml
===
--- llvm/utils/kate/llvm.xml
+++ llvm/utils/kate/llvm.xml
@@ -93,6 +93,7 @@
optsize 
readnone 
readonly 
+   nossp 
ssp 
sspreq 
sspstrong 
Index: llvm/utils/emacs/llvm-mode.el
===
--- llvm/utils/emacs/llvm-mode.el
+++ llvm/utils/emacs/llvm-mode.el
@@ -26,7 +26,7 @@
  "inaccessiblemem_or_argmemonly" "inlinehint" "jumptable" "minsize" "mustprogress" "naked" "nobuiltin"
  "noduplicate" "nofree" "noimplicitfloat" "noinline" "nonlazybind" "noredzone" "noreturn"
  "norecurse" "noundef" "nounwind" "optnone" "optsize" "readnone" "readonly" "returns_twice"
- "speculatable" "ssp" "sspreq" "sspstrong" "safestack" "sanitize_address" "sanitize_hwaddress" "sanitize_memtag"
+ "speculatable" "nossp" "ssp" "sspreq" "sspstrong" "safestack" "sanitize_address" "sanitize_hwaddress" "sanitize_memtag"
  "sanitize_thread" "sanitize_memory" "strictfp" "uwtable" "willreturn" "writeonly" "immarg") 'symbols) . font-lock-constant-face)
;; Variables
'("%[-a-zA-Z$._][-a-zA-Z$._0-9]*" . font-lock-variable-name-face)
Index: llvm/test/Verifier/function-attribute-nossp-ssp-sspreq-sspstrong.ll
===
--- /dev/null
+++ llvm/test/Verifier/function-attribute-nossp-ssp-sspreq-sspstrong.ll
@@ -0,0 +1,58 @@
+; RUN: not opt -verify -o - %s 2>&1 | FileCheck %s
+define void @test_1 () #1 { ret void }
+define void @test_2 () #2 { ret void }
+define void @test_3 () #3 { ret void }
+define void @test_4 () #4 { ret void }
+define void @test_5 () #5 { ret void }
+define void @test_6 () #6 { ret void }
+define void @test_7 () #7 { ret void }
+define void @test_8 () #8 { ret void }
+define void @test_9 () #9 { ret void }
+define void @test_10 () #10 { ret void }
+define void @test_11 () #10 { ret void }
+define void @test_12 () #10 { ret void }
+define void @test_13 () #10 { ret void }
+define void @test_14 () #10 { ret void }
+
+attributes #0 = { nossp }
+attributes #1 = { ssp }
+attributes #2 = { sspreq }
+attributes #3 = { sspstrong }
+
+attributes #4 = { nossp ssp }
+attributes #5 = { nossp sspreq }
+attributes #6 = { nossp sspstrong }
+

[PATCH] D87956: [IR] add fn attr for no_stack_protector; prevent inlining on mismatch

2020-10-22 Thread Bill Wendling via Phabricator via cfe-commits

void added a comment.

In D87956#2348433 , @nickdesaulniers 
wrote:

> In phab here, it looks like my newly added 
> clang/test/Frontend/optimization-remark-missed-inline-stack-protectors.c 
> fails on windows because I redirect stdout to /dev/null.  How does that work 
> for other tests?  I see other tests in that dir write to /dev/null.  Oh, I 
> guess `-o /dev/null` will just create a file called `"/dev/null"` on Windows? 
> So I should do that rather than `1> /dev/null`?

Why not just use "expected-error" and the like?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87956

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87956: [IR] add fn attr for no_stack_protector; prevent inlining on mismatch

2020-10-22 Thread Nick Desaulniers via Phabricator via cfe-commits

nickdesaulniers added a comment.

In phab here, it looks like my newly added 
clang/test/Frontend/optimization-remark-missed-inline-stack-protectors.c fails 
on windows because I redirect stdout to /dev/null.  How does that work for 
other tests?  I see other tests in that dir write to /dev/null.  Oh, I guess 
`-o /dev/null` will just create a file called `"/dev/null"` on Windows? So I 
should do that rather than `1> /dev/null`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87956

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87956: [IR] add fn attr for no_stack_protector; prevent inlining on mismatch

2020-10-22 Thread Nick Desaulniers via Phabricator via cfe-commits

nickdesaulniers added inline comments.



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.

void wrote:
> 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.
Ah this assertion is not quite right; we still would like to permit `nossp` 
callee being inlined into `nossp` caller.  Let me add a test case for that, 
then make the assertion more precise about a **mismatch** between caller and 
callee.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87956

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87956: [IR] add fn attr for no_stack_protector; prevent inlining on mismatch

2020-10-21 Thread Bill Wendling via Phabricator via cfe-commits

void accepted this revision.
void added a comment.
This revision is now accepted and ready to land.

Only a couple of nits, but I think this looks good.




Comment at: llvm/docs/BitCodeFormat.rst:1072
+* code 69: ``byref``
+* code 70: ``mustprogress``
+* code 71: ``nossp``

I'm tempted to say that these two adds should be a separate commit, but it's a 
nit and I'll leave it up to you.



Comment at: llvm/lib/Transforms/Utils/InlineFunction.cpp:1691
+Caller->hasFnAttribute(Attribute::StackProtectReq))
+  return InlineResult::failure("incompatible stack protector");
+

Suggestion: Improve the failure message to be a bit more descriptive. I.e. 
mention that it can't inline a function with stack protection into a function 
without stack protection and vice versa.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87956

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87956: [IR] add fn attr for no_stack_protector; prevent inlining on mismatch

[PATCH] D87956: [IR] add fn attr for no_stack_protector; prevent inlining on mismatch

[PATCH] D87956: [IR] add fn attr for no_stack_protector; prevent inlining on mismatch

[PATCH] D87956: [IR] add fn attr for no_stack_protector; prevent inlining on mismatch

[PATCH] D87956: [IR] add fn attr for no_stack_protector; prevent inlining on mismatch

[PATCH] D87956: [IR] add fn attr for no_stack_protector; prevent inlining on mismatch

[PATCH] D87956: [IR] add fn attr for no_stack_protector; prevent inlining on mismatch

[PATCH] D87956: [IR] add fn attr for no_stack_protector; prevent inlining on mismatch

[PATCH] D87956: [IR] add fn attr for no_stack_protector; prevent inlining on mismatch

[PATCH] D87956: [IR] add fn attr for no_stack_protector; prevent inlining on mismatch

[PATCH] D87956: [IR] add fn attr for no_stack_protector; prevent inlining on mismatch

[PATCH] D87956: [IR] add fn attr for no_stack_protector; prevent inlining on mismatch

[PATCH] D87956: [IR] add fn attr for no_stack_protector; prevent inlining on mismatch

[PATCH] D87956: [IR] add fn attr for no_stack_protector; prevent inlining on mismatch

[PATCH] D87956: [IR] add fn attr for no_stack_protector; prevent inlining on mismatch

[PATCH] D87956: [IR] add fn attr for no_stack_protector; prevent inlining on mismatch

16 matches

Site Navigation

Mail list logo

Footer information