Skip to content

Commit 2526d13

Browse files
igorkudrinigorkudrin
committed
[CFI][LowerTypeTests] Fix indirect call with alias.
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 is only taken through its alias, the function was not considered as 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 `unsantrap()`.
1 parent 6a74b0e commit 2526d13

File tree

3 files changed

+132
-28
lines changed

3 files changed

+132
-28
lines changed

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

Lines changed: 73 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -135,10 +135,14 @@ 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;
143+
// Fields for AliasSummary
144+
std::optional<uint64_t> Aliasee;
145+
// Fields for FunctionSummary
142146
std::vector<uint64_t> Refs;
143147
std::vector<uint64_t> TypeTests;
144148
std::vector<FunctionSummary::VFuncId> TypeTestAssumeVCalls,
@@ -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,16 +204,16 @@ 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");
@@ -217,52 +222,90 @@ template <> struct CustomMappingTraits<GlobalValueSummaryMapTy> {
217222
if (!V.count(KeyInt))
218223
V.emplace(KeyInt, /*IsAnalysis=*/false);
219224
auto &Elem = V.find(KeyInt)->second;
220-
for (auto &FSum : FSums) {
225+
for (auto &GVSum : GVSums) {
226+
GlobalValueSummary::GVFlags GVFlags(
227+
static_cast<GlobalValue::LinkageTypes>(GVSum.Linkage),
228+
static_cast<GlobalValue::VisibilityTypes>(GVSum.Visibility),
229+
GVSum.NotEligibleToImport, GVSum.Live, GVSum.IsLocal,
230+
GVSum.CanAutoHide,
231+
static_cast<GlobalValueSummary::ImportKind>(GVSum.ImportType));
232+
if (GVSum.Aliasee) {
233+
auto ASum = std::make_unique<AliasSummary>(GVFlags);
234+
if (!V.count(*GVSum.Aliasee))
235+
V.emplace(*GVSum.Aliasee, /*IsAnalysis=*/false);
236+
ValueInfo AliaseeVI(/*IsAnalysis=*/false, &*V.find(*GVSum.Aliasee));
237+
// Note: Aliasee cannot be filled until all summaries are loaded.
238+
// This is done in fixAliaseeLinks() which is called in
239+
// MappingTraits<ModuleSummaryIndex>::mapping().
240+
ASum->setAliasee(AliaseeVI, /*Aliasee=*/nullptr);
241+
Elem.SummaryList.push_back(std::move(ASum));
242+
continue;
243+
}
221244
std::vector<ValueInfo> Refs;
222-
for (auto &RefGUID : FSum.Refs) {
245+
for (auto &RefGUID : GVSum.Refs) {
223246
if (!V.count(RefGUID))
224247
V.emplace(RefGUID, /*IsAnalysis=*/false);
225248
Refs.push_back(ValueInfo(/*IsAnalysis=*/false, &*V.find(RefGUID)));
226249
}
227250
Elem.SummaryList.push_back(std::make_unique<FunctionSummary>(
228-
GlobalValueSummary::GVFlags(
229-
static_cast<GlobalValue::LinkageTypes>(FSum.Linkage),
230-
static_cast<GlobalValue::VisibilityTypes>(FSum.Visibility),
231-
FSum.NotEligibleToImport, FSum.Live, FSum.IsLocal,
232-
FSum.CanAutoHide,
233-
static_cast<GlobalValueSummary::ImportKind>(FSum.ImportType)),
234-
/*NumInsts=*/0, FunctionSummary::FFlags{}, /*EntryCount=*/0, Refs,
235-
ArrayRef<FunctionSummary::EdgeTy>{}, std::move(FSum.TypeTests),
236-
std::move(FSum.TypeTestAssumeVCalls),
237-
std::move(FSum.TypeCheckedLoadVCalls),
238-
std::move(FSum.TypeTestAssumeConstVCalls),
239-
std::move(FSum.TypeCheckedLoadConstVCalls),
251+
GVFlags, /*NumInsts=*/0, FunctionSummary::FFlags{}, /*EntryCount=*/0,
252+
Refs, ArrayRef<FunctionSummary::EdgeTy>{}, std::move(GVSum.TypeTests),
253+
std::move(GVSum.TypeTestAssumeVCalls),
254+
std::move(GVSum.TypeCheckedLoadVCalls),
255+
std::move(GVSum.TypeTestAssumeConstVCalls),
256+
std::move(GVSum.TypeCheckedLoadConstVCalls),
240257
ArrayRef<FunctionSummary::ParamAccess>{}, ArrayRef<CallsiteInfo>{},
241258
ArrayRef<AllocInfo>{}));
242259
}
243260
}
244261
static void output(IO &io, GlobalValueSummaryMapTy &V) {
245262
for (auto &P : V) {
246-
std::vector<FunctionSummaryYaml> FSums;
263+
std::vector<GlobalValueSummaryYaml> GVSums;
247264
for (auto &Sum : P.second.SummaryList) {
248265
if (auto *FSum = dyn_cast<FunctionSummary>(Sum.get())) {
249266
std::vector<uint64_t> Refs;
250267
for (auto &VI : FSum->refs())
251268
Refs.push_back(VI.getGUID());
252-
FSums.push_back(FunctionSummaryYaml{
269+
GVSums.push_back(GlobalValueSummaryYaml{
253270
FSum->flags().Linkage, FSum->flags().Visibility,
254271
static_cast<bool>(FSum->flags().NotEligibleToImport),
255272
static_cast<bool>(FSum->flags().Live),
256273
static_cast<bool>(FSum->flags().DSOLocal),
257274
static_cast<bool>(FSum->flags().CanAutoHide),
258-
FSum->flags().ImportType, Refs, FSum->type_tests(),
259-
FSum->type_test_assume_vcalls(), FSum->type_checked_load_vcalls(),
275+
FSum->flags().ImportType, /*Aliasee=*/std::nullopt, Refs,
276+
FSum->type_tests(), FSum->type_test_assume_vcalls(),
277+
FSum->type_checked_load_vcalls(),
260278
FSum->type_test_assume_const_vcalls(),
261279
FSum->type_checked_load_const_vcalls()});
262-
}
280+
} else if (auto *ASum = dyn_cast<AliasSummary>(Sum.get());
281+
ASum && ASum->hasAliasee()) {
282+
GVSums.push_back(GlobalValueSummaryYaml{
283+
ASum->flags().Linkage, ASum->flags().Visibility,
284+
static_cast<bool>(ASum->flags().NotEligibleToImport),
285+
static_cast<bool>(ASum->flags().Live),
286+
static_cast<bool>(ASum->flags().DSOLocal),
287+
static_cast<bool>(ASum->flags().CanAutoHide),
288+
ASum->flags().ImportType,
289+
/*Aliasee=*/ASum->getAliaseeGUID()});
290+
}
291+
}
292+
if (!GVSums.empty())
293+
io.mapRequired(llvm::utostr(P.first).c_str(), GVSums);
294+
}
295+
}
296+
static void fixAliaseeLinks(GlobalValueSummaryMapTy &V) {
297+
for (auto &P : V) {
298+
for (auto &Sum : P.second.SummaryList) {
299+
if (auto *Alias = dyn_cast<AliasSummary>(Sum.get())) {
300+
ValueInfo AliaseeVI = Alias->getAliaseeVI();
301+
auto AliaseeSL = AliaseeVI.getSummaryList();
302+
if (AliaseeSL.empty()) {
303+
ValueInfo EmptyVI;
304+
Alias->setAliasee(EmptyVI, nullptr);
305+
} else
306+
Alias->setAliasee(AliaseeVI, AliaseeSL[0].get());
307+
}
263308
}
264-
if (!FSums.empty())
265-
io.mapRequired(llvm::utostr(P.first).c_str(), FSums);
266309
}
267310
}
268311
};
@@ -282,6 +325,9 @@ template <> struct CustomMappingTraits<TypeIdSummaryMapTy> {
282325
template <> struct MappingTraits<ModuleSummaryIndex> {
283326
static void mapping(IO &io, ModuleSummaryIndex& index) {
284327
io.mapOptional("GlobalValueMap", index.GlobalValueMap);
328+
if (!io.outputting())
329+
CustomMappingTraits<GlobalValueSummaryMapTy>::fixAliaseeLinks(
330+
index.GlobalValueMap);
285331
io.mapOptional("TypeIdMap", index.TypeIdMap);
286332
io.mapOptional("WithGlobalValueDeadStripping",
287333
index.WithGlobalValueDeadStripping);

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2083,8 +2083,12 @@ bool LowerTypeTestsModule::lower() {
20832083
for (auto &I : *ExportSummary)
20842084
for (auto &GVS : I.second.SummaryList)
20852085
if (GVS->isLive())
2086-
for (const auto &Ref : GVS->refs())
2086+
for (const auto &Ref : GVS->refs()) {
20872087
AddressTaken.insert(Ref.getGUID());
2088+
for (auto &RefGVS : Ref.getSummaryList())
2089+
if (auto Alias = dyn_cast<AliasSummary>(RefGVS.get()))
2090+
AddressTaken.insert(Alias->getAliaseeGUID());
2091+
}
20882092

20892093
NamedMDNode *CfiFunctionsMD = M.getNamedMetadata("cfi.functions");
20902094
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)