-
Notifications
You must be signed in to change notification settings - Fork 12.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Clang][CodeGen] Fix use of CXXThisValue with StrictVTablePointers #68169
Conversation
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-codegen ChangesWhen emitting non-virtual base initializers for the constructor prologue, With this fix, we always launder the original CXXThisValue. This fixes #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. Full diff: https://github.com/llvm/llvm-project/pull/68169.diff 2 Files Affected:
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
new file mode 100644
index 000000000000000..692ca23a69abe6a
--- /dev/null
+++ b/clang/test/CodeGenCXX/strict-vtable-pointers-GH67937.cpp
@@ -0,0 +1,22 @@
+// 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
+
+struct A {
+ virtual ~A();
+};
+struct B : virtual A {};
+class C : B {};
+C foo;
+
+// CHECK-LABEL: define {{.*}} @"??0C@@QEAA@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@XZ"(ptr {{.*}} %2) #2
+// CHECK-NEXT: br label %ctor.skip_vbases
+// CHECK-EMPTY:
+// CHECK-NEXT: ctor.skip_vbases:
+// CHECK-NEXT: %3 = call ptr @llvm.launder.invariant.group.p0(ptr %this1)
+// CHECK-NEXT: %call3 = call noundef ptr @"??0B@@QEAA@XZ"(ptr {{.*}} %3, i32 noundef 0) #2
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was going to delegate the review to @aeubanks, who was the last one I'm aware of to touch the strict vptr feature, but I think this is a pretty straightforward fix.
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 llvm#67937
Local branch amd-gfx ab27814 Merged main:7539bcf994ed into amd-gfx:0e8798f5c1bd Remote branch main 5099dc3 [Clang][CodeGen] Fix use of CXXThisValue with StrictVTablePointers (llvm#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 #67937