Skip to content

Commit 2661e99

Browse files
PiJoulesPiJoules
authored
[llvm] Ensure propagated constants in the vtable are aligned (#136630)
It's possible for virtual constant propagation in whole program devirtualization to create unaligned loads. We originally saw this with 4-byte aligned relative vtables where we could store 8-byte values before/after the vtable. But since the vtable is 4-byte aligned and we unconditionally do an 8-byte load, we can't guarantee that the stored constant will always be aligned to 8 bytes. We can also see this with normal vtables whenever a 1-byte char is stored in the vtable because the offset calculation for the GEP doesn't take into account the original vtable alignment. This patch introduces two changes to virtual constant propagation: 1. Do not propagate constants whose preferred alignment is larger than the vtable alignment. This is required because if the constants are stored in the vtable, we can only guarantee the constant will be stored at an address at most aligned to the vtable's alignment. 2. Round up the offset used in the GEP before the load to ensure it's at an address suitably aligned such that we can load from it. This patch updates tests to reflect this alignment change and adds some cases for relative vtables.
1 parent 5c551cb commit 2661e99

File tree

7 files changed

+121
-83
lines changed

7 files changed

+121
-83
lines changed

‎llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -298,7 +298,9 @@ wholeprogramdevirt::findLowestOffset(ArrayRef<VirtualCallTarget> Targets,
298298
++Byte;
299299
}
300300
}
301-
return (MinByte + I) * 8;
301+
// Rounding up ensures the constant is always stored at address we
302+
// can directly load from without misalignment.
303+
return alignTo((MinByte + I) * 8, Size);
302304
NextI:;
303305
}
304306
}
@@ -1834,9 +1836,19 @@ bool DevirtModule::tryVirtualConstProp(
18341836
if (!RetType)
18351837
return false;
18361838
unsigned BitWidth = RetType->getBitWidth();
1839+
1840+
// TODO: Since we can evaluated these constants at compile-time, we can save
1841+
// some space by calculating the smallest range of values that all these
1842+
// constants can fit in, then only allocate enough space to fit those values.
1843+
// At each callsite, we can get the original type by doing a sign/zero
1844+
// extension. For example, if we would store an i64, but we can see that all
1845+
// the values fit into an i16, then we can store an i16 before/after the
1846+
// vtable and at each callsite do a s/zext.
18371847
if (BitWidth > 64)
18381848
return false;
18391849

1850+
Align TypeAlignment = M.getDataLayout().getPrefTypeAlign(RetType);
1851+
18401852
// Make sure that each function is defined, does not access memory, takes at
18411853
// least one argument, does not use its first argument (which we assume is
18421854
// 'this'), and has the same return type.
@@ -1861,6 +1873,18 @@ bool DevirtModule::tryVirtualConstProp(
18611873
Fn->arg_empty() || !Fn->arg_begin()->use_empty() ||
18621874
Fn->getReturnType() != RetType)
18631875
return false;
1876+
1877+
// This only works if the integer size is at most the alignment of the
1878+
// vtable. If the table is underaligned, then we can't guarantee that the
1879+
// constant will always be aligned to the integer type alignment. For
1880+
// example, if the table is `align 1`, we can never guarantee that an i32
1881+
// stored before/after the vtable is 32-bit aligned without changing the
1882+
// alignment of the new global.
1883+
GlobalVariable *GV = Target.TM->Bits->GV;
1884+
Align TableAlignment = M.getDataLayout().getValueOrABITypeAlignment(
1885+
GV->getAlign(), GV->getValueType());
1886+
if (TypeAlignment > TableAlignment)
1887+
return false;
18641888
}
18651889

