[clang] [Clang][CodeGen] Do not set inbounds flag in `EmitMemberDataPointerAddress` when the base pointer is null (PR #130952)

2025-04-10 Thread LLVM Continuous Integration via cfe-commits

llvm-ci wrote:

LLVM Buildbot has detected a new failure on builder 
`openmp-offload-amdgpu-runtime-2` running on `rocm-worker-hw-02` while building 
`clang` at step 5 "compile-openmp".

Full details are available at: 
https://lab.llvm.org/buildbot/#/builders/10/builds/3246


Here is the relevant piece of the build log for the reference

```
Step 5 (compile-openmp) failure: build (failure)
...
0.148 [390/130/177] Generating header ctype.h from 
/home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.src/runtimes/../libc/include/ctype.yaml
0.148 [389/130/178] Generating 
/home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.build/include/amdgcn-amd-amdhsa/llvm-libc-decls/locale.h
0.148 [389/129/179] Generating header locale.h from 
/home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.src/runtimes/../libc/include/locale.yaml
0.149 [387/130/180] Generating 
/home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.build/include/amdgcn-amd-amdhsa/llvm-libc-decls/uchar.h
0.150 [386/130/181] Generating header uchar.h from 
/home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.src/runtimes/../libc/include/uchar.yaml
0.150 [385/130/182] Generating 
/home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.build/include/amdgcn-amd-amdhsa/llvm-libc-decls/time.h
0.150 [384/130/183] Generating header time.h from 
/home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.src/runtimes/../libc/include/time.yaml
0.151 [383/130/184] Building CXX object 
libc/src/math/amdgpu/CMakeFiles/libc.src.math.amdgpu.lgamma_r.dir/lgamma_r.cpp.o
0.151 [382/130/185] Generating 
/home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.build/include/amdgcn-amd-amdhsa/llvm-libc-decls/string.h
0.152 [381/130/186] Building CXX object 
libc/src/stdlib/CMakeFiles/libc.src.stdlib.memalignment.dir/memalignment.cpp.o
FAILED: 
libc/src/stdlib/CMakeFiles/libc.src.stdlib.memalignment.dir/memalignment.cpp.o 
/home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.build/./bin/clang++ 
--target=amdgcn-amd-amdhsa -DLIBC_NAMESPACE=__llvm_libc_21_0_0_git 
-D__LIBC_USE_FLOAT16_CONVERSION 
-I/home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.src/libc -isystem 
/home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.build/include/amdgcn-amd-amdhsa
 -O3 -DNDEBUG --target=amdgcn-amd-amdhsa 
-DLIBC_QSORT_IMPL=LIBC_QSORT_QUICK_SORT -DLIBC_ADD_NULL_CHECKS 
"-DLIBC_MATH=(LIBC_MATH_SKIP_ACCURATE_PASS | LIBC_MATH_SMALL_TABLES | 
LIBC_MATH_NO_ERRNO | LIBC_MATH_NO_EXCEPT)" -fpie -DLIBC_FULL_BUILD -nostdlibinc 
-ffixed-point -fno-exceptions -fno-lax-vector-conversions -fno-unwind-tables 
-fno-asynchronous-unwind-tables -fno-rtti -ftrivial-auto-var-init=pattern 
-fno-omit-frame-pointer -Wall -Wextra -Werror -Wconversion -Wno-sign-conversion 
-Wdeprecated -Wno-c99-extensions -Wno-gnu-imaginary-constant -Wno-pedantic 
-Wimplicit-fallthrough -Wwrite-strings -Wextra-semi -Wnewline-eof 
-Wnonportable-system-include-path -Wstrict-prototypes -Wthread-safety 
-Wglobal-constructors -nogpulib -fvisibility=hidden -fconvergent-functions 
-flto -Wno-multi-gpu -Xclang -mcode-object-version=none 
-DLIBC_COPT_PUBLIC_PACKAGING -UNDEBUG -MD -MT 
libc/src/stdlib/CMakeFiles/libc.src.stdlib.memalignment.dir/memalignment.cpp.o 
-MF 
libc/src/stdlib/CMakeFiles/libc.src.stdlib.memalignment.dir/memalignment.cpp.o.d
 -o 
libc/src/stdlib/CMakeFiles/libc.src.stdlib.memalignment.dir/memalignment.cpp.o 
-c 
/home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.src/libc/src/stdlib/memalignment.cpp
/home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.src/libc/src/stdlib/memalignment.cpp:20:3:
 error: unknown type name 'uintptr_t'
   20 |   uintptr_t addr = reinterpret_cast(p);
  |   ^
/home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.src/libc/src/stdlib/memalignment.cpp:20:37:
 error: unknown type name 'uintptr_t'
   20 |   uintptr_t addr = reinterpret_cast(p);
  | ^
2 errors generated.
0.152 [381/129/187] Generating 
/home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.build/include/amdgcn-amd-amdhsa/llvm-libc-decls/ctype.h
0.152 [381/128/188] Generating 
/home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.build/include/amdgcn-amd-amdhsa/llvm-libc-decls/stdlib.h
0.152 [381/127/189] Generating header stdlib.h from 
/home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.src/runtimes/../libc/include/stdlib.yaml
0.152 [381/126/190] Generating 
/home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.build/include/amdgcn-amd-amdhsa/llvm-libc-decls/stdio.h
0.152 [381/125/191] Generating 
/home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.build/include/amdgcn-amd-amdhsa/llvm-libc-decls/signal.h
0.154 [381/124/192] Building CXX object 
libc/src/strings/CMakeFiles/libc.src.strings.bcopy.dir/bcopy.cpp.o
0.157 [381/123/193] Generating header signal.h from 
/home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.src/runtimes/../libc/include/signal.yaml
0.165 [38

[clang] [Clang][CodeGen] Do not set inbounds flag in `EmitMemberDataPointerAddress` when the base pointer is null (PR #130952)

2025-04-10 Thread Yingwei Zheng via cfe-commits

https://github.com/dtcxzyw closed 
https://github.com/llvm/llvm-project/pull/130952
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang][CodeGen] Do not set inbounds flag in `EmitMemberDataPointerAddress` when the base pointer is null (PR #130952)

2025-04-10 Thread Yingwei Zheng via cfe-commits

https://github.com/dtcxzyw updated 
https://github.com/llvm/llvm-project/pull/130952

>From 9a9c7cf2eff8c740789e9702a190af6dbd3f5014 Mon Sep 17 00:00:00 2001
From: Yingwei Zheng 
Date: Wed, 12 Mar 2025 20:17:28 +0800
Subject: [PATCH 1/3] [Clang][CodeGen] Do not set inbounds in
 `EmitMemberDataPointerAddress` when the base pointer is null

---
 clang/lib/CodeGen/ItaniumCXXABI.cpp   | 11 +++---
 clang/lib/CodeGen/MicrosoftCXXABI.cpp | 10 ++---
 .../microsoft-abi-member-pointers.cpp | 21 +++
 .../CodeGenCXX/pointers-to-data-members.cpp   | 21 +++
 4 files changed, 57 insertions(+), 6 deletions(-)

diff --git a/clang/lib/CodeGen/ItaniumCXXABI.cpp 
b/clang/lib/CodeGen/ItaniumCXXABI.cpp
index 2822d526a54b0..400510d7056b3 100644
--- a/clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ b/clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -872,9 +872,14 @@ llvm::Value *ItaniumCXXABI::EmitMemberDataPointerAddress(
 
   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.
+  // Specially, we don't add inbounds flags if the base pointer is a constant
+  // null. This is a workaround for old-style container_of macros.
+  llvm::Value *BaseAddr = Base.emitRawPointer(CGF);
+  return Builder.CreateGEP(CGF.Int8Ty, BaseAddr, MemPtr, "memptr.offset",
+   isa(BaseAddr)
+   ? llvm::GEPNoWrapFlags::none()
+   : llvm::GEPNoWrapFlags::inBounds());
 }
 
 // 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..c32ca3be3405e 100644
--- a/clang/lib/CodeGen/MicrosoftCXXABI.cpp
+++ b/clang/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -3269,9 +3269,13 @@ 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.
+  // Specially, we don't add inbounds flags if the base pointer is a constant
+  // null. This is a workaround for old-style container_of macros.
+  return Builder.CreateGEP(CGF.Int8Ty, Addr, FieldOffset, "memptr.offset",
+   isa(Addr)
+   ? llvm::GEPNoWrapFlags::none()
+   : llvm::GEPNoWrapFlags::inBounds());
 }
 
 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::*)()) == 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(&((Node*)nullptr->*member));
+return reinterpret_cast(reinterpret_cast(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(&((Node*)nullptr->*member));
+return reinterpret_cast(reinterpret_cast(list) - offset);
+  }
+}

>From af9661089cfb2954ced2f73add29f1e2070a1140 Mon Sep 17 00:00:00 2001
From: Yingwei Zheng 
Date: Thu, 10 Apr 2025 12:19:15 +0800
Subject: [PATCH 2/3] [Clang][CodeGen] Address review comments.

---
 clang/lib/CodeGen/CGCXXABI.cpp|  7 +++
 clang/lib/CodeGen/CGCXXABI.h  |  2 +-
 clang/lib/CodeGen/CGClass.cpp | 15 ++-
 clang/lib/CodeGen/CGExpr.cpp  | 11 ++-
 clang/lib/CodeGen/CodeGenFunction.h   |  2 +-

[clang] [Clang][CodeGen] Do not set inbounds flag in `EmitMemberDataPointerAddress` when the base pointer is null (PR #130952)

2025-04-10 Thread Eli Friedman via cfe-commits


@@ -660,8 +660,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);

efriedma-quic wrote:

This sent me down a really deep rabbit-hole to figure out when this code 
actually runs... apparently, in C++98 mode, we delay creating temporaries for 
things that would be xvalues in newer standard versions, and then we try to fix 
things up later using skipRValueSubobjectAdjustments().  We don't have an 
in-tree tests for this particular codepath.  The whole thing is really ugly, 
and we should really come up with a better solution...

In any case, the base object should be an alloca here, so inbounds should be 
fine.

https://github.com/llvm/llvm-project/pull/130952
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang][CodeGen] Do not set inbounds flag in `EmitMemberDataPointerAddress` when the base pointer is null (PR #130952)

2025-04-10 Thread Eli Friedman via cfe-commits

https://github.com/efriedma-quic edited 
https://github.com/llvm/llvm-project/pull/130952
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang][CodeGen] Do not set inbounds flag in `EmitMemberDataPointerAddress` when the base pointer is null (PR #130952)

2025-04-10 Thread Eli Friedman via cfe-commits

https://github.com/efriedma-quic approved this pull request.

LGTM

https://github.com/llvm/llvm-project/pull/130952
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang][CodeGen] Do not set inbounds flag in `EmitMemberDataPointerAddress` when the base pointer is null (PR #130952)

2025-04-09 Thread Yingwei Zheng via cfe-commits

https://github.com/dtcxzyw updated 
https://github.com/llvm/llvm-project/pull/130952

>From 0f6ff605da3cbadc5311d4bf6c08fe98970a69c3 Mon Sep 17 00:00:00 2001
From: Yingwei Zheng 
Date: Thu, 10 Apr 2025 09:54:33 +0800
Subject: [PATCH 1/5] [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(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(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 CodeGenFuncti

[clang] [Clang][CodeGen] Do not set inbounds flag in `EmitMemberDataPointerAddress` when the base pointer is null (PR #130952)

2025-03-14 Thread via cfe-commits

llvmbot wrote:




@llvm/pr-subscribers-clang

Author: Yingwei Zheng (dtcxzyw)


Changes

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


---
Full diff: https://github.com/llvm/llvm-project/pull/130952.diff


4 Files Affected:

- (modified) clang/lib/CodeGen/ItaniumCXXABI.cpp (+8-3) 
- (modified) clang/lib/CodeGen/MicrosoftCXXABI.cpp (+7-3) 
- (modified) clang/test/CodeGenCXX/microsoft-abi-member-pointers.cpp (+21) 
- (modified) clang/test/CodeGenCXX/pointers-to-data-members.cpp (+21) 


``diff
diff --git a/clang/lib/CodeGen/ItaniumCXXABI.cpp 
b/clang/lib/CodeGen/ItaniumCXXABI.cpp
index b145da0f0ec09..b2eaea683ebe7 100644
--- a/clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ b/clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -872,9 +872,14 @@ llvm::Value *ItaniumCXXABI::EmitMemberDataPointerAddress(
 
   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.
+  // Specially, we don't add inbounds flags if the base pointer is a constant
+  // null. This is a workaround for old-style container_of macros.
+  llvm::Value *BaseAddr = Base.emitRawPointer(CGF);
+  return Builder.CreateGEP(CGF.Int8Ty, BaseAddr, MemPtr, "memptr.offset",
+   isa(BaseAddr)
+   ? llvm::GEPNoWrapFlags::none()
+   : llvm::GEPNoWrapFlags::inBounds());
 }
 
 // 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 4b55fc3f17bd7..3501b2b2fdca8 100644
--- a/clang/lib/CodeGen/MicrosoftCXXABI.cpp
+++ b/clang/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -3265,9 +3265,13 @@ 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.
+  // Specially, we don't add inbounds flags if the base pointer is a constant
+  // null. This is a workaround for old-style container_of macros.
+  return Builder.CreateGEP(CGF.Int8Ty, Addr, FieldOffset, "memptr.offset",
+   isa(Addr)
+   ? llvm::GEPNoWrapFlags::none()
+   : llvm::GEPNoWrapFlags::inBounds());
 }
 
 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::*)()) == 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(&((Node*)nullptr->*member));
+return reinterpret_cast(reinterpret_cast(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(&((Node*)nullptr->*member));
+return reinterpret_cast(reinterpret_cast(list) - offset);
+  }
+}

``




https://github.com/llvm/llvm-project/pull/130952
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mai

[clang] [Clang][CodeGen] Do not set inbounds flag in `EmitMemberDataPointerAddress` when the base pointer is null (PR #130952)

2025-03-12 Thread Yingwei Zheng via cfe-commits

https://github.com/dtcxzyw created 
https://github.com/llvm/llvm-project/pull/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


>From 824a5e7551437233ed6a7990d43572b8c50a5397 Mon Sep 17 00:00:00 2001
From: Yingwei Zheng 
Date: Wed, 12 Mar 2025 20:17:28 +0800
Subject: [PATCH] [Clang][CodeGen] Do not set inbounds in
 `EmitMemberDataPointerAddress` when the base pointer is null

---
 clang/lib/CodeGen/ItaniumCXXABI.cpp   | 11 +++---
 clang/lib/CodeGen/MicrosoftCXXABI.cpp | 10 ++---
 .../microsoft-abi-member-pointers.cpp | 21 +++
 .../CodeGenCXX/pointers-to-data-members.cpp   | 21 +++
 4 files changed, 57 insertions(+), 6 deletions(-)

diff --git a/clang/lib/CodeGen/ItaniumCXXABI.cpp 
b/clang/lib/CodeGen/ItaniumCXXABI.cpp
index b145da0f0ec09..b2eaea683ebe7 100644
--- a/clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ b/clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -872,9 +872,14 @@ llvm::Value *ItaniumCXXABI::EmitMemberDataPointerAddress(
 
   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.
+  // Specially, we don't add inbounds flags if the base pointer is a constant
+  // null. This is a workaround for old-style container_of macros.
+  llvm::Value *BaseAddr = Base.emitRawPointer(CGF);
+  return Builder.CreateGEP(CGF.Int8Ty, BaseAddr, MemPtr, "memptr.offset",
+   isa(BaseAddr)
+   ? llvm::GEPNoWrapFlags::none()
+   : llvm::GEPNoWrapFlags::inBounds());
 }
 
 // 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 4b55fc3f17bd7..3501b2b2fdca8 100644
--- a/clang/lib/CodeGen/MicrosoftCXXABI.cpp
+++ b/clang/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -3265,9 +3265,13 @@ 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.
+  // Specially, we don't add inbounds flags if the base pointer is a constant
+  // null. This is a workaround for old-style container_of macros.
+  return Builder.CreateGEP(CGF.Int8Ty, Addr, FieldOffset, "memptr.offset",
+   isa(Addr)
+   ? llvm::GEPNoWrapFlags::none()
+   : llvm::GEPNoWrapFlags::inBounds());
 }
 
 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::*)()) == 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(&((Node*)nullptr->*member));
+return reinterpret_cast(reinterpret_cast(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(&((Node*)nullptr->*member));
+return reinterpret_cast(reinterpret_cast(list) - offset);
+  }

[clang] [Clang][CodeGen] Do not set inbounds flag in `EmitMemberDataPointerAddress` when the base pointer is null (PR #130952)

2025-03-12 Thread via cfe-commits

llvmbot wrote:




@llvm/pr-subscribers-clang-codegen

Author: Yingwei Zheng (dtcxzyw)


Changes

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


---
Full diff: https://github.com/llvm/llvm-project/pull/130952.diff


4 Files Affected:

- (modified) clang/lib/CodeGen/ItaniumCXXABI.cpp (+8-3) 
- (modified) clang/lib/CodeGen/MicrosoftCXXABI.cpp (+7-3) 
- (modified) clang/test/CodeGenCXX/microsoft-abi-member-pointers.cpp (+21) 
- (modified) clang/test/CodeGenCXX/pointers-to-data-members.cpp (+21) 


``diff
diff --git a/clang/lib/CodeGen/ItaniumCXXABI.cpp 
b/clang/lib/CodeGen/ItaniumCXXABI.cpp
index b145da0f0ec09..b2eaea683ebe7 100644
--- a/clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ b/clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -872,9 +872,14 @@ llvm::Value *ItaniumCXXABI::EmitMemberDataPointerAddress(
 
   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.
+  // Specially, we don't add inbounds flags if the base pointer is a constant
+  // null. This is a workaround for old-style container_of macros.
+  llvm::Value *BaseAddr = Base.emitRawPointer(CGF);
+  return Builder.CreateGEP(CGF.Int8Ty, BaseAddr, MemPtr, "memptr.offset",
+   isa(BaseAddr)
+   ? llvm::GEPNoWrapFlags::none()
+   : llvm::GEPNoWrapFlags::inBounds());
 }
 
 // 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 4b55fc3f17bd7..3501b2b2fdca8 100644
--- a/clang/lib/CodeGen/MicrosoftCXXABI.cpp
+++ b/clang/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -3265,9 +3265,13 @@ 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.
+  // Specially, we don't add inbounds flags if the base pointer is a constant
+  // null. This is a workaround for old-style container_of macros.
+  return Builder.CreateGEP(CGF.Int8Ty, Addr, FieldOffset, "memptr.offset",
+   isa(Addr)
+   ? llvm::GEPNoWrapFlags::none()
+   : llvm::GEPNoWrapFlags::inBounds());
 }
 
 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::*)()) == 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(&((Node*)nullptr->*member));
+return reinterpret_cast(reinterpret_cast(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(&((Node*)nullptr->*member));
+return reinterpret_cast(reinterpret_cast(list) - offset);
+  }
+}

``




https://github.com/llvm/llvm-project/pull/130952
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi