llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-codegen @llvm/pr-subscribers-clang Author: Adam Magier (AdamMagierFOSS) <details> <summary>Changes</summary> …folded overflowing offsets EmitGEPOffsetInBytes has two paths: for fully constant GEPs it subtracts pointer values and always returns OffsetOverflows=false, but for non-constant GEPs it iterates operands using checked arithmetic. The assertion in EmitCheckedInBoundsGEP assumed a constant TotalOffset implies no overflow, conflating the two paths. In the non-constant path, the offset can be entirely constant-folded (constant index * constant element size) while the GEP itself remains non-constant (runtime base pointer). If that arithmetic overflows intptr_t, we get a constant TotalOffset with OffsetOverflows=true, triggering the assertion. This is easily hit on 16-bit targets like MSP430 with realistic struct array indices. Remove the assertion. The existing codegen already handles this correctly: the constant OffsetOverflows=true propagates into the check condition, making the sanitizer always fire at runtime. Fixes: https://github.com/llvm/llvm-project/issues/48168 Assisted-by: Kiro CLI / Claude Opus 4.6 (1M context) --- Full diff: https://github.com/llvm/llvm-project/pull/191278.diff 2 Files Affected: - (modified) clang/lib/CodeGen/CGExprScalar.cpp (+3-4) - (added) clang/test/CodeGen/ubsan-pointer-overflow-constant-fold.c (+19) ``````````diff diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp index a8dcf22992983..3d9d0e491d19b 100644 --- a/clang/lib/CodeGen/CGExprScalar.cpp +++ b/clang/lib/CodeGen/CGExprScalar.cpp @@ -6413,10 +6413,9 @@ CodeGenFunction::EmitCheckedInBoundsGEP(llvm::Type *ElemTy, Value *Ptr, GEPOffsetAndOverflow EvaluatedGEP = EmitGEPOffsetInBytes(Ptr, GEPVal, getLLVMContext(), CGM, Builder); - assert((!isa<llvm::Constant>(EvaluatedGEP.TotalOffset) || - EvaluatedGEP.OffsetOverflows == Builder.getFalse()) && - "If the offset got constant-folded, we don't expect that there was an " - "overflow."); + // Note: TotalOffset can be a constant even when the GEP is non-constant + // (e.g. runtime base pointer with constant index). In that case, + // OffsetOverflows may be true if the constant offset computation overflowed. auto *Zero = llvm::ConstantInt::getNullValue(IntPtrTy); diff --git a/clang/test/CodeGen/ubsan-pointer-overflow-constant-fold.c b/clang/test/CodeGen/ubsan-pointer-overflow-constant-fold.c new file mode 100644 index 0000000000000..84450c9ae5c76 --- /dev/null +++ b/clang/test/CodeGen/ubsan-pointer-overflow-constant-fold.c @@ -0,0 +1,19 @@ +// REQUIRES: msp430-registered-target +// RUN: %clang_cc1 -triple msp430 -emit-llvm -fsanitize=pointer-overflow -O0 %s -o - | FileCheck %s + +// Verify that pointer-overflow checks are emitted correctly when the GEP +// offset is a constant that overflows the 16-bit intptr_t. Previously this +// triggered an assertion in EmitCheckedInBoundsGEP because the code assumed +// a constant-folded offset could never have an overflow. + +struct sensor_reading { long timestamp; long value; }; + +struct sensor_reading *readings[]; + +// CHECK-LABEL: define {{.*}}@test_constant_offset_overflow +// CHECK: getelementptr inbounds %struct.sensor_reading +// CHECK: call void @__ubsan_handle_pointer_overflow +void test_constant_offset_overflow(void) { + // sizeof(struct sensor_reading) == 8, index 4096: 4096 * 8 = 32768 overflows i16. + readings[0][4096]; +} `````````` </details> https://github.com/llvm/llvm-project/pull/191278 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
