https://github.com/dtcxzyw updated
https://github.com/llvm/llvm-project/pull/130734
>From 0f6ff605da3cbadc5311d4bf6c08fe98970a69c3 Mon Sep 17 00:00:00 2001
From: Yingwei Zheng <dtcxzyw2...@gmail.com>
Date: Thu, 10 Apr 2025 09:54:33 +0800
Subject: [PATCH] [Clang][CodeGen] Do not set inbounds flag for struct GEP with
null base pointers
---
clang/docs/ReleaseNotes.rst | 7 ++
clang/lib/CodeGen/CGExpr.cpp | 33 ++++++---
clang/lib/CodeGen/CodeGenFunction.h | 3 +-
...ptr-and-nonzero-offset-in-offsetof-idiom.c | 2 +-
...r-and-nonzero-offset-in-offsetof-idiom.cpp | 72 ++++++++++++++++++-
5 files changed, 105 insertions(+), 12 deletions(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index cd16641c25ed8..c2f7a519d270a 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -42,6 +42,13 @@ Potentially Breaking Changes
C/C++ Language Potentially Breaking Changes
-------------------------------------------
+- New LLVM optimizations have been implemented that optimize pointer
arithmetic on
+ null pointers more aggressively. As part of this, clang has implemented a
special
+ case for old-style offsetof idioms like ``((int)(&(((struct S
*)0)->field)))``, to
+ ensure they are not caught by these optimizations. It is also possible to
use
+ ``-fwrapv-pointer`` or ``-fno-delete-null-pointer-checks`` to make pointer
arithmetic
+ on null pointers well-defined. (#GH130734, #GH130742)
+
C++ Specific Potentially Breaking Changes
-----------------------------------------
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 451034395976f..c8ff2c880a655 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -4785,6 +4785,16 @@ LValue CodeGenFunction::EmitMemberExpr(const MemberExpr
*E) {
}
Expr *BaseExpr = E->getBase();
+ bool IsInBounds = !getLangOpts().PointerOverflowDefined;
+ if (IsInBounds) {
+ // Check whether the underlying base pointer is a constant null.
+ // If so, we do not set inbounds flag for GEP to avoid breaking some
+ // old-style offsetof idioms.
+ Expr *UnderlyingBaseExpr = BaseExpr->IgnoreParens();
+ while (auto *BaseMemberExpr = dyn_cast<MemberExpr>(UnderlyingBaseExpr))
+ UnderlyingBaseExpr = BaseMemberExpr->getBase()->IgnoreParens();
+ IsInBounds = !getContext().isSentinelNullExpr(UnderlyingBaseExpr);
+ }
// If this is s.x, emit s as an lvalue. If it is s->x, emit s as a scalar.
LValue BaseLV;
if (E->isArrow()) {
@@ -4806,7 +4816,7 @@ LValue CodeGenFunction::EmitMemberExpr(const MemberExpr
*E) {
NamedDecl *ND = E->getMemberDecl();
if (auto *Field = dyn_cast<FieldDecl>(ND)) {
- LValue LV = EmitLValueForField(BaseLV, Field);
+ LValue LV = EmitLValueForField(BaseLV, Field, IsInBounds);
setObjCGCLValueClass(getContext(), E, LV);
if (getLangOpts().OpenMP) {
// If the member was explicitly marked as nontemporal, mark it as
@@ -4892,12 +4902,15 @@ unsigned CodeGenFunction::getDebugInfoFIndex(const
RecordDecl *Rec,
/// Get the address of a zero-sized field within a record. The resulting
/// address doesn't necessarily have the right type.
static Address emitAddrOfZeroSizeField(CodeGenFunction &CGF, Address Base,
- const FieldDecl *Field) {
+ const FieldDecl *Field,
+ bool IsInBounds) {
CharUnits Offset = CGF.getContext().toCharUnitsFromBits(
CGF.getContext().getFieldOffset(Field));
if (Offset.isZero())
return Base;
Base = Base.withElementType(CGF.Int8Ty);
+ if (!IsInBounds)
+ return CGF.Builder.CreateConstByteGEP(Base, Offset);
return CGF.Builder.CreateConstInBoundsByteGEP(Base, Offset);
}
@@ -4906,16 +4919,16 @@ static Address emitAddrOfZeroSizeField(CodeGenFunction
&CGF, Address Base,
///
/// The resulting address doesn't necessarily have the right type.
static Address emitAddrOfFieldStorage(CodeGenFunction &CGF, Address base,
- const FieldDecl *field) {
+ const FieldDecl *field, bool IsInBounds)
{
if (isEmptyFieldForLayout(CGF.getContext(), field))
- return emitAddrOfZeroSizeField(CGF, base, field);
+ return emitAddrOfZeroSizeField(CGF, base, field, IsInBounds);
const RecordDecl *rec = field->getParent();
unsigned idx =
CGF.CGM.getTypes().getCGRecordLayout(rec).getLLVMFieldNo(field);
- if (CGF.getLangOpts().PointerOverflowDefined)
+ if (!IsInBounds)
return CGF.Builder.CreateConstGEP2_32(base, 0, idx, field->getName());
return CGF.Builder.CreateStructGEP(base, idx, field->getName());
@@ -4953,8 +4966,8 @@ static bool hasAnyVptr(const QualType Type, const
ASTContext &Context) {
return false;
}
-LValue CodeGenFunction::EmitLValueForField(LValue base,
- const FieldDecl *field) {
+LValue CodeGenFunction::EmitLValueForField(LValue base, const FieldDecl *field,
+ bool IsInBounds) {
LValueBaseInfo BaseInfo = base.getBaseInfo();
if (field->isBitField()) {
@@ -5090,7 +5103,7 @@ LValue CodeGenFunction::EmitLValueForField(LValue base,
if (!IsInPreservedAIRegion &&
(!getDebugInfo() || !rec->hasAttr<BPFPreserveAccessIndexAttr>()))
// For structs, we GEP to the field that the record layout suggests.
- addr = emitAddrOfFieldStorage(*this, addr, field);
+ addr = emitAddrOfFieldStorage(*this, addr, field, IsInBounds);
else
// Remember the original struct field index
addr = emitPreserveStructAccess(*this, base, addr, field);
@@ -5134,7 +5147,9 @@ CodeGenFunction::EmitLValueForFieldInitialization(LValue
Base,
if (!FieldType->isReferenceType())
return EmitLValueForField(Base, Field);
- Address V = emitAddrOfFieldStorage(*this, Base.getAddress(), Field);
+ Address V = emitAddrOfFieldStorage(
+ *this, Base.getAddress(), Field,
+ /*IsInBounds=*/!getLangOpts().PointerOverflowDefined);
// Make sure that the address is pointing to the right type.
llvm::Type *llvmType = ConvertTypeForMem(FieldType);
diff --git a/clang/lib/CodeGen/CodeGenFunction.h
b/clang/lib/CodeGen/CodeGenFunction.h
index 2b1062d6d307c..27c464bc98a12 100644
--- a/clang/lib/CodeGen/CodeGenFunction.h
+++ b/clang/lib/CodeGen/CodeGenFunction.h
@@ -4459,7 +4459,8 @@ class CodeGenFunction : public CodeGenTypeCache {
const ObjCIvarDecl *Ivar);
llvm::Value *EmitIvarOffsetAsPointerDiff(const ObjCInterfaceDecl *Interface,
const ObjCIvarDecl *Ivar);
- LValue EmitLValueForField(LValue Base, const FieldDecl *Field);
+ LValue EmitLValueForField(LValue Base, const FieldDecl *Field,
+ bool IsInBounds = false);
LValue EmitLValueForLambdaField(const FieldDecl *Field);
LValue EmitLValueForLambdaField(const FieldDecl *Field,
llvm::Value *ThisValue);
diff --git
a/clang/test/CodeGen/catch-nullptr-and-nonzero-offset-in-offsetof-idiom.c
b/clang/test/CodeGen/catch-nullptr-and-nonzero-offset-in-offsetof-idiom.c
index 68c0ee3a3a885..46e22fbdb38ac 100644
--- a/clang/test/CodeGen/catch-nullptr-and-nonzero-offset-in-offsetof-idiom.c
+++ b/clang/test/CodeGen/catch-nullptr-and-nonzero-offset-in-offsetof-idiom.c
@@ -17,7 +17,7 @@ struct S {
// CHECK-LABEL: @get_offset_of_y_naively(
// CHECK-NEXT: entry:
-// CHECK-NEXT: ret i64 ptrtoint (ptr getelementptr inbounds nuw
([[STRUCT_S:%.*]], ptr null, i32 0, i32 1) to i64)
+// CHECK-NEXT: ret i64 ptrtoint (ptr getelementptr ([[STRUCT_S:%.*]], ptr
null, i32 0, i32 1) to i64)
//
uintptr_t get_offset_of_y_naively(void) {
return ((uintptr_t)(&(((struct S *)0)->y)));
diff --git
a/clang/test/CodeGenCXX/catch-nullptr-and-nonzero-offset-in-offsetof-idiom.cpp
b/clang/test/CodeGenCXX/catch-nullptr-and-nonzero-offset-in-offsetof-idiom.cpp
index 34d4f4c9e34eb..6a0d6265eff96 100644
---
a/clang/test/CodeGenCXX/catch-nullptr-and-nonzero-offset-in-offsetof-idiom.cpp
+++
b/clang/test/CodeGenCXX/catch-nullptr-and-nonzero-offset-in-offsetof-idiom.cpp
@@ -10,12 +10,82 @@ struct S {
// CHECK-LABEL: @_Z23get_offset_of_y_naivelyv(
// CHECK-NEXT: entry:
-// CHECK-NEXT: ret i64 ptrtoint (ptr getelementptr inbounds nuw
([[STRUCT_S:%.*]], ptr null, i32 0, i32 1) to i64)
+// CHECK-NEXT: ret i64 ptrtoint (ptr getelementptr ([[STRUCT_S:%.*]], ptr
null, i32 0, i32 1) to i64)
//
uintptr_t get_offset_of_y_naively() {
return ((uintptr_t)(&(((S *)nullptr)->y)));
}
+struct Empty {};
+
+struct T {
+ int a;
+ S s;
+ [[no_unique_address]] Empty e1;
+ int b;
+ [[no_unique_address]] Empty e2;
+};
+
+// CHECK-LABEL: @_Z30get_offset_of_y_naively_nestedv(
+// CHECK-NEXT: entry:
+// CHECK-NEXT: ret i64 ptrtoint (ptr getelementptr ([[STRUCT_S:%.*]], ptr
getelementptr ([[STRUCT_T:%.*]], ptr null, i32 0, i32 1), i32 0, i32 1) to i64)
+//
+uintptr_t get_offset_of_y_naively_nested() {
+ return ((uintptr_t)(&(((T *)nullptr)->s.y)));
+}
+
+// CHECK-LABEL: @_Z42get_offset_of_y_naively_nested_with_parensv(
+// CHECK-NEXT: entry:
+// CHECK-NEXT: ret i64 ptrtoint (ptr getelementptr ([[STRUCT_S:%.*]], ptr
getelementptr ([[STRUCT_T:%.*]], ptr null, i32 0, i32 1), i32 0, i32 1) to i64)
+//
+uintptr_t get_offset_of_y_naively_nested_with_parens() {
+ return ((uintptr_t)(&((((T *)nullptr)->s).y)));
+}
+
+// CHECK-LABEL: @_Z26get_offset_of_zero_storagev(
+// CHECK-NEXT: entry:
+// CHECK-NEXT: ret i64 ptrtoint (ptr getelementptr (i8, ptr null, i64 16)
to i64)
+//
+uintptr_t get_offset_of_zero_storage() {
+ return ((uintptr_t)(&(((T *)nullptr)->e2)));
+}
+
+namespace std { typedef decltype(__nullptr) nullptr_t; }
+// CHECK-LABEL: @_Z29get_offset_of_y_integral_zerov(
+// CHECK-NEXT: entry:
+// CHECK-NEXT: ret i64 ptrtoint (ptr getelementptr ([[STRUCT_S:%.*]], ptr
null, i32 0, i32 1) to i64)
+//
+uintptr_t get_offset_of_y_integral_zero() {
+ return ((uintptr_t)(&(((S *)0)->y)));
+}
+
+// CHECK-LABEL: @_Z37get_offset_of_y_integral_zero_voidptrv(
+// CHECK-NEXT: entry:
+// CHECK-NEXT: ret i64 ptrtoint (ptr getelementptr ([[STRUCT_S:%.*]], ptr
null, i32 0, i32 1) to i64)
+//
+uintptr_t get_offset_of_y_integral_zero_voidptr() {
+ return ((uintptr_t)(&(((S *)(void*)0)->y)));
+}
+
+// CHECK-LABEL: @_Z25get_offset_of_y_nullptr_tv(
+// CHECK-NEXT: entry:
+// CHECK-NEXT: ret i64 ptrtoint (ptr getelementptr ([[STRUCT_S:%.*]], ptr
null, i32 0, i32 1) to i64)
+//
+uintptr_t get_offset_of_y_nullptr_t() {
+ return ((uintptr_t)(&(((S *)std::nullptr_t{})->y)));
+}
+
+// CHECK-LABEL: @_Z32get_offset_of_y_nullptr_constantv(
+// CHECK-NEXT: entry:
+// CHECK-NEXT: [[NULL:%.*]] = alloca ptr, align 8
+// CHECK-NEXT: store ptr null, ptr [[NULL]], align 8
+// CHECK-NEXT: ret i64 ptrtoint (ptr getelementptr inbounds nuw
([[STRUCT_S:%.*]], ptr null, i32 0, i32 1) to i64)
+//
+uintptr_t get_offset_of_y_nullptr_constant() {
+ constexpr void *null = nullptr;
+ return ((uintptr_t)(&(((S *)null)->y)));
+}
+
// CHECK-LABEL: @_Z27get_offset_of_y_via_builtinv(
// CHECK-NEXT: entry:
// CHECK-NEXT: ret i64 4
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
