[PATCH] D65417: [SCCP] Update condition to avoid overflow.

[Sanjoy Das (Work Account) via Phabricator via llvm-commits]

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

lgtm with some nits



================
Comment at: lib/Analysis/ConstantFolding.cpp:547
   // If we're not accessing anything in this constant, the result is undefined.
-  if (Offset + BytesLoaded <= 0)
+  if (Offset >= InitializerSize)
     return UndefValue::get(IntType);
----------------
OOC is there a reason to flip the order of the two checks?  If yes, can you please update the commit message (so as to not confuse folks digging through the commit history in the future)?


================
Comment at: test/Transforms/SCCP/ubsan_overflow.ll:12
+  %3 = load i8, i8* %2, align 1
+  unreachable
+}
----------------
We probably should remove the `unreachable`.  As written the function has unconditional UB which means some other optimization within SCCP could kick in.

Probably the best thing to do is to return the value you loaded so that it can't get DCE'd away either (SCCP is unlikely to do that, but you never know).


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65417