UBSan false positive: runtime error: load of address with insufficient space for an object of type 'int'

[hvdijk]

Please consider this small reduced testcase:

int x;
int main(void) {
  int array[4] = {0};
  int *ptr;
  if (x) {
    ptr = 0;
  } else {
    ptr = array + 4;
  }
  return ptr[-1];
}

This, as far as I know, is a strictly conforming C program. But Clang's UBSan rejects it as of version 15:

$ clang-14 test.c -O3 -o test -fsanitize=undefined && ./test
$ clang-15 test.c -O3 -o test -fsanitize=undefined && ./test
test.c:10:10: runtime error: load of address 0xffffd0f236dc with insufficient space for an object of type 'int'
0xffffd0f236dc: note: pointer points here
  00 00 00 00 00 00 00 00  00 38 f2 d0 ff ff 00 00  c4 84 f1 eb 72 e1 00 00  78 38 f2 d0 ff ff 00 00
              ^
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior test.c:10:10 in

This continues to be rejected in all later versions too. From what I can tell debugging, it's going wrong in llvm::lowerObjectSizeCall: this sets EvalOptions.EvalMode = ObjectSizeOpts::Mode::ExactSizeFromOffset. ExactSizeFromOffset allows combining SizeOffsetValues if the sizes are the same, even if the offsets are different. So, we combine "0 bytes into a 0-byte-sized object" and "16 bytes into a 16-byte-sized object" into just "0 bytes into a 0-byte-sized object". But that is wrong when we then use that to conclude that we cannot access any bytes before that object.

If I change LLVM as

diff --git a/llvm/lib/Analysis/MemoryBuiltins.cpp b/llvm/lib/Analysis/MemoryBuiltins.cpp
index e1abf5e4d885..8abbb074647c 100644
--- a/llvm/lib/Analysis/MemoryBuiltins.cpp
+++ b/llvm/lib/Analysis/MemoryBuiltins.cpp
@@ -610,7 +610,7 @@ Value *llvm::lowerObjectSizeCall(
     EvalOptions.EvalMode =
         MaxVal ? ObjectSizeOpts::Mode::Max : ObjectSizeOpts::Mode::Min;
   else
-    EvalOptions.EvalMode = ObjectSizeOpts::Mode::ExactSizeFromOffset;
+    EvalOptions.EvalMode = ObjectSizeOpts::Mode::ExactUnderlyingSizeAndOffset;

   EvalOptions.NullIsUnknownSize =
       cast<ConstantInt>(ObjectSize->getArgOperand(2))->isOne();

then this program gets accepted. But I expect this change to not be right, I expect this to result in poorer codegen where lowerObjectSizeCall is already doing the right thing.

[hvdijk]

This happened ever since PHI node handling was added in D121897 d8e0a6d, cc @serge-sans-paille & @nikic

[serge-sans-paille]

I managed to reproduce and analyze the issue. I have a working patch but I need to test it some more, keep in touch.