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

[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 i32 %x2
>   }

that's fine but I still don't understand why the counterexample to my version 
says %x2 in @src can be undef


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83360



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

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

[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/

https://reviews.llvm.org/D83360



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

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

[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
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D77219: UBSan ␇ runtime

[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



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

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

[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



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

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

[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.


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

https://reviews.llvm.org/D63423



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

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

[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
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[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.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[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



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

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

[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-commits

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

[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

[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 idiom unless it turns out to be extremely 
common.

I think it would be better style to cast the sizeof() to a ptrdiff_t rather 
than to an int, but as long as d is a ptrdiff_t this won't matter.


Repository:
  rL LLVM

https://reviews.llvm.org/D33305



___
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

[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 += d / sizeof(T);
return 0;
  }


Repository:
  rL LLVM

https://reviews.llvm.org/D33305



___
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

[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://reviews.llvm.org/D33305



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

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

[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://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[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

[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



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

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

[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/mailman/listinfo/cfe-commits