Author: Yingwei Zheng Date: 2025-04-11T09:04:23+08:00 New Revision: 1711996805c506f5717193aaeedf3752dfdd900d
URL: https://github.com/llvm/llvm-project/commit/1711996805c506f5717193aaeedf3752dfdd900d DIFF: https://github.com/llvm/llvm-project/commit/1711996805c506f5717193aaeedf3752dfdd900d.diff LOG: [Clang][CodeGen] Do not set inbounds flag for struct GEP with null base pointers (#130734) In the LLVM middle-end we want to fold `gep inbounds null, idx -> null`: https://alive2.llvm.org/ce/z/5ZkPx- This pattern is common in real-world programs (https://github.com/dtcxzyw/llvm-opt-benchmark/pull/55#issuecomment-1870963906). Generally, it exists in some (actually) unreachable blocks, which is introduced by JumpThreading. However, some old-style offsetof macros are still widely used in real-world C/C++ code (e.g., hwloc/slurm/luajit). To avoid breaking existing code and inconvenience to downstream users, this patch removes the inbounds flag from the struct gep if the base pointer is null. Added: Modified: clang/docs/ReleaseNotes.rst clang/lib/CodeGen/CGExpr.cpp clang/lib/CodeGen/CodeGenFunction.h clang/test/CodeGen/catch-nullptr-and-nonzero-offset-in-offsetof-idiom.c clang/test/CodeGenCXX/catch-nullptr-and-nonzero-offset-in-offsetof-idiom.cpp Removed: ################################################################################ diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index beab7600e2c32..fca2c4248155c 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 62eaff4e6a978..03aa2e27ee075 100644 --- a/clang/lib/CodeGen/CGExpr.cpp +++ b/clang/lib/CodeGen/CGExpr.cpp @@ -4772,6 +4772,13 @@ EmitExtVectorElementExpr(const ExtVectorElementExpr *E) { Base.getBaseInfo(), TBAAAccessInfo()); } +bool CodeGenFunction::isUnderlyingBasePointerConstantNull(const Expr *E) { + const Expr *UnderlyingBaseExpr = E->IgnoreParens(); + while (auto *BaseMemberExpr = dyn_cast<MemberExpr>(UnderlyingBaseExpr)) + UnderlyingBaseExpr = BaseMemberExpr->getBase()->IgnoreParens(); + return getContext().isSentinelNullExpr(UnderlyingBaseExpr); +} + LValue CodeGenFunction::EmitMemberExpr(const MemberExpr *E) { if (DeclRefExpr *DRE = tryToConvertMemberExprToDeclRefExpr(*this, E)) { EmitIgnoredExpr(E->getBase()); @@ -4779,6 +4786,11 @@ LValue CodeGenFunction::EmitMemberExpr(const MemberExpr *E) { } Expr *BaseExpr = E->getBase(); + // 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. + bool IsInBounds = !getLangOpts().PointerOverflowDefined && + !isUnderlyingBasePointerConstantNull(BaseExpr); // 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()) { @@ -4800,7 +4812,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 @@ -4886,12 +4898,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); } @@ -4900,16 +4915,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()); @@ -4947,8 +4962,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()) { @@ -4971,7 +4986,7 @@ LValue CodeGenFunction::EmitLValueForField(LValue base, (!getDebugInfo() || !rec->hasAttr<BPFPreserveAccessIndexAttr>())) { if (Idx != 0) { // For structs, we GEP to the field that the record layout suggests. - if (getLangOpts().PointerOverflowDefined) + if (!IsInBounds) Addr = Builder.CreateConstGEP2_32(Addr, 0, Idx, field->getName()); else Addr = Builder.CreateStructGEP(Addr, Idx, field->getName()); @@ -5084,7 +5099,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); @@ -5128,7 +5143,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 a398ba55dcdc7..9d0a46bc4808b 100644 --- a/clang/lib/CodeGen/CodeGenFunction.h +++ b/clang/lib/CodeGen/CodeGenFunction.h @@ -4461,7 +4461,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 = true); LValue EmitLValueForLambdaField(const FieldDecl *Field); LValue EmitLValueForLambdaField(const FieldDecl *Field, llvm::Value *ThisValue); @@ -4558,6 +4559,8 @@ class CodeGenFunction : public CodeGenTypeCache { const CXXRecordDecl *RD); bool isPointerKnownNonNull(const Expr *E); + /// Check whether the underlying base pointer is a constant null. + bool isUnderlyingBasePointerConstantNull(const Expr *E); /// Create the discriminator from the storage address and the entity hash. llvm::Value *EmitPointerAuthBlendDiscriminator(llvm::Value *StorageAddress, 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
