Skip to content

Commit c598627

Browse files
igorkudrinNoumanAmir657
igorkudrin
authored and
NoumanAmir657
committed
[CFI][LowerTypeTests] Fix indirect call with alias (llvm#113987)
This is a fixed version of llvm#106185, which was reverted in llvm#113978 due to a buildbot failure. Motivation example: ``` > cat test.cpp extern "C" [[gnu::weak]] void f() {} void alias() __attribute__((alias("f"))); int main() { auto p = alias; p(); } > clang test.cpp -fsanitize=cfi-icall -flto=thin -fuse-ld=lld > ./a.out [1] 1868 illegal hardware instruction ./a.out ``` If the address of a function was only taken through its alias, the function was not considered exported and therefore was not included in the CFI jumptable. This resulted in `@llvm.type.test()` being lowered to `false`, and consequently the indirect call to the function was eventually optimized to `ubsantrap()`.
1 parent 2ae30bd commit c598627

File tree

3 files changed

+139
-35
lines changed

3 files changed

+139
-35
lines changed

‎llvm/include/llvm/IR/ModuleSummaryIndexYAML.h

Lines changed: 80 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -135,16 +135,20 @@ template <> struct MappingTraits<TypeIdSummary> {
135135
}
136136
};
137137

138-
struct FunctionSummaryYaml {
138+
struct GlobalValueSummaryYaml {
139+
// Commonly used fields
139140
unsigned Linkage, Visibility;
140141
bool NotEligibleToImport, Live, IsLocal, CanAutoHide;
141142
unsigned ImportType;
142-
std::vector<uint64_t> Refs;
143-
std::vector<uint64_t> TypeTests;
144-
std::vector<FunctionSummary::VFuncId> TypeTestAssumeVCalls,
145-
TypeCheckedLoadVCalls;
146-
std::vector<FunctionSummary::ConstVCall> TypeTestAssumeConstVCalls,
147-
TypeCheckedLoadConstVCalls;
143+
// Fields for AliasSummary
144+
std::optional<uint64_t> Aliasee;
145+
// Fields for FunctionSummary
146+
std::vector<uint64_t> Refs = {};
147+
std::vector<uint64_t> TypeTests = {};
148+
std::vector<FunctionSummary::VFuncId> TypeTestAssumeVCalls = {};
149+
std::vector<FunctionSummary::VFuncId> TypeCheckedLoadVCalls = {};
150+
std::vector<FunctionSummary::ConstVCall> TypeTestAssumeConstVCalls = {};
151+
std::vector<FunctionSummary::ConstVCall> TypeCheckedLoadConstVCalls = {};
148152
};
149153

150154
} // End yaml namespace
@@ -176,15 +180,16 @@ LLVM_YAML_IS_SEQUENCE_VECTOR(FunctionSummary::ConstVCall)
176180
namespace llvm {
177181
namespace yaml {
178182

179-
template <> struct MappingTraits<FunctionSummaryYaml> {
180-
static void mapping(IO &io, FunctionSummaryYaml& summary) {
183+
template <> struct MappingTraits<GlobalValueSummaryYaml> {
184+
static void mapping(IO &io, GlobalValueSummaryYaml &summary) {
181185
io.mapOptional("Linkage", summary.Linkage);
182186
io.mapOptional("Visibility", summary.Visibility);
183187
io.mapOptional("NotEligibleToImport", summary.NotEligibleToImport);
184188
io.mapOptional("Live", summary.Live);
185189
io.mapOptional("Local", summary.IsLocal);
186190
io.mapOptional("CanAutoHide", summary.CanAutoHide);
187191
io.mapOptional("ImportType", summary.ImportType);
192+
io.mapOptional("Aliasee", summary.Aliasee);
188193
io.mapOptional("Refs", summary.Refs);
189194
io.mapOptional("TypeTests", summary.TypeTests);
190195
io.mapOptional("TypeTestAssumeVCalls", summary.TypeTestAssumeVCalls);
@@ -199,69 +204,107 @@ template <> struct MappingTraits<FunctionSummaryYaml> {
199204
} // End yaml namespace
200205
} // End llvm namespace
201206

202-
LLVM_YAML_IS_SEQUENCE_VECTOR(FunctionSummaryYaml)
207+
LLVM_YAML_IS_SEQUENCE_VECTOR(GlobalValueSummaryYaml)
203208

204209
namespace llvm {
205210
namespace yaml {
206211

207212
// FIXME: Add YAML mappings for the rest of the module summary.
208213
template <> struct CustomMappingTraits<GlobalValueSummaryMapTy> {
209214
static void inputOne(IO &io, StringRef Key, GlobalValueSummaryMapTy &V) {
210-
std::vector<FunctionSummaryYaml> FSums;
211-
io.mapRequired(Key.str().c_str(), FSums);
215+
std::vector<GlobalValueSummaryYaml> GVSums;
216+
io.mapRequired(Key.str().c_str(), GVSums);
212217
uint64_t KeyInt;
213218
if (Key.getAsInteger(0, KeyInt)) {
214219
io.setError("key not an integer");
215220
return;
216221
}
217222
auto &Elem = V.try_emplace(KeyInt, /*IsAnalysis=*/false).first->second;
218-
for (auto &FSum : FSums) {
223+
for (auto &GVSum : GVSums) {
224+
GlobalValueSummary::GVFlags GVFlags(
225+
static_cast<GlobalValue::LinkageTypes>(GVSum.Linkage),
226+
static_cast<GlobalValue::VisibilityTypes>(GVSum.Visibility),
227+
GVSum.NotEligibleToImport, GVSum.Live, GVSum.IsLocal,
228+
GVSum.CanAutoHide,
229+
static_cast<GlobalValueSummary::ImportKind>(GVSum.ImportType));
230+
if (GVSum.Aliasee) {
231+
auto ASum = std::make_unique<AliasSummary>(GVFlags);
232+
if (!V.count(*GVSum.Aliasee))
233+
V.emplace(*GVSum.Aliasee, /*IsAnalysis=*/false);
234+
ValueInfo AliaseeVI(/*IsAnalysis=*/false, &*V.find(*GVSum.Aliasee));
235+
// Note: Aliasee cannot be filled until all summaries are loaded.
236+
// This is done in fixAliaseeLinks() which is called in
237+
// MappingTraits<ModuleSummaryIndex>::mapping().
238+
ASum->setAliasee(AliaseeVI, /*Aliasee=*/nullptr);
239+
Elem.SummaryList.push_back(std::move(ASum));
240+
continue;
241+
}
219242
SmallVector<ValueInfo, 0> Refs;
220-
Refs.reserve(FSum.Refs.size());
221-
for (auto &RefGUID : FSum.Refs) {
243+
Refs.reserve(GVSum.Refs.size());
244+
for (auto &RefGUID : GVSum.Refs) {
222245
auto It = V.try_emplace(RefGUID, /*IsAnalysis=*/false).first;
223246
Refs.push_back(ValueInfo(/*IsAnalysis=*/false, &*It));
224247
}
225248
Elem.SummaryList.push_back(std::make_unique<FunctionSummary>(
226-
GlobalValueSummary::GVFlags(
227-
static_cast<GlobalValue::LinkageTypes>(FSum.Linkage),
228-
static_cast<GlobalValue::VisibilityTypes>(FSum.Visibility),
229-
FSum.NotEligibleToImport, FSum.Live, FSum.IsLocal,
230-
FSum.CanAutoHide,
231-
static_cast<GlobalValueSummary::ImportKind>(FSum.ImportType)),
232-
/*NumInsts=*/0, FunctionSummary::FFlags{}, std::move(Refs),
233-
SmallVector<FunctionSummary::EdgeTy, 0>{}, std::move(FSum.TypeTests),
234-
std::move(FSum.TypeTestAssumeVCalls),
235-
std::move(FSum.TypeCheckedLoadVCalls),
236-
std::move(FSum.TypeTestAssumeConstVCalls),
237-
std::move(FSum.TypeCheckedLoadConstVCalls),
249+
GVFlags, /*NumInsts=*/0, FunctionSummary::FFlags{}, std::move(Refs),
250+
SmallVector<FunctionSummary::EdgeTy, 0>{}, std::move(GVSum.TypeTests),
251+
std::move(GVSum.TypeTestAssumeVCalls),
252+
std::move(GVSum.TypeCheckedLoadVCalls),
253+
std::move(GVSum.TypeTestAssumeConstVCalls),
254+
std::move(GVSum.TypeCheckedLoadConstVCalls),
238255
ArrayRef<FunctionSummary::ParamAccess>{}, ArrayRef<CallsiteInfo>{},
239256
ArrayRef<AllocInfo>{}));
240257
}
241258
}
242259
static void output(IO &io, GlobalValueSummaryMapTy &V) {
243260
for (auto &P : V) {
244-
std::vector<FunctionSummaryYaml> FSums;
261+
std::vector<GlobalValueSummaryYaml> GVSums;
245262
for (auto &Sum : P.second.SummaryList) {
246263
if (auto *FSum = dyn_cast<FunctionSummary>(Sum.get())) {
247264
std::vector<uint64_t> Refs;
248265
Refs.reserve(FSum->refs().size());
249266
for (auto &VI : FSum->refs())
250267
Refs.push_back(VI.getGUID());
251-
FSums.push_back(FunctionSummaryYaml{
268+
GVSums.push_back(GlobalValueSummaryYaml{
252269
FSum->flags().Linkage, FSum->flags().Visibility,
253270
static_cast<bool>(FSum->flags().NotEligibleToImport),
254271
static_cast<bool>(FSum->flags().Live),
255272
static_cast<bool>(FSum->flags().DSOLocal),
256273
static_cast<bool>(FSum->flags().CanAutoHide),
257-
FSum->flags().ImportType, Refs, FSum->type_tests(),
258-
FSum->type_test_assume_vcalls(), FSum->type_checked_load_vcalls(),
274+
FSum->flags().ImportType, /*Aliasee=*/std::nullopt, Refs,
275+
FSum->type_tests(), FSum->type_test_assume_vcalls(),
276+
FSum->type_checked_load_vcalls(),
259277
FSum->type_test_assume_const_vcalls(),
260278
FSum->type_checked_load_const_vcalls()});
261-
}
279+
} else if (auto *ASum = dyn_cast<AliasSummary>(Sum.get());
280+
ASum && ASum->hasAliasee()) {
281+
GVSums.push_back(GlobalValueSummaryYaml{
282+
ASum->flags().Linkage, ASum->flags().Visibility,
283+
static_cast<bool>(ASum->flags().NotEligibleToImport),
284+
static_cast<bool>(ASum->flags().Live),
285+
static_cast<bool>(ASum->flags().DSOLocal),
286+
static_cast<bool>(ASum->flags().CanAutoHide),
287+
ASum->flags().ImportType,
288+
/*Aliasee=*/ASum->getAliaseeGUID()});
289+
}
290+
}
291+
if (!GVSums.empty())
292+
io.mapRequired(llvm::utostr(P.first).c_str(), GVSums);
293+
}
294+
}
295+
static void fixAliaseeLinks(GlobalValueSummaryMapTy &V) {
296+
for (auto &P : V) {
297+
for (auto &Sum : P.second.SummaryList) {
298+
if (auto *Alias = dyn_cast<AliasSummary>(Sum.get())) {
299+
ValueInfo AliaseeVI = Alias->getAliaseeVI();
300+
auto AliaseeSL = AliaseeVI.getSummaryList();
301+
if (AliaseeSL.empty()) {
302+
ValueInfo EmptyVI;
303+
Alias->setAliasee(EmptyVI, nullptr);
304+
} else
305+
Alias->setAliasee(AliaseeVI, AliaseeSL[0].get());
306+
}
262307
}
263-
if (!FSums.empty())
264-
io.mapRequired(llvm::utostr(P.first).c_str(), FSums);
265308
}
266309
}
267310
};
@@ -281,6 +324,9 @@ template <> struct CustomMappingTraits<TypeIdSummaryMapTy> {
281324
template <> struct MappingTraits<ModuleSummaryIndex> {
282325
static void mapping(IO &io, ModuleSummaryIndex& index) {
283326
io.mapOptional("GlobalValueMap", index.GlobalValueMap);
327+
if (!io.outputting())
328+
CustomMappingTraits<GlobalValueSummaryMapTy>::fixAliaseeLinks(
329+
index.GlobalValueMap);
284330
io.mapOptional("TypeIdMap", index.TypeIdMap);
285331
io.mapOptional("WithGlobalValueDeadStripping",
286332
index.WithGlobalValueDeadStripping);

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2096,8 +2096,12 @@ bool LowerTypeTestsModule::lower() {
20962096
for (auto &I : *ExportSummary)
20972097
for (auto &GVS : I.second.SummaryList)
20982098
if (GVS->isLive())
2099-
for (const auto &Ref : GVS->refs())
2099+
for (const auto &Ref : GVS->refs()) {
21002100
AddressTaken.insert(Ref.getGUID());
2101+
for (auto &RefGVS : Ref.getSummaryList())
2102+
if (auto Alias = dyn_cast<AliasSummary>(RefGVS.get()))
2103+
AddressTaken.insert(Alias->getAliaseeGUID());
2104+
}
21012105

21022106
NamedMDNode *CfiFunctionsMD = M.getNamedMetadata("cfi.functions");
21032107
if (CfiFunctionsMD) {

‎llvm/test/Transforms/LowerTypeTests/cfi-icall-alias.ll

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
;; Check that if the address of a weak function is only taken through an alias,
2+
;; it is still added to a list of exported functions and @llvm.type.test() is
3+
;; lowered to an actual check against the generated CFI jumptable.
4+
5+
RUN: rm -rf %t.dir && split-file %s %t.dir && cd %t.dir
6+
RUN: opt test.ll --thinlto-bc --thinlto-split-lto-unit -o test.bc
7+
RUN: llvm-modextract test.bc -n 0 -o test0.bc
8+
RUN: llvm-modextract test.bc -n 1 -o test1.bc
9+
10+
;; Check that a CFI jumptable is generated.
11+
RUN: opt test1.bc -passes=lowertypetests -lowertypetests-read-summary=in.yaml \
12+
RUN: -lowertypetests-summary-action=export -lowertypetests-write-summary=exported.yaml \
13+
RUN: -S -o - | FileCheck %s --check-prefix=REGULAR
14+
REGULAR: @__typeid__ZTSFvvE_global_addr = hidden alias i8, ptr @.cfi.jumptable
15+
REGULAR: @f = alias void (), ptr @.cfi.jumptable
16+
REGULAR: define private void @.cfi.jumptable()
17+
18+
;; CHECK that @llvm.type.test() is lowered to an actual check.
19+
RUN: opt test0.bc -passes=lowertypetests -lowertypetests-read-summary=exported.yaml \
20+
RUN: -lowertypetests-summary-action=import -S -o - | FileCheck %s --check-prefix=THIN
21+
THIN: define i1 @test() {
22+
THIN-NEXT: %1 = icmp eq i64 ptrtoint (ptr @alias to i64), ptrtoint (ptr @__typeid__ZTSFvvE_global_addr to i64)
23+
THIN-NEXT: ret i1 %1
24+
THIN-NEXT: }
25+
26+
;--- test.ll
27+
target triple = "x86_64-pc-linux-gnu"
28+
29+
@alias = alias void(), ptr @f
30+
31+
define weak void @f() !type !0 {
32+
ret void
33+
}
34+
35+
define i1 @test() {
36+
%1 = call i1 @llvm.type.test(ptr nonnull @alias, metadata !"_ZTSFvvE")
37+
ret i1 %1
38+
}
39+
40+
declare i1 @llvm.type.test(ptr, metadata)
41+
42+
!0 = !{i64 0, !"_ZTSFvvE"}
43+
;--- in.yaml
44+
---
45+
GlobalValueMap:
46+
8346051122425466633: # guid("test")
47+
- Live: true
48+
Refs: [5833419078793185394] # guid("alias")
49+
TypeTests: [9080559750644022485] # guid("_ZTSFvvE")
50+
5833419078793185394: # guid("alias")
51+
- Aliasee: 14740650423002898831 # guid("f")
52+
14740650423002898831: # guid("f")
53+
-
54+
...

0 commit comments

Comments
 (0)