[PATCH] D144319: [SimplifyCFG] Check if the return instruction causes undefined behavior

Sun Feb 19 03:27:01 PST 2023

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

LGTM

In D144319#4137514 <https://reviews.llvm.org/D144319#4137514>, @DianQK wrote:

> According to https://github.com/google/sanitizers/wiki/SanitizerBotReproduceBuild, I can reproduce the issue locally. But this build is taking too long, and I'm still verifying the sanitizer checks after the fix.

It's probably fine to just recommit and keep an eye on the buildbot.

In D144319#4137515 <https://reviews.llvm.org/D144319#4137515>, @DianQK wrote:

> Another thing I'm curious about is why all the `gep zero` have been removed in the `llvm/test/Transforms/SimplifyCFG/UnreachableEliminate.ll` file at the 8979ae42769e529b0f6fce3268492ffb49bd54b9 <https://github.com/llvm/llvm-project/commit/8979ae42769e529b0f6fce3268492ffb49bd54b9#diff-1ef9510bac94e9a0b139fc4adac4ca65b8687248f5691a7fae468eab2a396a1bL404>.

With opaque pointers zero-index GEPs can no longer occur in canonical IR, so they get dropped to test a more realistic input.

================
Comment at: llvm/lib/Transforms/Utils/SimplifyCFG.cpp:7124
       if (GEP->getPointerOperand() == I) {
         if (!GEP->isInBounds() || !GEP->hasAllZeroIndices())
           PtrValueMayBeModified = true;
----------------
As a side note, I'm pretty sure this was supposed to read `!GEP->isInBounds() && !GEP->hasAllZeroIndices()`, the current check doesn't make a lot of sense.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144319