Author: Yingwei Zheng Date: 2025-04-11T10:51:08+08:00 New Revision: 04c38981a9ce3e6225669c0e41cab947e3e7989f
URL: https://github.com/llvm/llvm-project/commit/04c38981a9ce3e6225669c0e41cab947e3e7989f DIFF: https://github.com/llvm/llvm-project/commit/04c38981a9ce3e6225669c0e41cab947e3e7989f.diff LOG: [Clang][CodeGen] Do not set inbounds flag in `EmitMemberDataPointerAddress` when the base pointer is null (#130952) See also https://github.com/llvm/llvm-project/pull/130734 for the original motivation. This pattern (`container_of`) is also widely used by real-world programs. Examples: https://github.com/llvm/llvm-project/blob/1d89d7d5d76e391b035f50343e2a4890506c6f2b/llvm/include/llvm/IR/SymbolTableListTraits.h#L77-L87 https://github.com/nodejs/node/blob/a2a53cb728ec5f776ac16879ce0f480a8e838320/src/util-inl.h#L134-L137 https://github.com/search?q=*%29nullptr-%3E*&type=code Added: Modified: clang/docs/ReleaseNotes.rst clang/lib/CodeGen/CGCXXABI.cpp clang/lib/CodeGen/CGCXXABI.h clang/lib/CodeGen/CGClass.cpp clang/lib/CodeGen/CGExpr.cpp clang/lib/CodeGen/CodeGenFunction.h clang/lib/CodeGen/ItaniumCXXABI.cpp clang/lib/CodeGen/MicrosoftCXXABI.cpp clang/test/CodeGenCXX/microsoft-abi-member-pointers.cpp clang/test/CodeGenCXX/pointers-to-data-members.cpp Removed: ################################################################################ diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index fca2c4248155c..5983621a7a875 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -47,7 +47,7 @@ C/C++ Language Potentially Breaking Changes 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) + on null pointers well-defined. (#GH130734, #GH130742, #GH130952) C++ Specific Potentially Breaking Changes ----------------------------------------- diff --git a/clang/lib/CodeGen/CGCXXABI.cpp b/clang/lib/CodeGen/CGCXXABI.cpp index 9f77fbec21380..93dc17bac95e0 100644 --- a/clang/lib/CodeGen/CGCXXABI.cpp +++ b/clang/lib/CodeGen/CGCXXABI.cpp @@ -60,10 +60,9 @@ CGCallee CGCXXABI::EmitLoadOfMemberFunctionPointer( return CGCallee::forDirect(FnPtr, FPT); } -llvm::Value * -CGCXXABI::EmitMemberDataPointerAddress(CodeGenFunction &CGF, const Expr *E, - Address Base, llvm::Value *MemPtr, - const MemberPointerType *MPT) { +llvm::Value *CGCXXABI::EmitMemberDataPointerAddress( + CodeGenFunction &CGF, const Expr *E, Address Base, llvm::Value *MemPtr, + const MemberPointerType *MPT, bool IsInBounds) { ErrorUnsupportedABI(CGF, "loads of member pointers"); llvm::Type *Ty = llvm::PointerType::get(CGF.getLLVMContext(), Base.getAddressSpace()); diff --git a/clang/lib/CodeGen/CGCXXABI.h b/clang/lib/CodeGen/CGCXXABI.h index 148a7ba6df7e6..da2d367da7096 100644 --- a/clang/lib/CodeGen/CGCXXABI.h +++ b/clang/lib/CodeGen/CGCXXABI.h @@ -192,7 +192,7 @@ class CGCXXABI { virtual llvm::Value * EmitMemberDataPointerAddress(CodeGenFunction &CGF, const Expr *E, Address Base, llvm::Value *MemPtr, - const MemberPointerType *MPT); + const MemberPointerType *MPT, bool IsInBounds); /// Perform a derived-to-base, base-to-derived, or bitcast member /// pointer conversion. diff --git a/clang/lib/CodeGen/CGClass.cpp b/clang/lib/CodeGen/CGClass.cpp index c683dbb0af825..0f2422a6a665a 100644 --- a/clang/lib/CodeGen/CGClass.cpp +++ b/clang/lib/CodeGen/CGClass.cpp @@ -147,16 +147,13 @@ Address CodeGenFunction::LoadCXXThisAddress() { /// Emit the address of a field using a member data pointer. /// /// \param E Only used for emergency diagnostics -Address -CodeGenFunction::EmitCXXMemberDataPointerAddress(const Expr *E, Address base, - llvm::Value *memberPtr, - const MemberPointerType *memberPtrType, - LValueBaseInfo *BaseInfo, - TBAAAccessInfo *TBAAInfo) { +Address CodeGenFunction::EmitCXXMemberDataPointerAddress( + const Expr *E, Address base, llvm::Value *memberPtr, + const MemberPointerType *memberPtrType, bool IsInBounds, + LValueBaseInfo *BaseInfo, TBAAAccessInfo *TBAAInfo) { // Ask the ABI to compute the actual address. - llvm::Value *ptr = - CGM.getCXXABI().EmitMemberDataPointerAddress(*this, E, base, - memberPtr, memberPtrType); + llvm::Value *ptr = CGM.getCXXABI().EmitMemberDataPointerAddress( + *this, E, base, memberPtr, memberPtrType, IsInBounds); QualType memberType = memberPtrType->getPointeeType(); CharUnits memberAlign = diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp index 03aa2e27ee075..4d6e776f42660 100644 --- a/clang/lib/CodeGen/CGExpr.cpp +++ b/clang/lib/CodeGen/CGExpr.cpp @@ -654,8 +654,8 @@ EmitMaterializeTemporaryExpr(const MaterializeTemporaryExpr *M) { case SubobjectAdjustment::MemberPointerAdjustment: { llvm::Value *Ptr = EmitScalarExpr(Adjustment.Ptr.RHS); - Object = EmitCXXMemberDataPointerAddress(E, Object, Ptr, - Adjustment.Ptr.MPT); + Object = EmitCXXMemberDataPointerAddress( + E, Object, Ptr, Adjustment.Ptr.MPT, /*IsInBounds=*/true); break; } } @@ -6295,9 +6295,10 @@ EmitPointerToDataMemberBinaryExpr(const BinaryOperator *E) { LValueBaseInfo BaseInfo; TBAAAccessInfo TBAAInfo; - Address MemberAddr = - EmitCXXMemberDataPointerAddress(E, BaseAddr, OffsetV, MPT, &BaseInfo, - &TBAAInfo); + bool IsInBounds = !getLangOpts().PointerOverflowDefined && + !isUnderlyingBasePointerConstantNull(E->getLHS()); + Address MemberAddr = EmitCXXMemberDataPointerAddress( + E, BaseAddr, OffsetV, MPT, IsInBounds, &BaseInfo, &TBAAInfo); return MakeAddrLValue(MemberAddr, MPT->getPointeeType(), BaseInfo, TBAAInfo); } diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h index 9d0a46bc4808b..cdddc69effb86 100644 --- a/clang/lib/CodeGen/CodeGenFunction.h +++ b/clang/lib/CodeGen/CodeGenFunction.h @@ -4642,7 +4642,7 @@ class CodeGenFunction : public CodeGenTypeCache { // Compute the object pointer. Address EmitCXXMemberDataPointerAddress( const Expr *E, Address base, llvm::Value *memberPtr, - const MemberPointerType *memberPtrType, + const MemberPointerType *memberPtrType, bool IsInBounds, LValueBaseInfo *BaseInfo = nullptr, TBAAAccessInfo *TBAAInfo = nullptr); RValue EmitCXXMemberPointerCallExpr(const CXXMemberCallExpr *E, ReturnValueSlot ReturnValue, diff --git a/clang/lib/CodeGen/ItaniumCXXABI.cpp b/clang/lib/CodeGen/ItaniumCXXABI.cpp index 2822d526a54b0..35485dc6d867f 100644 --- a/clang/lib/CodeGen/ItaniumCXXABI.cpp +++ b/clang/lib/CodeGen/ItaniumCXXABI.cpp @@ -130,11 +130,10 @@ class ItaniumCXXABI : public CodeGen::CGCXXABI { llvm::Value *MemFnPtr, const MemberPointerType *MPT) override; - llvm::Value * - EmitMemberDataPointerAddress(CodeGenFunction &CGF, const Expr *E, - Address Base, - llvm::Value *MemPtr, - const MemberPointerType *MPT) override; + llvm::Value *EmitMemberDataPointerAddress(CodeGenFunction &CGF, const Expr *E, + Address Base, llvm::Value *MemPtr, + const MemberPointerType *MPT, + bool IsInBounds) override; llvm::Value *EmitMemberPointerConversion(CodeGenFunction &CGF, const CastExpr *E, @@ -867,14 +866,16 @@ CGCallee ItaniumCXXABI::EmitLoadOfMemberFunctionPointer( /// base object. llvm::Value *ItaniumCXXABI::EmitMemberDataPointerAddress( CodeGenFunction &CGF, const Expr *E, Address Base, llvm::Value *MemPtr, - const MemberPointerType *MPT) { + const MemberPointerType *MPT, bool IsInBounds) { assert(MemPtr->getType() == CGM.PtrDiffTy); CGBuilderTy &Builder = CGF.Builder; - // Apply the offset, which we assume is non-null. - return Builder.CreateInBoundsGEP(CGF.Int8Ty, Base.emitRawPointer(CGF), MemPtr, - "memptr.offset"); + // Apply the offset. + llvm::Value *BaseAddr = Base.emitRawPointer(CGF); + return Builder.CreateGEP(CGF.Int8Ty, BaseAddr, MemPtr, "memptr.offset", + IsInBounds ? llvm::GEPNoWrapFlags::inBounds() + : llvm::GEPNoWrapFlags::none()); } // See if it's possible to return a constant signed pointer. diff --git a/clang/lib/CodeGen/MicrosoftCXXABI.cpp b/clang/lib/CodeGen/MicrosoftCXXABI.cpp index 7bef436302526..93ca8704de742 100644 --- a/clang/lib/CodeGen/MicrosoftCXXABI.cpp +++ b/clang/lib/CodeGen/MicrosoftCXXABI.cpp @@ -689,10 +689,10 @@ class MicrosoftCXXABI : public CGCXXABI { llvm::Value *MemPtr, const MemberPointerType *MPT) override; - llvm::Value * - EmitMemberDataPointerAddress(CodeGenFunction &CGF, const Expr *E, - Address Base, llvm::Value *MemPtr, - const MemberPointerType *MPT) override; + llvm::Value *EmitMemberDataPointerAddress(CodeGenFunction &CGF, const Expr *E, + Address Base, llvm::Value *MemPtr, + const MemberPointerType *MPT, + bool IsInBounds) override; llvm::Value *EmitNonNullMemberPointerConversion( const MemberPointerType *SrcTy, const MemberPointerType *DstTy, @@ -3240,7 +3240,7 @@ llvm::Value *MicrosoftCXXABI::AdjustVirtualBase( llvm::Value *MicrosoftCXXABI::EmitMemberDataPointerAddress( CodeGenFunction &CGF, const Expr *E, Address Base, llvm::Value *MemPtr, - const MemberPointerType *MPT) { + const MemberPointerType *MPT, bool IsInBounds) { assert(MPT->isMemberDataPointer()); CGBuilderTy &Builder = CGF.Builder; const CXXRecordDecl *RD = MPT->getMostRecentCXXRecordDecl(); @@ -3269,9 +3269,10 @@ llvm::Value *MicrosoftCXXABI::EmitMemberDataPointerAddress( Addr = Base.emitRawPointer(CGF); } - // Apply the offset, which we assume is non-null. - return Builder.CreateInBoundsGEP(CGF.Int8Ty, Addr, FieldOffset, - "memptr.offset"); + // Apply the offset. + return Builder.CreateGEP(CGF.Int8Ty, Addr, FieldOffset, "memptr.offset", + IsInBounds ? llvm::GEPNoWrapFlags::inBounds() + : llvm::GEPNoWrapFlags::none()); } llvm::Value * diff --git a/clang/test/CodeGenCXX/microsoft-abi-member-pointers.cpp b/clang/test/CodeGenCXX/microsoft-abi-member-pointers.cpp index fc8a31e0350e5..fe4cab164249f 100644 --- a/clang/test/CodeGenCXX/microsoft-abi-member-pointers.cpp +++ b/clang/test/CodeGenCXX/microsoft-abi-member-pointers.cpp @@ -964,3 +964,24 @@ static_assert(sizeof(b::d) == 16, ""); static_assert(sizeof(void (a<int>::*)()) == 16, ""); #endif } + +namespace ContainerOf { + using size_t = unsigned long long; + + struct List { + int data; + }; + + struct Node { + int data; + struct List list1; + struct List list2; + }; + + // CHECK-LABEL: define{{.*}} ptr @"?getOwner@ContainerOf@@ + // CHECK: %memptr.offset = getelementptr i8, ptr null, {{.*}} + Node* getOwner(List *list, List Node::*member) { + size_t offset = reinterpret_cast<size_t>(&((Node*)nullptr->*member)); + return reinterpret_cast<Node*>(reinterpret_cast<char*>(list) - offset); + } +} diff --git a/clang/test/CodeGenCXX/pointers-to-data-members.cpp b/clang/test/CodeGenCXX/pointers-to-data-members.cpp index cf1d6c018409d..2ee6c65cf167d 100644 --- a/clang/test/CodeGenCXX/pointers-to-data-members.cpp +++ b/clang/test/CodeGenCXX/pointers-to-data-members.cpp @@ -265,3 +265,24 @@ namespace PR47864 { struct D : B { int m; }; auto x = (int B::*)&D::m; } + +namespace ContainerOf { + using size_t = unsigned long long; + + struct List { + int data; + }; + + struct Node { + int data; + struct List list1; + struct List list2; + }; + + // CHECK-LABEL: define{{.*}} ptr @_ZN11ContainerOf8getOwnerEPNS_4ListEMNS_4NodeES0_ + // CHECK: %memptr.offset = getelementptr i8, ptr null, i64 {{.*}} + Node* getOwner(List *list, List Node::*member) { + size_t offset = reinterpret_cast<size_t>(&((Node*)nullptr->*member)); + return reinterpret_cast<Node*>(reinterpret_cast<char*>(list) - offset); + } +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
