[PATCH] Use GEPs correctly when adjusting "this" in MicrosoftCXXABI
Timur Iskhodzhanov
timurrrr at google.com
Fri Oct 18 08:46:33 PDT 2013
Hi rnk,
Please see
http://llvm.org/docs/LangRef.html#id208
http://llvm.org/docs/GetElementPtr.html#what-happens-if-an-array-index-is-out-of-bounds
Looks like we had an UB :)
I'll fix the GEP UB in the thunk emission code as part of my next vtordisp patch.
Also, force the offsets of these GEPs to be 32-bit as this is the range of this adjustment even on x64.
http://llvm-reviews.chandlerc.com/D1977
Files:
lib/CodeGen/MicrosoftCXXABI.cpp
test/CodeGenCXX/microsoft-abi-exceptions.cpp
test/CodeGenCXX/microsoft-abi-multiple-nonvirtual-inheritance.cpp
test/CodeGenCXX/microsoft-abi-virtual-inheritance.cpp
Index: lib/CodeGen/MicrosoftCXXABI.cpp
===================================================================
--- lib/CodeGen/MicrosoftCXXABI.cpp
+++ lib/CodeGen/MicrosoftCXXABI.cpp
@@ -632,8 +632,16 @@
if (!StaticOffset.isZero()) {
assert(StaticOffset.isPositive());
This = CGF.Builder.CreateBitCast(This, charPtrTy);
- This = CGF.Builder
- .CreateConstInBoundsGEP1_64(This, StaticOffset.getQuantity());
+ if (ML.VBase) {
+ // Non-virtual adjustment might result in a pointer outside the allocated
+ // object, e.g. if the final overrider class is laid out after the virtual
+ // base that declares a method in the most derived class.
+ // FIXME: Update the code that emits this adjustment in thunks prologues.
+ This = CGF.Builder.CreateConstGEP1_32(This, StaticOffset.getQuantity());
+ } else {
+ This = CGF.Builder.CreateConstInBoundsGEP1_32(This,
+ StaticOffset.getQuantity());
+ }
}
return This;
}
@@ -713,7 +721,8 @@
This = CGF.Builder.CreateBitCast(This, charPtrTy);
assert(Adjustment.isPositive());
- This = CGF.Builder.CreateConstGEP1_64(This, -Adjustment.getQuantity());
+ This =
+ CGF.Builder.CreateConstInBoundsGEP1_32(This, -Adjustment.getQuantity());
return CGF.Builder.CreateBitCast(This, thisTy);
}
Index: test/CodeGenCXX/microsoft-abi-exceptions.cpp
===================================================================
--- test/CodeGenCXX/microsoft-abi-exceptions.cpp
+++ test/CodeGenCXX/microsoft-abi-exceptions.cpp
@@ -141,7 +141,7 @@
//
// We shouldn't do any vbptr loads, just constant GEPs.
// WIN32-NOT: load
-// WIN32: getelementptr inbounds i8* %{{.*}}, i64 4
+// WIN32: getelementptr i8* %{{.*}}, i32 4
// WIN32-NOT: load
// WIN32: bitcast i8* %{{.*}} to %"struct.crash_on_partial_destroy::B"*
// WIN32: invoke x86_thiscallcc void @"\01??1B at crash_on_partial_destroy@@UAE at XZ"
Index: test/CodeGenCXX/microsoft-abi-multiple-nonvirtual-inheritance.cpp
===================================================================
--- test/CodeGenCXX/microsoft-abi-multiple-nonvirtual-inheritance.cpp
+++ test/CodeGenCXX/microsoft-abi-multiple-nonvirtual-inheritance.cpp
@@ -88,7 +88,7 @@
// need to adjust 'this' before use.
//
// CHECK: %[[THIS_ADDR:.*]] = alloca %struct.ChildOverride*, align 4
-// CHECK: %[[THIS_i8:.*]] = getelementptr i8* %[[ECX:.*]], i64 -4
+// CHECK: %[[THIS_i8:.*]] = getelementptr inbounds i8* %[[ECX:.*]], i32 -4
// CHECK: %[[THIS:.*]] = bitcast i8* %[[THIS_i8]] to %struct.ChildOverride*
// CHECK: store %struct.ChildOverride* %[[THIS]], %struct.ChildOverride** %[[THIS_ADDR]], align 4
@@ -109,14 +109,14 @@
//
// CHECK: %[[CHILD_i8:.*]] = bitcast %struct.ChildOverride* %[[CHILD]] to i8*
//
-// CHECK: %[[VFPTR_i8:.*]] = getelementptr inbounds i8* %[[CHILD_i8]], i64 4
+// CHECK: %[[VFPTR_i8:.*]] = getelementptr inbounds i8* %[[CHILD_i8]], i32 4
// CHECK: %[[VFPTR:.*]] = bitcast i8* %[[VFPTR_i8]] to void (i8*)***
// CHECK: %[[VFTABLE:.*]] = load void (i8*)*** %[[VFPTR]]
// CHECK: %[[VFUN:.*]] = getelementptr inbounds void (i8*)** %[[VFTABLE]], i64 0
// CHECK: %[[VFUN_VALUE:.*]] = load void (i8*)** %[[VFUN]]
//
// CHECK: %[[CHILD_i8:.*]] = bitcast %struct.ChildOverride* %[[CHILD]] to i8*
-// CHECK: %[[RIGHT:.*]] = getelementptr inbounds i8* %[[CHILD_i8]], i64 4
+// CHECK: %[[RIGHT:.*]] = getelementptr inbounds i8* %[[CHILD_i8]], i32 4
//
// CHECK: call x86_thiscallcc void %[[VFUN_VALUE]](i8* %[[RIGHT]])
// CHECK: ret
@@ -130,7 +130,7 @@
// CHECK: define x86_thiscallcc void @"\01?right at GrandchildOverride@@UAEXXZ"(i8*
//
// CHECK: %[[THIS_ADDR:.*]] = alloca %struct.GrandchildOverride*, align 4
-// CHECK: %[[THIS_i8:.*]] = getelementptr i8* %[[ECX:.*]], i64 -4
+// CHECK: %[[THIS_i8:.*]] = getelementptr inbounds i8* %[[ECX:.*]], i32 -4
// CHECK: %[[THIS:.*]] = bitcast i8* %[[THIS_i8]] to %struct.GrandchildOverride*
// CHECK: store %struct.GrandchildOverride* %[[THIS]], %struct.GrandchildOverride** %[[THIS_ADDR]], align 4
Index: test/CodeGenCXX/microsoft-abi-virtual-inheritance.cpp
===================================================================
--- test/CodeGenCXX/microsoft-abi-virtual-inheritance.cpp
+++ test/CodeGenCXX/microsoft-abi-virtual-inheritance.cpp
@@ -54,7 +54,7 @@
// CHECK-LABEL: define x86_thiscallcc void @"\01??1B@@UAE at XZ"
// Adjust the this parameter:
// CHECK: %[[THIS_PARAM_i8:.*]] = bitcast %struct.B* {{.*}} to i8*
- // CHECK: %[[THIS_i8:.*]] = getelementptr i8* %[[THIS_PARAM_i8]], i64 -8
+ // CHECK: %[[THIS_i8:.*]] = getelementptr inbounds i8* %[[THIS_PARAM_i8]], i32 -8
// CHECK: %[[THIS:.*]] = bitcast i8* %[[THIS_i8]] to %struct.B*
// CHECK: store %struct.B* %[[THIS]], %struct.B** %[[THIS_ADDR:.*]], align 4
// CHECK: %[[THIS:.*]] = load %struct.B** %[[THIS_ADDR]]
@@ -87,7 +87,7 @@
// CHECK2-LABEL: define linkonce_odr x86_thiscallcc void @"\01??_DB@@UAE at XZ"(%struct.B*
// CHECK2: %[[THIS:.*]] = load %struct.B** {{.*}}
// CHECK2: %[[THIS_i8:.*]] = bitcast %struct.B* %[[THIS]] to i8*
- // CHECK2: %[[B_i8:.*]] = getelementptr inbounds i8* %[[THIS_i8]], i64 8
+ // CHECK2: %[[B_i8:.*]] = getelementptr i8* %[[THIS_i8]], i32 8
// CHECK2: %[[B:.*]] = bitcast i8* %[[B_i8]] to %struct.B*
// CHECK2: call x86_thiscallcc void @"\01??1B@@UAE at XZ"(%struct.B* %[[B]])
// CHECK2: %[[THIS_i8:.*]] = bitcast %struct.B* %[[THIS]] to i8*
@@ -98,7 +98,7 @@
// CHECK2-LABEL: define linkonce_odr x86_thiscallcc void @"\01??_GB@@UAEPAXI at Z"
// CHECK2: %[[THIS_PARAM_i8:.*]] = bitcast %struct.B* {{.*}} to i8*
- // CHECK2: %[[THIS_i8:.*]] = getelementptr i8* %[[THIS_PARAM_i8:.*]], i64 -8
+ // CHECK2: %[[THIS_i8:.*]] = getelementptr inbounds i8* %[[THIS_PARAM_i8:.*]], i32 -8
// CHECK2: %[[THIS:.*]] = bitcast i8* %[[THIS_i8]] to %struct.B*
// CHECK2: store %struct.B* %[[THIS]], %struct.B** %[[THIS_ADDR:.*]], align 4
// CHECK2: %[[THIS:.*]] = load %struct.B** %[[THIS_ADDR]]
@@ -114,7 +114,7 @@
// need to adjust 'this' before use.
//
// CHECK: %[[THIS_ADDR:.*]] = alloca %struct.B*, align 4
-// CHECK: %[[THIS_i8:.*]] = getelementptr i8* %[[ECX:.*]], i64 -8
+// CHECK: %[[THIS_i8:.*]] = getelementptr inbounds i8* %[[ECX:.*]], i32 -8
// CHECK: %[[THIS:.*]] = bitcast i8* %[[THIS_i8]] to %struct.B*
// CHECK: store %struct.B* %[[THIS]], %struct.B** %[[THIS_ADDR]], align 4
@@ -290,22 +290,22 @@
D::~D() {
// CHECK-LABEL: define x86_thiscallcc void @"\01??1D at diamond@@UAE at XZ"(%"struct.diamond::D"*)
// CHECK: %[[ARG_i8:.*]] = bitcast %"struct.diamond::D"* %{{.*}} to i8*
- // CHECK: %[[THIS_i8:.*]] = getelementptr i8* %[[ARG_i8]], i64 -24
+ // CHECK: %[[THIS_i8:.*]] = getelementptr inbounds i8* %[[ARG_i8]], i32 -24
// CHECK: %[[THIS:.*]] = bitcast i8* %[[THIS_i8]] to %"struct.diamond::D"*
// CHECK: store %"struct.diamond::D"* %[[THIS]], %"struct.diamond::D"** %[[THIS_VAL:.*]], align 4
// CHECK: %[[THIS:.*]] = load %"struct.diamond::D"** %[[THIS_VAL]]
// CHECK: %[[D_i8:.*]] = bitcast %"struct.diamond::D"* %[[THIS]] to i8*
// CHECK: %[[C_i8:.*]] = getelementptr inbounds i8* %[[D_i8]], i64 4
// CHECK: %[[C:.*]] = bitcast i8* %[[C_i8]] to %"struct.diamond::C"*
// CHECK: %[[C_i8:.*]] = bitcast %"struct.diamond::C"* %[[C]] to i8*
- // CHECK: %[[ARG_i8:.*]] = getelementptr inbounds i8* %{{.*}}, i64 16
+ // CHECK: %[[ARG_i8:.*]] = getelementptr i8* %{{.*}}, i32 16
// FIXME: We might consider changing the dtor this parameter type to i8*.
// CHECK: %[[ARG:.*]] = bitcast i8* %[[ARG_i8]] to %"struct.diamond::C"*
// CHECK: call x86_thiscallcc void @"\01??1C at diamond@@UAE at XZ"(%"struct.diamond::C"* %[[ARG]])
// CHECK: %[[B:.*]] = bitcast %"struct.diamond::D"* %[[THIS]] to %"struct.diamond::B"*
// CHECK: %[[B_i8:.*]] = bitcast %"struct.diamond::B"* %[[B]] to i8*
- // CHECK: %[[ARG_i8:.*]] = getelementptr inbounds i8* %[[B_i8]], i64 4
+ // CHECK: %[[ARG_i8:.*]] = getelementptr i8* %[[B_i8]], i32 4
// CHECK: %[[ARG:.*]] = bitcast i8* %[[ARG_i8]] to %"struct.diamond::B"*
// CHECK: call x86_thiscallcc void @"\01??1B at diamond@@UAE at XZ"(%"struct.diamond::B"* %[[ARG]])
// CHECK: ret void
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D1977.1.patch
Type: text/x-patch
Size: 8218 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131018/f970401a/attachment.bin>
More information about the cfe-commits
mailing list