[PATCH] D83360: [InstSimplify] Remove select ?, undef, X -> X and select ?, X, undef -> X

2020-07-08 Thread John Regehr via Phabricator via cfe-commits
regehr added a comment. > Did you mean to check something like the following? > > define i32 @src(i1 %cond, i32 %x) { > %x2 = freeze i32 %x > %s = select i1 %cond, i32 %x2, i32 undef > ret i32 %s > } > > define i32 @tgt(i1 %cond, i32 %x) { > %x2 = freeze i32 %x > ret

[PATCH] D83360: [InstSimplify] Remove select ?, undef, X -> X and select ?, X, undef -> X

2020-07-08 Thread John Regehr via Phabricator via cfe-commits
regehr added a comment. @craig.topper ok, I agree that should work. alive doesn't like it -- is this an alive bug @nlopes? a freeze should not yield undef. https://alive2.llvm.org/ce/z/mWAsYv Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83360/new/

[PATCH] D83360: [InstSimplify] Remove select ?, undef, X -> X and select ?, X, undef -> X

2020-07-08 Thread John Regehr via Phabricator via cfe-commits
regehr added a comment. @majnemer should work: https://alive2.llvm.org/ce/z/vL4yn4 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83360/new/ https://reviews.llvm.org/D83360 ___ cfe-commits mailing list

[PATCH] D77219: UBSan ␇ runtime

2020-04-01 Thread John Regehr via Phabricator via cfe-commits
regehr added a comment. Sorry but I don't think this can land until it has options for sending sanitizer output to Slack channels and SMS numbers. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77219/new/ https://reviews.llvm.org/D77219

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-19 Thread John Regehr via Phabricator via cfe-commits
regehr added a comment. I am also in the "be a bit conservative at first and see how things shake out" camp, but it's y'all doing the hard work here, not me :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63423/new/ https://reviews.llvm.org/D63423 __

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-17 Thread John Regehr via Phabricator via cfe-commits
regehr added a comment. In D63423#1547216 , @xbolva00 wrote: > The only remaining question is, as your said, whether or not to diagnose xors > in macros. @regerh @jfb ? IMO it's better to not warn about xors in macros-- I like the current tradeoffs. C

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-17 Thread John Regehr via Phabricator via cfe-commits
regehr added a comment. Just wanted to say that I think I agree with the design choices here! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63423/new/ https://reviews.llvm.org/D63423 ___ cfe-commits mailing list cfe-commits@lists.llvm.org h

[PATCH] D53949: [clang][CodeGen] Implicit Conversion Sanitizer: discover the world of CompoundAssign operators

2018-11-01 Thread John Regehr via Phabricator via cfe-commits
regehr added a comment. Regarding ++ and --, I think for now it's fine to just document that these aren't instrumented. Repository: rC Clang https://reviews.llvm.org/D53949 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llv

[PATCH] D53949: [clang][CodeGen] Implicit Conversion Sanitizer: discover the world of CompoundAssign operators

2018-10-31 Thread John Regehr via Phabricator via cfe-commits
regehr added a comment. I do not agree that ++ is performed on the original type. The C99 standard (6.5.3.1.2) appears to be very clear on this point: "The expression ++E is equivalent to (E+=1)." Repository: rC Clang https://reviews.llvm.org/D53949 __

[PATCH] D53949: [clang][CodeGen] Implicit Conversion Sanitizer: discover the world of CompoundAssign operators

2018-10-31 Thread John Regehr via Phabricator via cfe-commits
regehr added a comment. This patch doesn't appear to yet fix the "x++" or "x--" cases. Repository: rC Clang https://reviews.llvm.org/D53949 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe

[PATCH] D53949: [clang][CodeGen] Implicit Conversion Sanitizer: discover the world of CompoundAssign operators

2018-10-31 Thread John Regehr via Phabricator via cfe-commits
regehr added a comment. I can test this and write a few test cases. Repository: rC Clang https://reviews.llvm.org/D53949 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33305: [ubsan] Add a check for pointer overflow UB

2017-06-07 Thread John Regehr via Phabricator via cfe-commits
regehr added a comment. Well, my second program should subtract a multiple of sizeof(T). But anyway, my view is that this is a real overflow and a nasty consequence of the unsigned size_t and the usual arithmetic conversions and I don't think we want to try to poke a hole in UBSan to allow this

[PATCH] D33305: [ubsan] Add a check for pointer overflow UB

2017-06-07 Thread John Regehr via Phabricator via cfe-commits
regehr added a comment. Sorry, let's go with this example instead, which makes it clear that the program is attempting to do something completely sensible: #include typedef struct { int x[2]; } T; int main(void) { T f[1000]; T *p = &f[500]; ptrdiff_t d = -10; p

[PATCH] D33305: [ubsan] Add a check for pointer overflow UB

2017-06-07 Thread John Regehr via Phabricator via cfe-commits
regehr added a comment. I'm taking a look. For reference here's the test program I'm using. #include typedef struct { int x[2]; } T; int main(void) { T f; T *p = &f; ptrdiff_t d = -3293184; p += d / sizeof(T); return 0; } Repository: rL LLVM https://r

[PATCH] D33910: [ubsan] Detect invalid unsigned pointer index expression (clang)

2017-06-05 Thread John Regehr via Phabricator via cfe-commits
regehr added a comment. I'm afraid I don't have a better name for this. Here's the obligatory gcc explorer link though: https://godbolt.org/g/s10h0O https://reviews.llvm.org/D33910 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://list

[PATCH] D29369: [ubsan] Omit superflous overflow checks for promoted arithmetic (PR20193)

2017-02-09 Thread John Regehr via Phabricator via cfe-commits
regehr added a comment. Paging @dtzWill https://reviews.llvm.org/D29369 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D29437: [ubsan] Detect signed overflow UB in remainder operations

2017-02-01 Thread John Regehr via Phabricator via cfe-commits
regehr added a comment. Does this check need to be sensitive to the dialect of C/C++ that the user asked for? I know that it used to be the case that the standard could be read either way for this case, but as you observe it is now unambiguously UB. https://reviews.llvm.org/D29437 _

[PATCH] D29369: [ubsan] Omit superflous overflow checks for promoted arithmetic (PR20193)

2017-02-01 Thread John Regehr via Phabricator via cfe-commits
regehr added a comment. Out of curiosity, how many of these superfluous checks are not subsequently eliminated by InstCombine? https://reviews.llvm.org/D29369 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mai