erichkeane added a comment.

I haven't looked at the tests because I don't terribly understand the sanitizer 
IR (hopefully someone else can take a look), but the logic/motivation seems 
solid to me.



================
Comment at: clang/lib/CodeGen/CGExprScalar.cpp:2422
   } else if (type->isIntegerType()) {
-    // Note that signed integer inc/dec with width less than int can't
-    // overflow because of promotion rules; we're just eliding a few steps 
here.
-    if (E->canOverflow() && type->isSignedIntegerOrEnumerationType()) {
+    if (CGF.SanOpts.hasOneOf(SanitizerKind::ImplicitIntegerTruncation) &&
+        type->isPromotableIntegerType()) {
----------------
Should this be 'has' instead of 'hasOneOf'?  


================
Comment at: clang/lib/CodeGen/CGExprScalar.cpp:2428
+      // promotion rules, so we omit promotion+demotion, which means that we 
can
+      // not catch lossy "demotion". Since we still want to catch those cases,
+      // if the sanitizer is enabled, let's perform the promotion, perform
----------------
comma at the end here isn't required.  The one after 'enabled' needs to be a 
semicolon I think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70539



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

Reply via email to