Skip to content

Commit 1302610

Browse files
tobias-stadlertobias-stadler
authored
[MergeFunc] Fix crash caused by bitcasting ArrayType (llvm#133259)
createCast in MergeFunctions did not consider ArrayTypes, which results in the creation of a bitcast between ArrayTypes in the thunk function, leading to an assertion failure in the provided test case. The version of createCast in GlobalMergeFunctions does handle ArrayTypes, so this common code has been factored out into the IRBuilder.
1 parent c154d66 commit 1302610

File tree

5 files changed

+124
-71
lines changed

5 files changed

+124
-71
lines changed

‎llvm/include/llvm/IR/IRBuilder.h‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2299,6 +2299,13 @@ class IRBuilderBase {
22992299
// isSigned parameter.
23002300
Value *CreateIntCast(Value *, Type *, const char *) = delete;
23012301

2302+
/// Cast between aggregate types that must have identical structure but may
2303+
/// differ in their leaf types. The leaf values are recursively extracted,
2304+
/// casted, and then reinserted into a value of type DestTy. The leaf types
2305+
/// must be castable using a bitcast or ptrcast, because signedness is
2306+
/// not specified.
2307+
Value *CreateAggregateCast(Value *V, Type *DestTy);
2308+
23022309
//===--------------------------------------------------------------------===//
23032310
// Instruction creation methods: Compare Instructions
23042311
//===--------------------------------------------------------------------===//

‎llvm/lib/CodeGen/GlobalMergeFunctions.cpp‎

Lines changed: 5 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -140,44 +140,6 @@ static bool ignoreOp(const Instruction *I, unsigned OpIdx) {
140140
return true;
141141
}
142142

143-
static Value *createCast(IRBuilder<> &Builder, Value *V, Type *DestTy) {
144-
Type *SrcTy = V->getType();
145-
if (SrcTy->isStructTy()) {
146-
assert(DestTy->isStructTy());
147-
assert(SrcTy->getStructNumElements() == DestTy->getStructNumElements());
148-
Value *Result = PoisonValue::get(DestTy);
149-
for (unsigned int I = 0, E = SrcTy->getStructNumElements(); I < E; ++I) {
150-
Value *Element =
151-
createCast(Builder, Builder.CreateExtractValue(V, ArrayRef(I)),
152-
DestTy->getStructElementType(I));
153-
154-
Result = Builder.CreateInsertValue(Result, Element, ArrayRef(I));
155-
}
156-
return Result;
157-
}
158-
assert(!DestTy->isStructTy());
159-
if (auto *SrcAT = dyn_cast<ArrayType>(SrcTy)) {
160-
auto *DestAT = dyn_cast<ArrayType>(DestTy);
161-
assert(DestAT);
162-
assert(SrcAT->getNumElements() == DestAT->getNumElements());
163-
Value *Result = PoisonValue::get(DestTy);
164-
for (unsigned int I = 0, E = SrcAT->getNumElements(); I < E; ++I) {
165-
Value *Element =
166-
createCast(Builder, Builder.CreateExtractValue(V, ArrayRef(I)),
167-
DestAT->getElementType());
168-
169-
Result = Builder.CreateInsertValue(Result, Element, ArrayRef(I));
170-
}
171-
return Result;
172-
}
173-
assert(!DestTy->isArrayTy());
174-
if (SrcTy->isIntegerTy() && DestTy->isPointerTy())
175-
return Builder.CreateIntToPtr(V, DestTy);
176-
if (SrcTy->isPointerTy() && DestTy->isIntegerTy())
177-
return Builder.CreatePtrToInt(V, DestTy);
178-
return Builder.CreateBitCast(V, DestTy);
179-
}
180-
181143
void GlobalMergeFunc::analyze(Module &M) {
182144
++NumAnalyzedModues;
183145
for (Function &Func : M) {
@@ -268,7 +230,7 @@ static Function *createMergedFunction(FuncMergeInfo &FI,
268230
if (OrigC->getType() != NewArg->getType()) {
269231
IRBuilder<> Builder(Inst->getParent(), Inst->getIterator());
270232
Inst->setOperand(OpndIndex,
271-
createCast(Builder, NewArg, OrigC->getType()));
233+
Builder.CreateAggregateCast(NewArg, OrigC->getType()));
272234
} else {
273235
Inst->setOperand(OpndIndex, NewArg);
274236
}
@@ -297,15 +259,16 @@ static void createThunk(FuncMergeInfo &FI, ArrayRef<Constant *> Params,
297259

298260
// Add arguments which are passed through Thunk.
299261
for (Argument &AI : Thunk->args()) {
300-
Args.push_back(createCast(Builder, &AI, ToFuncTy->getParamType(ParamIdx)));
262+
Args.push_back(
263+
Builder.CreateAggregateCast(&AI, ToFuncTy->getParamType(ParamIdx)));
301264
++ParamIdx;
302265
}
303266

304267
// Add new arguments defined by Params.
305268
for (auto *Param : Params) {
306269
assert(ParamIdx < ToFuncTy->getNumParams());
307270
Args.push_back(
308-
createCast(Builder, Param, ToFuncTy->getParamType(ParamIdx)));
271+
Builder.CreateAggregateCast(Param, ToFuncTy->getParamType(ParamIdx)));
309272
++ParamIdx;
310273
}
311274

@@ -319,7 +282,7 @@ static void createThunk(FuncMergeInfo &FI, ArrayRef<Constant *> Params,
319282
if (Thunk->getReturnType()->isVoidTy())
320283
Builder.CreateRetVoid();
321284
else
322-
Builder.CreateRet(createCast(Builder, CI, Thunk->getReturnType()));
285+
Builder.CreateRet(Builder.CreateAggregateCast(CI, Thunk->getReturnType()));
323286
}
324287

325288
// Check if the old merged/optimized IndexOperandHashMap is compatible with

‎llvm/lib/IR/IRBuilder.cpp‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,40 @@ void IRBuilderBase::SetInstDebugLocation(Instruction *I) const {
7676
}
7777
}
7878

79+
Value *IRBuilderBase::CreateAggregateCast(Value *V, Type *DestTy) {
80+
Type *SrcTy = V->getType();
81+
if (SrcTy == DestTy)
82+
return V;
83+
84+
if (SrcTy->isAggregateType()) {
85+
unsigned NumElements;
86+
if (SrcTy->isStructTy()) {
87+
assert(DestTy->isStructTy() && "Expected StructType");
88+
assert(SrcTy->getStructNumElements() == DestTy->getStructNumElements() &&
89+
"Expected StructTypes with equal number of elements");
90+
NumElements = SrcTy->getStructNumElements();
91+
} else {
92+
assert(SrcTy->isArrayTy() && DestTy->isArrayTy() && "Expected ArrayType");
93+
assert(SrcTy->getArrayNumElements() == DestTy->getArrayNumElements() &&
94+
"Expected ArrayTypes with equal number of elements");
95+
NumElements = SrcTy->getArrayNumElements();
96+
}
97+
98+
Value *Result = PoisonValue::get(DestTy);
99+
for (unsigned I = 0; I < NumElements; ++I) {
100+
Type *ElementTy = SrcTy->isStructTy() ? DestTy->getStructElementType(I)
101+
: DestTy->getArrayElementType();
102+
Value *Element =
103+
CreateAggregateCast(CreateExtractValue(V, ArrayRef(I)), ElementTy);
104+
105+
Result = CreateInsertValue(Result, Element, ArrayRef(I));
106+
}
107+
return Result;
108+
}
109+
110+
return CreateBitOrPointerCast(V, DestTy);
111+
}
112+
79113
CallInst *
80114
IRBuilderBase::createCallHelper(Function *Callee, ArrayRef<Value *> Ops,
81115
const Twine &Name, FMFSource FMFSource,

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

Lines changed: 2 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -511,33 +511,6 @@ void MergeFunctions::replaceDirectCallers(Function *Old, Function *New) {
511511
}
512512
}
513513

514-
// Helper for writeThunk,
515-
// Selects proper bitcast operation,
516-
// but a bit simpler then CastInst::getCastOpcode.
517-
static Value *createCast(IRBuilder<> &Builder, Value *V, Type *DestTy) {
518-
Type *SrcTy = V->getType();
519-
if (SrcTy->isStructTy()) {
520-
assert(DestTy->isStructTy());
521-
assert(SrcTy->getStructNumElements() == DestTy->getStructNumElements());
522-
Value *Result = PoisonValue::get(DestTy);
523-
for (unsigned int I = 0, E = SrcTy->getStructNumElements(); I < E; ++I) {
524-
Value *Element =
525-
createCast(Builder, Builder.CreateExtractValue(V, ArrayRef(I)),
526-
DestTy->getStructElementType(I));
527-
528-
Result = Builder.CreateInsertValue(Result, Element, ArrayRef(I));
529-
}
530-
return Result;
531-
}
532-
assert(!DestTy->isStructTy());
533-
if (SrcTy->isIntegerTy() && DestTy->isPointerTy())
534-
return Builder.CreateIntToPtr(V, DestTy);
535-
else if (SrcTy->isPointerTy() && DestTy->isIntegerTy())
536-
return Builder.CreatePtrToInt(V, DestTy);
537-
else
538-
return Builder.CreateBitCast(V, DestTy);
539-
}
540-
541514
// Erase the instructions in PDIUnrelatedWL as they are unrelated to the
542515
// parameter debug info, from the entry block.
543516
void MergeFunctions::eraseInstsUnrelatedToPDI(
@@ -789,7 +762,7 @@ void MergeFunctions::writeThunk(Function *F, Function *G) {
789762
unsigned i = 0;
790763
FunctionType *FFTy = F->getFunctionType();
791764
for (Argument &AI : H->args()) {
792-
Args.push_back(createCast(Builder, &AI, FFTy->getParamType(i)));
765+
Args.push_back(Builder.CreateAggregateCast(&AI, FFTy->getParamType(i)));
793766
++i;
794767
}
795768

@@ -804,7 +777,7 @@ void MergeFunctions::writeThunk(Function *F, Function *G) {
804777
if (H->getReturnType()->isVoidTy()) {
805778
RI = Builder.CreateRetVoid();
806779
} else {
807-
RI = Builder.CreateRet(createCast(Builder, CI, H->getReturnType()));
780+
RI = Builder.CreateRet(Builder.CreateAggregateCast(CI, H->getReturnType()));
808781
}
809782

810783
if (MergeFunctionsPDI) {

‎llvm/test/Transforms/MergeFunc/crash-cast-arrays.ll‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
; RUN: opt -S -passes=mergefunc < %s | FileCheck %s
2+
3+
target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-n32:64-S128-Fn32"
4+
5+
%A = type { double }
6+
; the intermediary struct causes A_arr and B_arr to be different types
7+
%A_struct = type { %A }
8+
%A_arr = type { [1 x %A_struct] }
9+
10+
%B = type { double }
11+
%B_struct = type { %B }
12+
%B_arr = type { [1 x %B_struct] }
13+
14+
; conversion between C_arr and D_arr is possible, but requires ptrcast
15+
%C = type { i64 }
16+
%C_struct = type { %C }
17+
%C_arr = type { [1 x %C_struct] }
18+
19+
%D = type { ptr }
20+
%D_struct = type { %D }
21+
%D_arr = type { [1 x %D_struct] }
22+
23+
declare void @noop()
24+
25+
define %A_arr @a() {
26+
; CHECK-LABEL: define %A_arr @a() {
27+
; CHECK-NEXT: call void @noop()
28+
; CHECK-NEXT: ret %A_arr zeroinitializer
29+
;
30+
call void @noop()
31+
ret %A_arr zeroinitializer
32+
}
33+
34+
define %C_arr @c() {
35+
; CHECK-LABEL: define %C_arr @c() {
36+
; CHECK-NEXT: call void @noop()
37+
; CHECK-NEXT: ret %C_arr zeroinitializer
38+
;
39+
call void @noop()
40+
ret %C_arr zeroinitializer
41+
}
42+
43+
define %B_arr @b() {
44+
; CHECK-LABEL: define %B_arr @b() {
45+
; CHECK-NEXT: [[TMP1:%.*]] = tail call %A_arr @a
46+
; CHECK-NEXT: [[TMP2:%.*]] = extractvalue %A_arr [[TMP1]], 0
47+
; CHECK-NEXT: [[TMP3:%.*]] = extractvalue [1 x %A_struct] [[TMP2]], 0
48+
; CHECK-NEXT: [[TMP4:%.*]] = extractvalue %A_struct [[TMP3]], 0
49+
; CHECK-NEXT: [[TMP5:%.*]] = extractvalue %A [[TMP4]], 0
50+
; CHECK-NEXT: [[TMP6:%.*]] = insertvalue %B poison, double [[TMP5]], 0
51+
; CHECK-NEXT: [[TMP7:%.*]] = insertvalue %B_struct poison, %B [[TMP6]], 0
52+
; CHECK-NEXT: [[TMP8:%.*]] = insertvalue [1 x %B_struct] poison, %B_struct [[TMP7]], 0
53+
; CHECK-NEXT: [[TMP9:%.*]] = insertvalue %B_arr poison, [1 x %B_struct] [[TMP8]], 0
54+
; CHECK-NEXT: ret %B_arr [[TMP9]]
55+
;
56+
call void @noop()
57+
ret %B_arr zeroinitializer
58+
}
59+
60+
define %D_arr @d() {
61+
; CHECK-LABEL: define %D_arr @d() {
62+
; CHECK-NEXT: [[TMP1:%.*]] = tail call %C_arr @c
63+
; CHECK-NEXT: [[TMP2:%.*]] = extractvalue %C_arr [[TMP1]], 0
64+
; CHECK-NEXT: [[TMP3:%.*]] = extractvalue [1 x %C_struct] [[TMP2]], 0
65+
; CHECK-NEXT: [[TMP4:%.*]] = extractvalue %C_struct [[TMP3]], 0
66+
; CHECK-NEXT: [[TMP5:%.*]] = extractvalue %C [[TMP4]], 0
67+
; CHECK-NEXT: [[TMP10:%.*]] = inttoptr i64 [[TMP5]] to ptr
68+
; CHECK-NEXT: [[TMP6:%.*]] = insertvalue %D poison, ptr [[TMP10]], 0
69+
; CHECK-NEXT: [[TMP7:%.*]] = insertvalue %D_struct poison, %D [[TMP6]], 0
70+
; CHECK-NEXT: [[TMP8:%.*]] = insertvalue [1 x %D_struct] poison, %D_struct [[TMP7]], 0
71+
; CHECK-NEXT: [[TMP9:%.*]] = insertvalue %D_arr poison, [1 x %D_struct] [[TMP8]], 0
72+
; CHECK-NEXT: ret %D_arr [[TMP9]]
73+
;
74+
call void @noop()
75+
ret %D_arr zeroinitializer
76+
}

0 commit comments

Comments
 (0)