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