[clang] [Clang][CodeGen] Fix use of CXXThisValue with StrictVTablePointers (PR #68169)
Matheus Izvekov via cfe-commits
cfe-commits at lists.llvm.org
Tue Oct 3 18:04:16 PDT 2023
https://github.com/mizvekov created https://github.com/llvm/llvm-project/pull/68169
When emitting non-virtual base initializers for the constructor prologue,
we would potentially use a re-laundered this pointer value from a
previous block, which subsequently would not dominate this use.
With this fix, we always launder the original CXXThisValue.
This fixes https://github.com/llvm/llvm-project/issues/67937
Note: First commit just adds the test, which will be fixed by the second commit. I will land the test separately after the first round.
>From 456b3d4e38fb80c599959d6b1ff5c93d94232394 Mon Sep 17 00:00:00 2001
From: Matheus Izvekov <mizvekov at gmail.com>
Date: Wed, 4 Oct 2023 02:34:31 +0200
Subject: [PATCH 1/2] [Clang][CodeGen][NFC] Add (broken) test case for GH67937
This adds a test case for yet unfixed https://github.com/llvm/llvm-project/issues/67937
---
.../strict-vtable-pointers-GH67937.cpp | 29 +++++++++++++++++++
1 file changed, 29 insertions(+)
create mode 100644 clang/test/CodeGenCXX/strict-vtable-pointers-GH67937.cpp
diff --git a/clang/test/CodeGenCXX/strict-vtable-pointers-GH67937.cpp b/clang/test/CodeGenCXX/strict-vtable-pointers-GH67937.cpp
new file mode 100644
index 000000000000000..6fb1e68c288b181
--- /dev/null
+++ b/clang/test/CodeGenCXX/strict-vtable-pointers-GH67937.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 %s -I%S -triple=x86_64-pc-windows-msvc -fstrict-vtable-pointers -disable-llvm-passes -disable-llvm-verifier -O1 -emit-llvm -o %t.ll
+// RUN: FileCheck %s < %t.ll
+// RUN: not llvm-as < %t.ll -o /dev/null 2>&1 | FileCheck %s --check-prefix=CHECK-VERIFIER
+
+struct A {
+ virtual ~A();
+};
+struct B : virtual A {};
+class C : B {};
+C foo;
+
+// FIXME: This is not supposed to generate invalid IR!
+// CHECK-VERIFIER: Instruction does not dominate all uses!
+// CHECK-VERIFIER-NEXT: %1 = call ptr @llvm.launder.invariant.group.p0(ptr %this1)
+// CHECK-VERIFIER-NEXT: %3 = call ptr @llvm.launder.invariant.group.p0(ptr %1)
+
+// CHECK-LABEL: define {{.*}} @"??0C@@QEAA at XZ"(ptr {{.*}} %this, i32 {{.*}} %is_most_derived)
+// CHECK: ctor.init_vbases:
+// CHECK-NEXT: %0 = getelementptr inbounds i8, ptr %this1, i64 0
+// CHECK-NEXT: store ptr @"??_8C@@7B@", ptr %0
+// CHECK-NEXT: %1 = call ptr @llvm.launder.invariant.group.p0(ptr %this1)
+// CHECK-NEXT: %2 = getelementptr inbounds i8, ptr %1, i64 8
+// CHECK-NEXT: %call = call noundef ptr @"??0A@@QEAA at XZ"(ptr {{.*}} %2) #2
+// CHECK-NEXT: br label %ctor.skip_vbases
+// CHECK-EMPTY:
+// CHECK-NEXT: ctor.skip_vbases:
+// FIXME: Should be using '%this1' instead of %1 below.
+// CHECK-NEXT: %3 = call ptr @llvm.launder.invariant.group.p0(ptr %1)
+// CHECK-NEXT: %call3 = call noundef ptr @"??0B@@QEAA at XZ"(ptr {{.*}} %3, i32 noundef 0) #2
>From 4073aca154fe55e56b0025509ce85690eba061df Mon Sep 17 00:00:00 2001
From: Matheus Izvekov <mizvekov at gmail.com>
Date: Wed, 4 Oct 2023 02:40:25 +0200
Subject: [PATCH 2/2] [Clang][CodeGen] Fix use of CXXThisValue with
StrictVTablePointers
When emitting non-virtual base initializers for the constructor prologue,
we would potentially use a re-laundered this pointer value from a
previous block, which subsequently would not dominate this use.
With this fix, we always launder the original CXXThisValue.
This fixes https://github.com/llvm/llvm-project/issues/67937
---
clang/lib/CodeGen/CGClass.cpp | 7 +++----
.../CodeGenCXX/strict-vtable-pointers-GH67937.cpp | 11 ++---------
2 files changed, 5 insertions(+), 13 deletions(-)
diff --git a/clang/lib/CodeGen/CGClass.cpp b/clang/lib/CodeGen/CGClass.cpp
index 57a424c6f176c47..d18f186ce5b4157 100644
--- a/clang/lib/CodeGen/CGClass.cpp
+++ b/clang/lib/CodeGen/CGClass.cpp
@@ -28,6 +28,7 @@
#include "clang/CodeGen/CGFunctionInfo.h"
#include "llvm/IR/Intrinsics.h"
#include "llvm/IR/Metadata.h"
+#include "llvm/Support/SaveAndRestore.h"
#include "llvm/Transforms/Utils/SanitizerStats.h"
#include <optional>
@@ -1291,10 +1292,10 @@ void CodeGenFunction::EmitCtorPrologue(const CXXConstructorDecl *CD,
assert(BaseCtorContinueBB);
}
- llvm::Value *const OldThis = CXXThisValue;
for (; B != E && (*B)->isBaseInitializer() && (*B)->isBaseVirtual(); B++) {
if (!ConstructVBases)
continue;
+ SaveAndRestore ThisRAII(CXXThisValue);
if (CGM.getCodeGenOpts().StrictVTablePointers &&
CGM.getCodeGenOpts().OptimizationLevel > 0 &&
isInitializerOfDynamicClass(*B))
@@ -1311,7 +1312,7 @@ void CodeGenFunction::EmitCtorPrologue(const CXXConstructorDecl *CD,
// Then, non-virtual base initializers.
for (; B != E && (*B)->isBaseInitializer(); B++) {
assert(!(*B)->isBaseVirtual());
-
+ SaveAndRestore ThisRAII(CXXThisValue);
if (CGM.getCodeGenOpts().StrictVTablePointers &&
CGM.getCodeGenOpts().OptimizationLevel > 0 &&
isInitializerOfDynamicClass(*B))
@@ -1319,8 +1320,6 @@ void CodeGenFunction::EmitCtorPrologue(const CXXConstructorDecl *CD,
EmitBaseInitializer(*this, ClassDecl, *B);
}
- CXXThisValue = OldThis;
-
InitializeVTablePointers(ClassDecl);
// And finally, initialize class members.
diff --git a/clang/test/CodeGenCXX/strict-vtable-pointers-GH67937.cpp b/clang/test/CodeGenCXX/strict-vtable-pointers-GH67937.cpp
index 6fb1e68c288b181..692ca23a69abe6a 100644
--- a/clang/test/CodeGenCXX/strict-vtable-pointers-GH67937.cpp
+++ b/clang/test/CodeGenCXX/strict-vtable-pointers-GH67937.cpp
@@ -1,6 +1,5 @@
-// RUN: %clang_cc1 %s -I%S -triple=x86_64-pc-windows-msvc -fstrict-vtable-pointers -disable-llvm-passes -disable-llvm-verifier -O1 -emit-llvm -o %t.ll
+// RUN: %clang_cc1 %s -I%S -triple=x86_64-pc-windows-msvc -fstrict-vtable-pointers -disable-llvm-passes -O1 -emit-llvm -o %t.ll
// RUN: FileCheck %s < %t.ll
-// RUN: not llvm-as < %t.ll -o /dev/null 2>&1 | FileCheck %s --check-prefix=CHECK-VERIFIER
struct A {
virtual ~A();
@@ -9,11 +8,6 @@ struct B : virtual A {};
class C : B {};
C foo;
-// FIXME: This is not supposed to generate invalid IR!
-// CHECK-VERIFIER: Instruction does not dominate all uses!
-// CHECK-VERIFIER-NEXT: %1 = call ptr @llvm.launder.invariant.group.p0(ptr %this1)
-// CHECK-VERIFIER-NEXT: %3 = call ptr @llvm.launder.invariant.group.p0(ptr %1)
-
// CHECK-LABEL: define {{.*}} @"??0C@@QEAA at XZ"(ptr {{.*}} %this, i32 {{.*}} %is_most_derived)
// CHECK: ctor.init_vbases:
// CHECK-NEXT: %0 = getelementptr inbounds i8, ptr %this1, i64 0
@@ -24,6 +18,5 @@ C foo;
// CHECK-NEXT: br label %ctor.skip_vbases
// CHECK-EMPTY:
// CHECK-NEXT: ctor.skip_vbases:
-// FIXME: Should be using '%this1' instead of %1 below.
-// CHECK-NEXT: %3 = call ptr @llvm.launder.invariant.group.p0(ptr %1)
+// CHECK-NEXT: %3 = call ptr @llvm.launder.invariant.group.p0(ptr %this1)
// CHECK-NEXT: %call3 = call noundef ptr @"??0B@@QEAA at XZ"(ptr {{.*}} %3, i32 noundef 0) #2
More information about the cfe-commits
mailing list