[llvm] [CFI][LowerTypeTests] Fix indirect call with alias. (PR #113987)
via llvm-commits
llvm-commits at lists.llvm.org
Mon Oct 28 17:44:30 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lto
@llvm/pr-subscribers-llvm-ir
Author: Igor Kudrin (igorkudrin)
<details>
<summary>Changes</summary>
This is a fixed version of #<!-- -->106185, which was reverted in #<!-- -->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()`.
---
Full diff: https://github.com/llvm/llvm-project/pull/113987.diff
3 Files Affected:
- (modified) llvm/include/llvm/IR/ModuleSummaryIndexYAML.h (+80-34)
- (modified) llvm/lib/Transforms/IPO/LowerTypeTests.cpp (+5-1)
- (added) llvm/test/Transforms/LowerTypeTests/cfi-icall-alias.ll (+54)
``````````diff
diff --git a/llvm/include/llvm/IR/ModuleSummaryIndexYAML.h b/llvm/include/llvm/IR/ModuleSummaryIndexYAML.h
index 902d1305c818ac..7c405025630c95 100644
--- a/llvm/include/llvm/IR/ModuleSummaryIndexYAML.h
+++ b/llvm/include/llvm/IR/ModuleSummaryIndexYAML.h
@@ -135,16 +135,20 @@ template <> struct MappingTraits<TypeIdSummary> {
}
};
-struct FunctionSummaryYaml {
+struct GlobalValueSummaryYaml {
+ // Commonly used fields
unsigned Linkage, Visibility;
bool NotEligibleToImport, Live, IsLocal, CanAutoHide;
unsigned ImportType;
- std::vector<uint64_t> Refs;
- std::vector<uint64_t> TypeTests;
- std::vector<FunctionSummary::VFuncId> TypeTestAssumeVCalls,
- TypeCheckedLoadVCalls;
- std::vector<FunctionSummary::ConstVCall> TypeTestAssumeConstVCalls,
- TypeCheckedLoadConstVCalls;
+ // Fields for AliasSummary
+ std::optional<uint64_t> Aliasee;
+ // Fields for FunctionSummary
+ std::vector<uint64_t> Refs = {};
+ std::vector<uint64_t> TypeTests = {};
+ std::vector<FunctionSummary::VFuncId> TypeTestAssumeVCalls = {};
+ std::vector<FunctionSummary::VFuncId> TypeCheckedLoadVCalls = {};
+ std::vector<FunctionSummary::ConstVCall> TypeTestAssumeConstVCalls = {};
+ std::vector<FunctionSummary::ConstVCall> TypeCheckedLoadConstVCalls = {};
};
} // End yaml namespace
@@ -176,8 +180,8 @@ LLVM_YAML_IS_SEQUENCE_VECTOR(FunctionSummary::ConstVCall)
namespace llvm {
namespace yaml {
-template <> struct MappingTraits<FunctionSummaryYaml> {
- static void mapping(IO &io, FunctionSummaryYaml& summary) {
+template <> struct MappingTraits<GlobalValueSummaryYaml> {
+ static void mapping(IO &io, GlobalValueSummaryYaml &summary) {
io.mapOptional("Linkage", summary.Linkage);
io.mapOptional("Visibility", summary.Visibility);
io.mapOptional("NotEligibleToImport", summary.NotEligibleToImport);
@@ -185,6 +189,7 @@ template <> struct MappingTraits<FunctionSummaryYaml> {
io.mapOptional("Local", summary.IsLocal);
io.mapOptional("CanAutoHide", summary.CanAutoHide);
io.mapOptional("ImportType", summary.ImportType);
+ io.mapOptional("Aliasee", summary.Aliasee);
io.mapOptional("Refs", summary.Refs);
io.mapOptional("TypeTests", summary.TypeTests);
io.mapOptional("TypeTestAssumeVCalls", summary.TypeTestAssumeVCalls);
@@ -199,7 +204,7 @@ template <> struct MappingTraits<FunctionSummaryYaml> {
} // End yaml namespace
} // End llvm namespace
-LLVM_YAML_IS_SEQUENCE_VECTOR(FunctionSummaryYaml)
+LLVM_YAML_IS_SEQUENCE_VECTOR(GlobalValueSummaryYaml)
namespace llvm {
namespace yaml {
@@ -207,61 +212,99 @@ namespace yaml {
// FIXME: Add YAML mappings for the rest of the module summary.
template <> struct CustomMappingTraits<GlobalValueSummaryMapTy> {
static void inputOne(IO &io, StringRef Key, GlobalValueSummaryMapTy &V) {
- std::vector<FunctionSummaryYaml> FSums;
- io.mapRequired(Key.str().c_str(), FSums);
+ std::vector<GlobalValueSummaryYaml> GVSums;
+ io.mapRequired(Key.str().c_str(), GVSums);
uint64_t KeyInt;
if (Key.getAsInteger(0, KeyInt)) {
io.setError("key not an integer");
return;
}
auto &Elem = V.try_emplace(KeyInt, /*IsAnalysis=*/false).first->second;
- for (auto &FSum : FSums) {
+ for (auto &GVSum : GVSums) {
+ GlobalValueSummary::GVFlags GVFlags(
+ static_cast<GlobalValue::LinkageTypes>(GVSum.Linkage),
+ static_cast<GlobalValue::VisibilityTypes>(GVSum.Visibility),
+ GVSum.NotEligibleToImport, GVSum.Live, GVSum.IsLocal,
+ GVSum.CanAutoHide,
+ static_cast<GlobalValueSummary::ImportKind>(GVSum.ImportType));
+ if (GVSum.Aliasee) {
+ auto ASum = std::make_unique<AliasSummary>(GVFlags);
+ if (!V.count(*GVSum.Aliasee))
+ V.emplace(*GVSum.Aliasee, /*IsAnalysis=*/false);
+ ValueInfo AliaseeVI(/*IsAnalysis=*/false, &*V.find(*GVSum.Aliasee));
+ // Note: Aliasee cannot be filled until all summaries are loaded.
+ // This is done in fixAliaseeLinks() which is called in
+ // MappingTraits<ModuleSummaryIndex>::mapping().
+ ASum->setAliasee(AliaseeVI, /*Aliasee=*/nullptr);
+ Elem.SummaryList.push_back(std::move(ASum));
+ continue;
+ }
SmallVector<ValueInfo, 0> Refs;
- Refs.reserve(FSum.Refs.size());
- for (auto &RefGUID : FSum.Refs) {
+ Refs.reserve(GVSum.Refs.size());
+ for (auto &RefGUID : GVSum.Refs) {
auto It = V.try_emplace(RefGUID, /*IsAnalysis=*/false).first;
Refs.push_back(ValueInfo(/*IsAnalysis=*/false, &*It));
}
Elem.SummaryList.push_back(std::make_unique<FunctionSummary>(
- GlobalValueSummary::GVFlags(
- static_cast<GlobalValue::LinkageTypes>(FSum.Linkage),
- static_cast<GlobalValue::VisibilityTypes>(FSum.Visibility),
- FSum.NotEligibleToImport, FSum.Live, FSum.IsLocal,
- FSum.CanAutoHide,
- static_cast<GlobalValueSummary::ImportKind>(FSum.ImportType)),
- /*NumInsts=*/0, FunctionSummary::FFlags{}, std::move(Refs),
- SmallVector<FunctionSummary::EdgeTy, 0>{}, std::move(FSum.TypeTests),
- std::move(FSum.TypeTestAssumeVCalls),
- std::move(FSum.TypeCheckedLoadVCalls),
- std::move(FSum.TypeTestAssumeConstVCalls),
- std::move(FSum.TypeCheckedLoadConstVCalls),
+ GVFlags, /*NumInsts=*/0, FunctionSummary::FFlags{}, std::move(Refs),
+ SmallVector<FunctionSummary::EdgeTy, 0>{}, std::move(GVSum.TypeTests),
+ std::move(GVSum.TypeTestAssumeVCalls),
+ std::move(GVSum.TypeCheckedLoadVCalls),
+ std::move(GVSum.TypeTestAssumeConstVCalls),
+ std::move(GVSum.TypeCheckedLoadConstVCalls),
ArrayRef<FunctionSummary::ParamAccess>{}, ArrayRef<CallsiteInfo>{},
ArrayRef<AllocInfo>{}));
}
}
static void output(IO &io, GlobalValueSummaryMapTy &V) {
for (auto &P : V) {
- std::vector<FunctionSummaryYaml> FSums;
+ std::vector<GlobalValueSummaryYaml> GVSums;
for (auto &Sum : P.second.SummaryList) {
if (auto *FSum = dyn_cast<FunctionSummary>(Sum.get())) {
std::vector<uint64_t> Refs;
Refs.reserve(FSum->refs().size());
for (auto &VI : FSum->refs())
Refs.push_back(VI.getGUID());
- FSums.push_back(FunctionSummaryYaml{
+ GVSums.push_back(GlobalValueSummaryYaml{
FSum->flags().Linkage, FSum->flags().Visibility,
static_cast<bool>(FSum->flags().NotEligibleToImport),
static_cast<bool>(FSum->flags().Live),
static_cast<bool>(FSum->flags().DSOLocal),
static_cast<bool>(FSum->flags().CanAutoHide),
- FSum->flags().ImportType, Refs, FSum->type_tests(),
- FSum->type_test_assume_vcalls(), FSum->type_checked_load_vcalls(),
+ FSum->flags().ImportType, /*Aliasee=*/std::nullopt, Refs,
+ FSum->type_tests(), FSum->type_test_assume_vcalls(),
+ FSum->type_checked_load_vcalls(),
FSum->type_test_assume_const_vcalls(),
FSum->type_checked_load_const_vcalls()});
- }
+ } else if (auto *ASum = dyn_cast<AliasSummary>(Sum.get());
+ ASum && ASum->hasAliasee()) {
+ GVSums.push_back(GlobalValueSummaryYaml{
+ ASum->flags().Linkage, ASum->flags().Visibility,
+ static_cast<bool>(ASum->flags().NotEligibleToImport),
+ static_cast<bool>(ASum->flags().Live),
+ static_cast<bool>(ASum->flags().DSOLocal),
+ static_cast<bool>(ASum->flags().CanAutoHide),
+ ASum->flags().ImportType,
+ /*Aliasee=*/ASum->getAliaseeGUID()});
+ }
+ }
+ if (!GVSums.empty())
+ io.mapRequired(llvm::utostr(P.first).c_str(), GVSums);
+ }
+ }
+ static void fixAliaseeLinks(GlobalValueSummaryMapTy &V) {
+ for (auto &P : V) {
+ for (auto &Sum : P.second.SummaryList) {
+ if (auto *Alias = dyn_cast<AliasSummary>(Sum.get())) {
+ ValueInfo AliaseeVI = Alias->getAliaseeVI();
+ auto AliaseeSL = AliaseeVI.getSummaryList();
+ if (AliaseeSL.empty()) {
+ ValueInfo EmptyVI;
+ Alias->setAliasee(EmptyVI, nullptr);
+ } else
+ Alias->setAliasee(AliaseeVI, AliaseeSL[0].get());
+ }
}
- if (!FSums.empty())
- io.mapRequired(llvm::utostr(P.first).c_str(), FSums);
}
}
};
@@ -281,6 +324,9 @@ template <> struct CustomMappingTraits<TypeIdSummaryMapTy> {
template <> struct MappingTraits<ModuleSummaryIndex> {
static void mapping(IO &io, ModuleSummaryIndex& index) {
io.mapOptional("GlobalValueMap", index.GlobalValueMap);
+ if (!io.outputting())
+ CustomMappingTraits<GlobalValueSummaryMapTy>::fixAliaseeLinks(
+ index.GlobalValueMap);
io.mapOptional("TypeIdMap", index.TypeIdMap);
io.mapOptional("WithGlobalValueDeadStripping",
index.WithGlobalValueDeadStripping);
diff --git a/llvm/lib/Transforms/IPO/LowerTypeTests.cpp b/llvm/lib/Transforms/IPO/LowerTypeTests.cpp
index 3fcfc6a876776d..6ba371069bb230 100644
--- a/llvm/lib/Transforms/IPO/LowerTypeTests.cpp
+++ b/llvm/lib/Transforms/IPO/LowerTypeTests.cpp
@@ -2083,8 +2083,12 @@ bool LowerTypeTestsModule::lower() {
for (auto &I : *ExportSummary)
for (auto &GVS : I.second.SummaryList)
if (GVS->isLive())
- for (const auto &Ref : GVS->refs())
+ for (const auto &Ref : GVS->refs()) {
AddressTaken.insert(Ref.getGUID());
+ for (auto &RefGVS : Ref.getSummaryList())
+ if (auto Alias = dyn_cast<AliasSummary>(RefGVS.get()))
+ AddressTaken.insert(Alias->getAliaseeGUID());
+ }
NamedMDNode *CfiFunctionsMD = M.getNamedMetadata("cfi.functions");
if (CfiFunctionsMD) {
diff --git a/llvm/test/Transforms/LowerTypeTests/cfi-icall-alias.ll b/llvm/test/Transforms/LowerTypeTests/cfi-icall-alias.ll
new file mode 100644
index 00000000000000..0c5324ee96c939
--- /dev/null
+++ b/llvm/test/Transforms/LowerTypeTests/cfi-icall-alias.ll
@@ -0,0 +1,54 @@
+;; Check that if the address of a weak function is only taken through an alias,
+;; it is still added to a list of exported functions and @llvm.type.test() is
+;; lowered to an actual check against the generated CFI jumptable.
+
+RUN: rm -rf %t.dir && split-file %s %t.dir && cd %t.dir
+RUN: opt test.ll --thinlto-bc --thinlto-split-lto-unit -o test.bc
+RUN: llvm-modextract test.bc -n 0 -o test0.bc
+RUN: llvm-modextract test.bc -n 1 -o test1.bc
+
+;; Check that a CFI jumptable is generated.
+RUN: opt test1.bc -passes=lowertypetests -lowertypetests-read-summary=in.yaml \
+RUN: -lowertypetests-summary-action=export -lowertypetests-write-summary=exported.yaml \
+RUN: -S -o - | FileCheck %s --check-prefix=REGULAR
+REGULAR: @__typeid__ZTSFvvE_global_addr = hidden alias i8, ptr @.cfi.jumptable
+REGULAR: @f = alias void (), ptr @.cfi.jumptable
+REGULAR: define private void @.cfi.jumptable()
+
+;; CHECK that @llvm.type.test() is lowered to an actual check.
+RUN: opt test0.bc -passes=lowertypetests -lowertypetests-read-summary=exported.yaml \
+RUN: -lowertypetests-summary-action=import -S -o - | FileCheck %s --check-prefix=THIN
+THIN: define i1 @test() {
+THIN-NEXT: %1 = icmp eq i64 ptrtoint (ptr @alias to i64), ptrtoint (ptr @__typeid__ZTSFvvE_global_addr to i64)
+THIN-NEXT: ret i1 %1
+THIN-NEXT: }
+
+;--- test.ll
+target triple = "x86_64-pc-linux-gnu"
+
+ at alias = alias void(), ptr @f
+
+define weak void @f() !type !0 {
+ ret void
+}
+
+define i1 @test() {
+ %1 = call i1 @llvm.type.test(ptr nonnull @alias, metadata !"_ZTSFvvE")
+ ret i1 %1
+}
+
+declare i1 @llvm.type.test(ptr, metadata)
+
+!0 = !{i64 0, !"_ZTSFvvE"}
+;--- in.yaml
+---
+GlobalValueMap:
+ 8346051122425466633: # guid("test")
+ - Live: true
+ Refs: [5833419078793185394] # guid("alias")
+ TypeTests: [9080559750644022485] # guid("_ZTSFvvE")
+ 5833419078793185394: # guid("alias")
+ - Aliasee: 14740650423002898831 # guid("f")
+ 14740650423002898831: # guid("f")
+ -
+...
``````````
</details>
https://github.com/llvm/llvm-project/pull/113987
More information about the llvm-commits
mailing list