18661890
for (auto &&CSByConstantArg : SlotInfo.ConstCSInfo) {
@@ -1880,6 +1904,9 @@ bool DevirtModule::tryVirtualConstProp(
18801904

18811905
// Find an allocation offset in bits in all vtables associated with the
18821906
// type.
1907+
// TODO: If there would be "holes" in the vtable that were added by
1908+
// padding, we could place i1s there to reduce any extra padding that
1909+
// would be introduced by the i1s.
18831910
uint64_t AllocBefore =
18841911
findLowestOffset(TargetsForSlot, /*IsAfter=*/false, BitWidth);
18851912
uint64_t AllocAfter =
@@ -1911,6 +1938,14 @@ bool DevirtModule::tryVirtualConstProp(
19111938
setAfterReturnValues(TargetsForSlot, AllocAfter, BitWidth, OffsetByte,
19121939
OffsetBit);
19131940

1941+
// In an earlier check we forbade constant propagation from operating on
1942+
// tables whose alignment is less than the alignment needed for loading
1943+
// the constant. Thus, the address we take the offset from will always be
1944+
// aligned to at least this integer alignment. Now, we need to ensure that
1945+
// the offset is also aligned to this integer alignment to ensure we always
1946+
// have an aligned load.
1947+
assert(OffsetByte % TypeAlignment.value() == 0);
1948+
19141949
if (RemarksEnabled || AreStatisticsEnabled())
19151950
for (auto &&Target : TargetsForSlot)
19161951
Target.WasDevirt = true;

‎llvm/test/Transforms/WholeProgramDevirt/virtual-const-prop-begin.ll

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,28 +9,28 @@ target datalayout = "e-p:64:64"
99
;; preserve alignment. Making them i16s allows them to stay at the beginning of
1010
;; the vtable. There are other tests where there's a mix of constants before and
1111
;; after the vtable but for this file we just want everything before the vtable.
12-
; CHECK: [[VT1DATA:@[^ ]*]] = private constant { [8 x i8], [3 x ptr], [0 x i8] } { [8 x i8] c"\00\00\00\00\00\03\00\02", [3 x ptr] [ptr @vf0i1, ptr @vf1i1, ptr @vf1i16], [0 x i8] zeroinitializer }, section "vt1sec", !type [[T8:![0-9]+]]
12+
; CHECK: [[VT1DATA:@[^ ]*]] = private constant { [8 x i8], [3 x ptr], [0 x i8] } { [8 x i8] c"\00\00\00\00\03\00\00\02", [3 x ptr] [ptr @vf0i1, ptr @vf1i1, ptr @vf1i16], [0 x i8] zeroinitializer }, section "vt1sec", !type [[T8:![0-9]+]]
1313
@vt1 = constant [3 x ptr] [
1414
ptr @vf0i1,
1515
ptr @vf1i1,
1616
ptr @vf1i16
1717
], section "vt1sec", !type !0
1818

19-
; CHECK: [[VT2DATA:@[^ ]*]] = private constant { [8 x i8], [3 x ptr], [0 x i8] } { [8 x i8] c"\00\00\00\00\00\04\00\01", [3 x ptr] [ptr @vf1i1, ptr @vf0i1, ptr @vf2i16], [0 x i8] zeroinitializer }, !type [[T8]]
19+
; CHECK: [[VT2DATA:@[^ ]*]] = private constant { [8 x i8], [3 x ptr], [0 x i8] } { [8 x i8] c"\00\00\00\00\04\00\00\01", [3 x ptr] [ptr @vf1i1, ptr @vf0i1, ptr @vf2i16], [0 x i8] zeroinitializer }, !type [[T8]]
2020
@vt2 = constant [3 x ptr] [
2121
ptr @vf1i1,
2222
ptr @vf0i1,
2323
ptr @vf2i16
2424
], !type !0
2525

26-
; CHECK: [[VT3DATA:@[^ ]*]] = private constant { [4 x i8], [3 x ptr], [0 x i8] } { [4 x i8] c"\00\05\00\02", [3 x ptr] [ptr @vf0i1, ptr @vf1i1, ptr @vf3i16], [0 x i8] zeroinitializer }, align 2, !type [[T5:![0-9]+]]
26+
; CHECK: [[VT3DATA:@[^ ]*]] = private constant { [4 x i8], [3 x ptr], [0 x i8] } { [4 x i8] c"\05\00\00\02", [3 x ptr] [ptr @vf0i1, ptr @vf1i1, ptr @vf3i16], [0 x i8] zeroinitializer }, align 2, !type [[T5:![0-9]+]]
2727
@vt3 = constant [3 x ptr] [
2828
ptr @vf0i1,
2929
ptr @vf1i1,
3030
ptr @vf3i16
3131
], align 2, !type !0
3232

33-
; CHECK: [[VT4DATA:@[^ ]*]] = private constant { [16 x i8], [3 x ptr], [0 x i8] } { [16 x i8] c"\00\00\00\00\00\00\00\00\00\00\00\00\00\06\00\01", [3 x ptr] [ptr @vf1i1, ptr @vf0i1, ptr @vf4i16], [0 x i8] zeroinitializer }, align 16, !type [[T16:![0-9]+]]
33+
; CHECK: [[VT4DATA:@[^ ]*]] = private constant { [16 x i8], [3 x ptr], [0 x i8] } { [16 x i8] c"\00\00\00\00\00\00\00\00\00\00\00\00\06\00\00\01", [3 x ptr] [ptr @vf1i1, ptr @vf0i1, ptr @vf4i16], [0 x i8] zeroinitializer }, align 16, !type [[T16:![0-9]+]]
3434
@vt4 = constant [3 x ptr] [
3535
ptr @vf1i1,
3636
ptr @vf0i1,
@@ -136,7 +136,7 @@ define i16 @call3(ptr %obj) {
136136
call void @llvm.assume(i1 %p)
137137
%fptrptr = getelementptr [3 x ptr], ptr %vtable, i16 0, i16 2
138138
%fptr = load ptr, ptr %fptrptr
139-
; CHECK: [[VTGEP3:%[^ ]*]] = getelementptr i8, ptr %vtable, i32 -3
139+
; CHECK: [[VTGEP3:%[^ ]*]] = getelementptr i8, ptr %vtable, i32 -4
140140
; CHECK: [[VTLOAD3:%[^ ]*]] = load i16, ptr [[VTGEP3]]
141141
%result = call i16 %fptr(ptr %obj)
142142
; CHECK: ret i16 [[VTLOAD3]]

‎llvm/test/Transforms/WholeProgramDevirt/virtual-const-prop-check.ll

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -37,28 +37,28 @@ target triple = "x86_64-unknown-linux-gnu"
3737

3838
; SKIP-ALL-NOT: devirtualized
3939

40-
; CHECK: [[VT1DATA:@[^ ]*]] = private constant { [8 x i8], [3 x ptr], [0 x i8] } { [8 x i8] c"\00\00\00\01\00\00\00\02", [3 x ptr] [ptr @vf0i1, ptr @vf1i1, ptr @vf1i32], [0 x i8] zeroinitializer }, section "vt1sec", !type [[T8:![0-9]+]]
40+
; CHECK: [[VT1DATA:@[^ ]*]] = private constant { [8 x i8], [3 x ptr], [4 x i8] } { [8 x i8] c"\00\00\00\00\00\00\00\02", [3 x ptr] [ptr @vf0i1, ptr @vf1i1, ptr @vf1i32], [4 x i8] c"\01\00\00\00" }, section "vt1sec", !type [[T8:![0-9]+]]
4141
@vt1 = constant [3 x ptr] [
4242
ptr @vf0i1,
4343
ptr @vf1i1,
4444
ptr @vf1i32
4545
], section "vt1sec", !type !0
4646

47-
; CHECK: [[VT2DATA:@[^ ]*]] = private constant { [8 x i8], [3 x ptr], [0 x i8] } { [8 x i8] c"\00\00\00\02\00\00\00\01", [3 x ptr] [ptr @vf1i1, ptr @vf0i1, ptr @vf2i32], [0 x i8] zeroinitializer }, !type [[T8]]
47+
; CHECK: [[VT2DATA:@[^ ]*]] = private constant { [8 x i8], [3 x ptr], [4 x i8] } { [8 x i8] c"\00\00\00\00\00\00\00\01", [3 x ptr] [ptr @vf1i1, ptr @vf0i1, ptr @vf2i32], [4 x i8] c"\02\00\00\00" }, !type [[T8]]
4848
@vt2 = constant [3 x ptr] [
4949
ptr @vf1i1,
5050
ptr @vf0i1,
5151
ptr @vf2i32
5252
], !type !0
5353

54-
; CHECK: [[VT3DATA:@[^ ]*]] = private constant { [8 x i8], [3 x ptr], [0 x i8] } { [8 x i8] c"\00\00\00\03\00\00\00\02", [3 x ptr] [ptr @vf0i1, ptr @vf1i1, ptr @vf3i32], [0 x i8] zeroinitializer }, !type [[T8]]
54+
; CHECK: [[VT3DATA:@[^ ]*]] = private constant { [8 x i8], [3 x ptr], [4 x i8] } { [8 x i8] c"\00\00\00\00\00\00\00\02", [3 x ptr] [ptr @vf0i1, ptr @vf1i1, ptr @vf3i32], [4 x i8] c"\03\00\00\00" }, !type [[T8]]
5555
@vt3 = constant [3 x ptr] [
5656
ptr @vf0i1,
5757
ptr @vf1i1,
5858
ptr @vf3i32
5959
], !type !0
6060

61-
; CHECK: [[VT4DATA:@[^ ]*]] = private constant { [8 x i8], [3 x ptr], [0 x i8] } { [8 x i8] c"\00\00\00\04\00\00\00\01", [3 x ptr] [ptr @vf1i1, ptr @vf0i1, ptr @vf4i32], [0 x i8] zeroinitializer }, !type [[T8]]
61+
; CHECK: [[VT4DATA:@[^ ]*]] = private constant { [8 x i8], [3 x ptr], [4 x i8] } { [8 x i8] c"\00\00\00\00\00\00\00\01", [3 x ptr] [ptr @vf1i1, ptr @vf0i1, ptr @vf4i32], [4 x i8] c"\04\00\00\00" }, !type [[T8]]
6262
@vt4 = constant [3 x ptr] [
6363
ptr @vf1i1,
6464
ptr @vf0i1,
@@ -95,10 +95,10 @@ i32 trunc (i64 sub (i64 ptrtoint (ptr dso_local_equivalent @vf0i1 to i64), i64 p
9595
i32 trunc (i64 sub (i64 ptrtoint (ptr dso_local_equivalent @vf4i32 to i64), i64 ptrtoint (ptr @vt7_rel to i64)) to i32)
9696
], !type !1
9797

98-
; CHECK: @vt1 = alias [3 x ptr], getelementptr inbounds ({ [8 x i8], [3 x ptr], [0 x i8] }, ptr [[VT1DATA]], i32 0, i32 1)
99-
; CHECK: @vt2 = alias [3 x ptr], getelementptr inbounds ({ [8 x i8], [3 x ptr], [0 x i8] }, ptr [[VT2DATA]], i32 0, i32 1)
100-
; CHECK: @vt3 = alias [3 x ptr], getelementptr inbounds ({ [8 x i8], [3 x ptr], [0 x i8] }, ptr [[VT3DATA]], i32 0, i32 1)
101-
; CHECK: @vt4 = alias [3 x ptr], getelementptr inbounds ({ [8 x i8], [3 x ptr], [0 x i8] }, ptr [[VT4DATA]], i32 0, i32 1)
98+
; CHECK: @vt1 = alias [3 x ptr], getelementptr inbounds ({ [8 x i8], [3 x ptr], [4 x i8] }, ptr [[VT1DATA]], i32 0, i32 1)
99+
; CHECK: @vt2 = alias [3 x ptr], getelementptr inbounds ({ [8 x i8], [3 x ptr], [4 x i8] }, ptr [[VT2DATA]], i32 0, i32 1)
100+
; CHECK: @vt3 = alias [3 x ptr], getelementptr inbounds ({ [8 x i8], [3 x ptr], [4 x i8] }, ptr [[VT3DATA]], i32 0, i32 1)
101+
; CHECK: @vt4 = alias [3 x ptr], getelementptr inbounds ({ [8 x i8], [3 x ptr], [4 x i8] }, ptr [[VT4DATA]], i32 0, i32 1)
102102
; CHECK: @vt6_rel = alias [3 x i32], getelementptr inbounds ({ [4 x i8], [3 x i32], [0 x i8] }, ptr [[VT6RELDATA]], i32 0, i32 1)
103103
; CHECK: @vt7_rel = alias [3 x i32], getelementptr inbounds ({ [4 x i8], [3 x i32], [0 x i8] }, ptr [[VT7RELDATA]], i32 0, i32 1)
104104

@@ -165,7 +165,7 @@ define i32 @call3(ptr %obj) {
165165
%vtable = load ptr, ptr %obj
166166
%pair = call {ptr, i1} @llvm.type.checked.load(ptr %vtable, i32 16, metadata !"typeid")
167167
%fptr = extractvalue {ptr, i1} %pair, 0
168-
; CHECK: [[VTGEP3:%[^ ]*]] = getelementptr i8, ptr %vtable, i32 -5
168+
; CHECK: [[VTGEP3:%[^ ]*]] = getelementptr i8, ptr %vtable, i32 24
169169
; CHECK: [[VTLOAD3:%[^ ]*]] = load i32, ptr [[VTGEP3]]
170170
%result = call i32 %fptr(ptr %obj)
171171
; CHECK: ret i32 [[VTLOAD3]]

‎llvm/test/Transforms/WholeProgramDevirt/virtual-const-prop-end.ll

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,30 +2,30 @@
22

33
target datalayout = "e-p:64:64"
44

5-
; CHECK: [[VT1DATA:@[^ ]*]] = private constant { [0 x i8], [4 x ptr], [5 x i8] } { [0 x i8] zeroinitializer, [4 x ptr] [ptr null, ptr @vf0i1, ptr @vf1i1, ptr @vf1i32], [5 x i8] c"\02\03\00\00\00" }, !type [[T8:![0-9]+]]
5+
; CHECK: [[VT1DATA:@[^ ]*]] = private constant { [0 x i8], [4 x ptr], [8 x i8] } { [0 x i8] zeroinitializer, [4 x ptr] [ptr null, ptr @vf0i1, ptr @vf1i1, ptr @vf1i32], [8 x i8] c"\02\00\00\00\03\00\00\00" }, !type [[T8:![0-9]+]]
66
@vt1 = constant [4 x ptr] [
77
ptr null,
88
ptr @vf0i1,
99
ptr @vf1i1,
1010
ptr @vf1i32
1111
], !type !1
1212

13-
; CHECK: [[VT2DATA:@[^ ]*]] = private constant { [0 x i8], [3 x ptr], [5 x i8] } { [0 x i8] zeroinitializer, [3 x ptr] [ptr @vf1i1, ptr @vf0i1, ptr @vf2i32], [5 x i8] c"\01\04\00\00\00" }, !type [[T0:![0-9]+]]
13+
; CHECK: [[VT2DATA:@[^ ]*]] = private constant { [0 x i8], [3 x ptr], [8 x i8] } { [0 x i8] zeroinitializer, [3 x ptr] [ptr @vf1i1, ptr @vf0i1, ptr @vf2i32], [8 x i8] c"\01\00\00\00\04\00\00\00" }, !type [[T0:![0-9]+]]
1414
@vt2 = constant [3 x ptr] [
1515
ptr @vf1i1,
1616
ptr @vf0i1,
1717
ptr @vf2i32
1818
], !type !0
1919

20-
; CHECK: [[VT3DATA:@[^ ]*]] = private constant { [0 x i8], [4 x ptr], [5 x i8] } { [0 x i8] zeroinitializer, [4 x ptr] [ptr null, ptr @vf0i1, ptr @vf1i1, ptr @vf3i32], [5 x i8] c"\02\05\00\00\00" }, !type [[T8]]
20+
; CHECK: [[VT3DATA:@[^ ]*]] = private constant { [0 x i8], [4 x ptr], [8 x i8] } { [0 x i8] zeroinitializer, [4 x ptr] [ptr null, ptr @vf0i1, ptr @vf1i1, ptr @vf3i32], [8 x i8] c"\02\00\00\00\05\00\00\00" }, !type [[T8]]
2121
@vt3 = constant [4 x ptr] [
2222
ptr null,
2323
ptr @vf0i1,
2424
ptr @vf1i1,
2525
ptr @vf3i32
2626
], !type !1
2727

28-
; CHECK: [[VT4DATA:@[^ ]*]] = private constant { [0 x i8], [3 x ptr], [5 x i8] } { [0 x i8] zeroinitializer, [3 x ptr] [ptr @vf1i1, ptr @vf0i1, ptr @vf4i32], [5 x i8] c"\01\06\00\00\00" }, !type [[T0]]
28+
; CHECK: [[VT4DATA:@[^ ]*]] = private constant { [0 x i8], [3 x ptr], [8 x i8] } { [0 x i8] zeroinitializer, [3 x ptr] [ptr @vf1i1, ptr @vf0i1, ptr @vf4i32], [8 x i8] c"\01\00\00\00\06\00\00\00" }, !type [[T0]]
2929
@vt4 = constant [3 x ptr] [
3030
ptr @vf1i1,
3131
ptr @vf0i1,
@@ -57,10 +57,10 @@ i32 trunc (i64 sub (i64 ptrtoint (ptr dso_local_equivalent @vf1i1 to i64), i64 p
5757
i32 trunc (i64 sub (i64 ptrtoint (ptr dso_local_equivalent @vf4i32 to i64), i64 ptrtoint (ptr @vt6_rel to i64)) to i32)
5858
], !type !2
5959

60-
; CHECK: @vt1 = alias [4 x ptr], getelementptr inbounds ({ [0 x i8], [4 x ptr], [5 x i8] }, ptr [[VT1DATA]], i32 0, i32 1)
61-
; CHECK: @vt2 = alias [3 x ptr], getelementptr inbounds ({ [0 x i8], [3 x ptr], [5 x i8] }, ptr [[VT2DATA]], i32 0, i32 1)
62-
; CHECK: @vt3 = alias [4 x ptr], getelementptr inbounds ({ [0 x i8], [4 x ptr], [5 x i8] }, ptr [[VT3DATA]], i32 0, i32 1)
63-
; CHECK: @vt4 = alias [3 x ptr], getelementptr inbounds ({ [0 x i8], [3 x ptr], [5 x i8] }, ptr [[VT4DATA]], i32 0, i32 1)
60+
; CHECK: @vt1 = alias [4 x ptr], getelementptr inbounds ({ [0 x i8], [4 x ptr], [8 x i8] }, ptr [[VT1DATA]], i32 0, i32 1)
61+
; CHECK: @vt2 = alias [3 x ptr], getelementptr inbounds ({ [0 x i8], [3 x ptr], [8 x i8] }, ptr [[VT2DATA]], i32 0, i32 1)
62+
; CHECK: @vt3 = alias [4 x ptr], getelementptr inbounds ({ [0 x i8], [4 x ptr], [8 x i8] }, ptr [[VT3DATA]], i32 0, i32 1)
63+
; CHECK: @vt4 = alias [3 x ptr], getelementptr inbounds ({ [0 x i8], [3 x ptr], [8 x i8] }, ptr [[VT4DATA]], i32 0, i32 1)
6464

6565
define i1 @vf0i1(ptr %this) readnone {
6666
ret i1 0
@@ -124,7 +124,7 @@ define i32 @call3(ptr %obj) {
124124
call void @llvm.assume(i1 %p)
125125
%fptrptr = getelementptr [3 x ptr], ptr %vtable, i32 0, i32 2
126126
%fptr = load ptr, ptr %fptrptr
127-
; CHECK: [[VTGEP3:%[^ ]*]] = getelementptr i8, ptr %vtable, i32 25
127+
; CHECK: [[VTGEP3:%[^ ]*]] = getelementptr i8, ptr %vtable, i32 28
128128
; CHECK: [[VTLOAD3:%[^ ]*]] = load i32, ptr [[VTGEP3]]
129129
%result = call i32 %fptr(ptr %obj)
130130
; CHECK: ret i32 [[VTLOAD3]]

0 commit comments

Comments
 (0)