Fznamznon created this revision.
Herald added a project: All.
Fznamznon requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

For simple derived type ConstantEmitter returns a struct of the same
size but different type which is then stored field-by-field into memory
via pointer to derived type. In case base type has more fields than derived,
the incorrect GEP is emitted. So, just cast pointer to derived type to
appropriate type with enough fields.

Fixes #60166


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D142534

Files:
  clang/lib/CodeGen/CGExprAgg.cpp
  clang/test/CodeGenCXX/cxx20-consteval-crash.cpp


Index: clang/test/CodeGenCXX/cxx20-consteval-crash.cpp
===================================================================
--- clang/test/CodeGenCXX/cxx20-consteval-crash.cpp
+++ clang/test/CodeGenCXX/cxx20-consteval-crash.cpp
@@ -92,3 +92,27 @@
 }
 } // namespace Issue55065
 
+namespace Issue60166 {
+
+struct Base {
+   void* one = nullptr;
+   void* two = nullptr;
+};
+
+struct Derived : Base {
+   void* three = nullptr;
+   consteval Derived() = default;
+};
+
+void method() {
+  // CHECK: %agg.tmp.ensured = alloca %"struct.Issue60166::Derived"
+  // CHECK: %0 = getelementptr inbounds { ptr, ptr, ptr }, ptr 
%agg.tmp.ensured, i32 0, i32 0
+  // CHECK: store ptr null, ptr %0, align 8
+  // CHECK: %1 = getelementptr inbounds { ptr, ptr, ptr }, ptr 
%agg.tmp.ensured, i32 0, i32 1
+  // CHECK: store ptr null, ptr %1, align 8
+  // CHECK: %2 = getelementptr inbounds { ptr, ptr, ptr }, ptr 
%agg.tmp.ensured, i32 0, i32 2
+  // CHECK: store ptr null, ptr %2, align 8
+   (void)Derived();
+}
+
+} // namespace Issue60166
Index: clang/lib/CodeGen/CGExprAgg.cpp
===================================================================
--- clang/lib/CodeGen/CGExprAgg.cpp
+++ clang/lib/CodeGen/CGExprAgg.cpp
@@ -131,7 +131,14 @@
     EnsureDest(E->getType());
 
     if (llvm::Value *Result = ConstantEmitter(CGF).tryEmitConstantExpr(E)) {
-      CGF.EmitAggregateStore(Result, Dest.getAddress(),
+      Address StoreDest = Dest.getAddress();
+      // The emitted value is guaranteed to have the same size as the
+      // destination but can have a different type. Just do a bitcast in this
+      // case to avoid incorrect GEPs.
+      if (Result->getType() != StoreDest.getType())
+        StoreDest =
+            CGF.Builder.CreateElementBitCast(StoreDest, Result->getType());
+      CGF.EmitAggregateStore(Result, StoreDest,
                              E->getType().isVolatileQualified());
       return;
     }


Index: clang/test/CodeGenCXX/cxx20-consteval-crash.cpp
===================================================================
--- clang/test/CodeGenCXX/cxx20-consteval-crash.cpp
+++ clang/test/CodeGenCXX/cxx20-consteval-crash.cpp
@@ -92,3 +92,27 @@
 }
 } // namespace Issue55065
 
+namespace Issue60166 {
+
+struct Base {
+   void* one = nullptr;
+   void* two = nullptr;
+};
+
+struct Derived : Base {
+   void* three = nullptr;
+   consteval Derived() = default;
+};
+
+void method() {
+  // CHECK: %agg.tmp.ensured = alloca %"struct.Issue60166::Derived"
+  // CHECK: %0 = getelementptr inbounds { ptr, ptr, ptr }, ptr %agg.tmp.ensured, i32 0, i32 0
+  // CHECK: store ptr null, ptr %0, align 8
+  // CHECK: %1 = getelementptr inbounds { ptr, ptr, ptr }, ptr %agg.tmp.ensured, i32 0, i32 1
+  // CHECK: store ptr null, ptr %1, align 8
+  // CHECK: %2 = getelementptr inbounds { ptr, ptr, ptr }, ptr %agg.tmp.ensured, i32 0, i32 2
+  // CHECK: store ptr null, ptr %2, align 8
+   (void)Derived();
+}
+
+} // namespace Issue60166
Index: clang/lib/CodeGen/CGExprAgg.cpp
===================================================================
--- clang/lib/CodeGen/CGExprAgg.cpp
+++ clang/lib/CodeGen/CGExprAgg.cpp
@@ -131,7 +131,14 @@
     EnsureDest(E->getType());
 
     if (llvm::Value *Result = ConstantEmitter(CGF).tryEmitConstantExpr(E)) {
-      CGF.EmitAggregateStore(Result, Dest.getAddress(),
+      Address StoreDest = Dest.getAddress();
+      // The emitted value is guaranteed to have the same size as the
+      // destination but can have a different type. Just do a bitcast in this
+      // case to avoid incorrect GEPs.
+      if (Result->getType() != StoreDest.getType())
+        StoreDest =
+            CGF.Builder.CreateElementBitCast(StoreDest, Result->getType());
+      CGF.EmitAggregateStore(Result, StoreDest,
                              E->getType().isVolatileQualified());
       return;
     }
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to