samsonov added a comment.
Sorry for delay.
Comment at: lib/CodeGen/CGExpr.cpp:2543
@@ +2542,3 @@
+ // RE: Bug: 25682
+ if(!CGM.getCodeGenOpts().MergeTraps ||
!CGM.getCodeGenOpts().OptimizationLevel || !TrapBB) {
+// If we aren't merging traps, set the function to not be o
danielaustin updated this revision to Diff 43459.
danielaustin added a comment.
Test case added which looks for the existence of multiple trap basic blocks in
generated LLVM-IR with optimization turned on. Flag renamed
(-fmerge-traps/-fno-merge-traps) and placed in a more sensible location
(Cod
samsonov added inline comments.
Comment at: lib/CodeGen/CGExpr.cpp:2543
@@ +2542,3 @@
+ // RE: Bug: 25682
+ if(!getLangOpts().mergeTraps) {
+ llvm::InlineAsm *EmptyAsm =
llvm::InlineAsm::get(llvm::FunctionType::get(CGM.VoidTy, false),
danielaustin wrote:
danielaustin added a comment.
Test makes sense, will add it with the revisions.
Comment at: include/clang/Basic/LangOptions.h:95
@@ -94,1 +94,3 @@
+ /// \brief Flag controlling whether or not trap calls are merged
+ /// at the end of each function.
samsonov
samsonov added a comment.
Please consider adding a test case.
Comment at: include/clang/Basic/LangOptions.h:95
@@ -94,1 +94,3 @@
+ /// \brief Flag controlling whether or not trap calls are merged
+ /// at the end of each function.
Why is it a language, not c
danielaustin updated this revision to Diff 42328.
danielaustin added a comment.
Patch that removes the debug-mode sanitizer setting and makes the
-fsanitize-merge-traps/-fno-sanitize-merge-traps flags, with the default
setting being to merge traps. Flags tested within the Android build system,
I don't think super-fine grained control would be required, a per-binary
setting should be sufficient for debugging purposes.
If everyone is ok with it, I'll get a diff up later tonight switching the
flag to fsanitize-merge-traps/fnosanitize-merge-traps
On Tue, Dec 8, 2015 at 1:17 PM Evgeniy Step
danielaustin added a comment.
I don't think super-fine grained control would be required, a per-binary
setting should be sufficient for debugging purposes.
If everyone is ok with it, I'll get a diff up later tonight switching the
flag to fsanitize-merge-traps/fnosanitize-merge-traps
Repository:
eugenis added a comment.
I misunderstood the meaning of -fsanitize-trap, and now I prefer
-fsanitize-merge-traps for the flag name.
Repository:
rL LLVM
http://reviews.llvm.org/D15208
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http:/
samsonov added a comment.
That is, I presume it's highly unlikely user will need to have fine-grained
setup for deciding which UBSan checks should be merged, and which should not.
So, probably having a single `-fsanitize-merge-traps` /
`-fno-sanitize-merge-traps` option would be enough. I'd li
danielaustin added a comment.
I'm not partial to what its called, or where it is. I'm completely ok with
-fsanitize-merge-checks
Repository:
rL LLVM
http://reviews.llvm.org/D15208
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lis
I'm not partial to what its called, or where it is. I'm completely ok with
-fsanitize-merge-checks
On Tue, Dec 8, 2015 at 1:07 PM Evgeniy Stepanov wrote:
> eugenis added a comment.
>
> Better -fsanitize-merge-checks, and it should apply to non-trap checks as
> well (i.e. SIGILL address should un
eugenis added a comment.
Better -fsanitize-merge-checks, and it should apply to non-trap checks as well
(i.e. SIGILL address should uniquely correspond to the failure source location).
Repository:
rL LLVM
http://reviews.llvm.org/D15208
___
cfe-c
samsonov added a comment.
That is, I presume it's highly unlikely user will need to have fine-grained
setup for deciding which UBSan checks should be merged, and which should not.
So, probably having a single `-fsanitize-merge-traps` /
`-fno-sanitize-merge-traps` option would be enough. I'd li
samsonov added a comment.
Sorry for the late response. Ugh, flag naming is hard, and it's far too
complicated for sanitizers already.
However, I'm opposed to passing this down as `-fsanitize=` option =/. So far
we're trying to make values of `-fsanitize=` correspond *only* to different
kinds o
danielaustin updated this revision to Diff 41957.
danielaustin added a comment.
Added a flag controlling whether or not the inlining occurs. Tested with ARM 32
bit (shamu) with AOSP. The patched compiler resulted in correct code generation
and no stability issues. The flag is probably not ideall
Makes sense, will add that as well.
On Thu, Dec 3, 2015 at 4:10 PM Alexey Samsonov wrote:
> samsonov added a comment.
>
> I agree with Richard here.
>
>
> Repository:
> rL LLVM
>
> http://reviews.llvm.org/D15208
>
>
>
>
___
cfe-commits mailing list
c
samsonov added a comment.
I agree with Richard here.
Repository:
rL LLVM
http://reviews.llvm.org/D15208
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
rsmith added a comment.
I would expect that people who are using this for hardening would be upset
about a 5% binary size increase. I'm OK with having this behind a flag, though.
Repository:
rL LLVM
http://reviews.llvm.org/D15208
___
cfe-commits
19 matches
Mail list logo
