Skip to content

Commit 04c3898

Browse files
authored
[Clang][CodeGen] Do not set inbounds flag in EmitMemberDataPointerAddress when the base pointer is null (#130952)
See also #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
1 parent f400013 commit 04c3898

File tree

10 files changed

+79
-38
lines changed

10 files changed

+79
-38
lines changed

clang/docs/ReleaseNotes.rst

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ C/C++ Language Potentially Breaking Changes
4747
case for old-style offsetof idioms like ``((int)(&(((struct S *)0)->field)))``, to
4848
ensure they are not caught by these optimizations. It is also possible to use
4949
``-fwrapv-pointer`` or ``-fno-delete-null-pointer-checks`` to make pointer arithmetic
50-
on null pointers well-defined. (#GH130734, #GH130742)
50+
on null pointers well-defined. (#GH130734, #GH130742, #GH130952)
5151

5252
C++ Specific Potentially Breaking Changes
5353
-----------------------------------------

clang/lib/CodeGen/CGCXXABI.cpp

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,10 +60,9 @@ CGCallee CGCXXABI::EmitLoadOfMemberFunctionPointer(
6060
return CGCallee::forDirect(FnPtr, FPT);
6161
}
6262

63-
llvm::Value *
64-
CGCXXABI::EmitMemberDataPointerAddress(CodeGenFunction &CGF, const Expr *E,
65-
Address Base, llvm::Value *MemPtr,
66-
const MemberPointerType *MPT) {
63+
llvm::Value *CGCXXABI::EmitMemberDataPointerAddress(
64+
CodeGenFunction &CGF, const Expr *E, Address Base, llvm::Value *MemPtr,
65+
const MemberPointerType *MPT, bool IsInBounds) {
6766
ErrorUnsupportedABI(CGF, "loads of member pointers");
6867
llvm::Type *Ty =
6968
llvm::PointerType::get(CGF.getLLVMContext(), Base.getAddressSpace());

clang/lib/CodeGen/CGCXXABI.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,7 @@ class CGCXXABI {
192192
virtual llvm::Value *
193193
EmitMemberDataPointerAddress(CodeGenFunction &CGF, const Expr *E,
194194
Address Base, llvm::Value *MemPtr,
195-
const MemberPointerType *MPT);
195+
const MemberPointerType *MPT, bool IsInBounds);
196196

197197
/// Perform a derived-to-base, base-to-derived, or bitcast member
198198
/// pointer conversion.

clang/lib/CodeGen/CGClass.cpp

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -147,16 +147,13 @@ Address CodeGenFunction::LoadCXXThisAddress() {
147147
/// Emit the address of a field using a member data pointer.
148148
///
149149
/// \param E Only used for emergency diagnostics
150-
Address
151-
CodeGenFunction::EmitCXXMemberDataPointerAddress(const Expr *E, Address base,
152-
llvm::Value *memberPtr,
153-
const MemberPointerType *memberPtrType,
154-
LValueBaseInfo *BaseInfo,
155-
TBAAAccessInfo *TBAAInfo) {
150+
Address CodeGenFunction::EmitCXXMemberDataPointerAddress(
151+
const Expr *E, Address base, llvm::Value *memberPtr,
152+
const MemberPointerType *memberPtrType, bool IsInBounds,
153+
LValueBaseInfo *BaseInfo, TBAAAccessInfo *TBAAInfo) {
156154
// Ask the ABI to compute the actual address.
157-
llvm::Value *ptr =
158-
CGM.getCXXABI().EmitMemberDataPointerAddress(*this, E, base,
159-
memberPtr, memberPtrType);
155+
llvm::Value *ptr = CGM.getCXXABI().EmitMemberDataPointerAddress(
156+
*this, E, base, memberPtr, memberPtrType, IsInBounds);
160157

161158
QualType memberType = memberPtrType->getPointeeType();
162159
CharUnits memberAlign =

clang/lib/CodeGen/CGExpr.cpp

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -654,8 +654,8 @@ EmitMaterializeTemporaryExpr(const MaterializeTemporaryExpr *M) {
654654

655655
case SubobjectAdjustment::MemberPointerAdjustment: {
656656
llvm::Value *Ptr = EmitScalarExpr(Adjustment.Ptr.RHS);
657-
Object = EmitCXXMemberDataPointerAddress(E, Object, Ptr,
658-
Adjustment.Ptr.MPT);
657+
Object = EmitCXXMemberDataPointerAddress(
658+
E, Object, Ptr, Adjustment.Ptr.MPT, /*IsInBounds=*/true);
659659
break;
660660
}
661661
}
@@ -6295,9 +6295,10 @@ EmitPointerToDataMemberBinaryExpr(const BinaryOperator *E) {
62956295

62966296
LValueBaseInfo BaseInfo;
62976297
TBAAAccessInfo TBAAInfo;
6298-
Address MemberAddr =
6299-
EmitCXXMemberDataPointerAddress(E, BaseAddr, OffsetV, MPT, &BaseInfo,
6300-
&TBAAInfo);
6298+
bool IsInBounds = !getLangOpts().PointerOverflowDefined &&
6299+
!isUnderlyingBasePointerConstantNull(E->getLHS());
6300+
Address MemberAddr = EmitCXXMemberDataPointerAddress(
6301+
E, BaseAddr, OffsetV, MPT, IsInBounds, &BaseInfo, &TBAAInfo);
63016302

63026303
return MakeAddrLValue(MemberAddr, MPT->getPointeeType(), BaseInfo, TBAAInfo);
63036304
}

clang/lib/CodeGen/CodeGenFunction.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4642,7 +4642,7 @@ class CodeGenFunction : public CodeGenTypeCache {
46424642
// Compute the object pointer.
46434643
Address EmitCXXMemberDataPointerAddress(
46444644
const Expr *E, Address base, llvm::Value *memberPtr,
4645-
const MemberPointerType *memberPtrType,
4645+
const MemberPointerType *memberPtrType, bool IsInBounds,
46464646
LValueBaseInfo *BaseInfo = nullptr, TBAAAccessInfo *TBAAInfo = nullptr);
46474647
RValue EmitCXXMemberPointerCallExpr(const CXXMemberCallExpr *E,
46484648
ReturnValueSlot ReturnValue,

clang/lib/CodeGen/ItaniumCXXABI.cpp

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -130,11 +130,10 @@ class ItaniumCXXABI : public CodeGen::CGCXXABI {
130130
llvm::Value *MemFnPtr,
131131
const MemberPointerType *MPT) override;
132132

133-
llvm::Value *
134-
EmitMemberDataPointerAddress(CodeGenFunction &CGF, const Expr *E,
135-
Address Base,
136-
llvm::Value *MemPtr,
137-
const MemberPointerType *MPT) override;
133+
llvm::Value *EmitMemberDataPointerAddress(CodeGenFunction &CGF, const Expr *E,
134+
Address Base, llvm::Value *MemPtr,
135+
const MemberPointerType *MPT,
136+
bool IsInBounds) override;
138137

139138
llvm::Value *EmitMemberPointerConversion(CodeGenFunction &CGF,
140139
const CastExpr *E,
@@ -867,14 +866,16 @@ CGCallee ItaniumCXXABI::EmitLoadOfMemberFunctionPointer(
867866
/// base object.
868867
llvm::Value *ItaniumCXXABI::EmitMemberDataPointerAddress(
869868
CodeGenFunction &CGF, const Expr *E, Address Base, llvm::Value *MemPtr,
870-
const MemberPointerType *MPT) {
869+
const MemberPointerType *MPT, bool IsInBounds) {
871870
assert(MemPtr->getType() == CGM.PtrDiffTy);
872871

873872
CGBuilderTy &Builder = CGF.Builder;
874873

875-
// Apply the offset, which we assume is non-null.
876-
return Builder.CreateInBoundsGEP(CGF.Int8Ty, Base.emitRawPointer(CGF), MemPtr,
877-
"memptr.offset");
874+
// Apply the offset.
875+
llvm::Value *BaseAddr = Base.emitRawPointer(CGF);
876+
return Builder.CreateGEP(CGF.Int8Ty, BaseAddr, MemPtr, "memptr.offset",
877+
IsInBounds ? llvm::GEPNoWrapFlags::inBounds()
878+
: llvm::GEPNoWrapFlags::none());
878879
}
879880

880881
// See if it's possible to return a constant signed pointer.

clang/lib/CodeGen/MicrosoftCXXABI.cpp

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -689,10 +689,10 @@ class MicrosoftCXXABI : public CGCXXABI {
689689
llvm::Value *MemPtr,
690690
const MemberPointerType *MPT) override;
691691

692-
llvm::Value *
693-
EmitMemberDataPointerAddress(CodeGenFunction &CGF, const Expr *E,
694-
Address Base, llvm::Value *MemPtr,
695-
const MemberPointerType *MPT) override;
692+
llvm::Value *EmitMemberDataPointerAddress(CodeGenFunction &CGF, const Expr *E,
693+
Address Base, llvm::Value *MemPtr,
694+
const MemberPointerType *MPT,
695+
bool IsInBounds) override;
696696

697697
llvm::Value *EmitNonNullMemberPointerConversion(
698698
const MemberPointerType *SrcTy, const MemberPointerType *DstTy,
@@ -3240,7 +3240,7 @@ llvm::Value *MicrosoftCXXABI::AdjustVirtualBase(
32403240

32413241
llvm::Value *MicrosoftCXXABI::EmitMemberDataPointerAddress(
32423242
CodeGenFunction &CGF, const Expr *E, Address Base, llvm::Value *MemPtr,
3243-
const MemberPointerType *MPT) {
3243+
const MemberPointerType *MPT, bool IsInBounds) {
32443244
assert(MPT->isMemberDataPointer());
32453245
CGBuilderTy &Builder = CGF.Builder;
32463246
const CXXRecordDecl *RD = MPT->getMostRecentCXXRecordDecl();
@@ -3269,9 +3269,10 @@ llvm::Value *MicrosoftCXXABI::EmitMemberDataPointerAddress(
32693269
Addr = Base.emitRawPointer(CGF);
32703270
}
32713271

3272-
// Apply the offset, which we assume is non-null.
3273-
return Builder.CreateInBoundsGEP(CGF.Int8Ty, Addr, FieldOffset,
3274-
"memptr.offset");
3272+
// Apply the offset.
3273+
return Builder.CreateGEP(CGF.Int8Ty, Addr, FieldOffset, "memptr.offset",
3274+
IsInBounds ? llvm::GEPNoWrapFlags::inBounds()
3275+
: llvm::GEPNoWrapFlags::none());
32753276
}
32763277

32773278
llvm::Value *

clang/test/CodeGenCXX/microsoft-abi-member-pointers.cpp

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -964,3 +964,24 @@ static_assert(sizeof(b::d) == 16, "");
964964
static_assert(sizeof(void (a<int>::*)()) == 16, "");
965965
#endif
966966
}
967+
968+
namespace ContainerOf {
969+
using size_t = unsigned long long;
970+
971+
struct List {
972+
int data;
973+
};
974+
975+
struct Node {
976+
int data;
977+
struct List list1;
978+
struct List list2;
979+
};
980+
981+
// CHECK-LABEL: define{{.*}} ptr @"?getOwner@ContainerOf@@
982+
// CHECK: %memptr.offset = getelementptr i8, ptr null, {{.*}}
983+
Node* getOwner(List *list, List Node::*member) {
984+
size_t offset = reinterpret_cast<size_t>(&((Node*)nullptr->*member));
985+
return reinterpret_cast<Node*>(reinterpret_cast<char*>(list) - offset);
986+
}
987+
}

clang/test/CodeGenCXX/pointers-to-data-members.cpp

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,3 +265,24 @@ namespace PR47864 {
265265
struct D : B { int m; };
266266
auto x = (int B::*)&D::m;
267267
}
268+
269+
namespace ContainerOf {
270+
using size_t = unsigned long long;
271+
272+
struct List {
273+
int data;
274+
};
275+
276+
struct Node {
277+
int data;
278+
struct List list1;
279+
struct List list2;
280+
};
281+
282+
// CHECK-LABEL: define{{.*}} ptr @_ZN11ContainerOf8getOwnerEPNS_4ListEMNS_4NodeES0_
283+
// CHECK: %memptr.offset = getelementptr i8, ptr null, i64 {{.*}}
284+
Node* getOwner(List *list, List Node::*member) {
285+
size_t offset = reinterpret_cast<size_t>(&((Node*)nullptr->*member));
286+
return reinterpret_cast<Node*>(reinterpret_cast<char*>(list) - offset);
287+
}
288+
}

0 commit comments

Comments
 (0)