[llvm] 90a6bb3 - [remangleIntrinsicFunction] Detect and resolve name clash
Jeroen Dobbelaere via llvm-commits
llvm-commits at lists.llvm.org
Tue Jul 13 02:40:06 PDT 2021
Author: Jeroen Dobbelaere
Date: 2021-07-13T11:21:12+02:00
New Revision: 90a6bb30fafa4e68d4af1fef62987fe187fa70ab
URL: https://github.com/llvm/llvm-project/commit/90a6bb30fafa4e68d4af1fef62987fe187fa70ab
DIFF: https://github.com/llvm/llvm-project/commit/90a6bb30fafa4e68d4af1fef62987fe187fa70ab.diff
LOG: [remangleIntrinsicFunction] Detect and resolve name clash
It is possible that the remangled name for an intrinsic already exists with a different (and wrong) prototype within the module.
As the bitcode reader keeps both versions of all remangled intrinsics around for a longer time, this can result in a
crash, as can be seen in https://bugs.llvm.org/show_bug.cgi?id=50923
This patch makes 'remangleIntrinsicFunction' aware of this situation. When it is detected, it moves the version with the wrong prototype to a different name. That version will be removed anyway once the module is completely loaded.
With thanks to @asbirlea for reporting this issue when trying out an lto build with the full restrict patches, and @efriedma for suggesting a sane resolution mechanism.
Reviewed By: apilipenko
Differential Revision: https://reviews.llvm.org/D105118
Added:
llvm/test/Assembler/remangle.ll
llvm/test/tools/llvm-link/Inputs/remangle1.ll
llvm/test/tools/llvm-link/Inputs/remangle2.ll
llvm/test/tools/llvm-link/remangle.ll
Modified:
llvm/include/llvm/IR/Intrinsics.h
llvm/lib/IR/Function.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/IR/Intrinsics.h b/llvm/include/llvm/IR/Intrinsics.h
index 15275001ab4c6..80a2f5a8cd3e4 100644
--- a/llvm/include/llvm/IR/Intrinsics.h
+++ b/llvm/include/llvm/IR/Intrinsics.h
@@ -244,6 +244,8 @@ namespace Intrinsic {
// Checks if the intrinsic name matches with its signature and if not
// returns the declaration with the same signature and remangled name.
+ // An existing GlobalValue with the wanted name but with a wrong prototype
+ // or of the wrong kind will be renamed by adding ".renamed" to the name.
llvm::Optional<Function*> remangleIntrinsicFunction(Function *F);
} // End Intrinsic namespace
diff --git a/llvm/lib/IR/Function.cpp b/llvm/lib/IR/Function.cpp
index b502e5c48dbc0..4f4a8dbbd983b 100644
--- a/llvm/lib/IR/Function.cpp
+++ b/llvm/lib/IR/Function.cpp
@@ -1676,11 +1676,26 @@ Optional<Function *> Intrinsic::remangleIntrinsicFunction(Function *F) {
Intrinsic::ID ID = F->getIntrinsicID();
StringRef Name = F->getName();
- if (Name ==
- Intrinsic::getName(ID, ArgTys, F->getParent(), F->getFunctionType()))
+ std::string WantedName =
+ Intrinsic::getName(ID, ArgTys, F->getParent(), F->getFunctionType());
+ if (Name == WantedName)
return None;
- auto NewDecl = Intrinsic::getDeclaration(F->getParent(), ID, ArgTys);
+ Function *NewDecl = [&] {
+ if (auto *ExistingGV = F->getParent()->getNamedValue(WantedName)) {
+ if (auto *ExistingF = dyn_cast<Function>(ExistingGV))
+ if (ExistingF->getFunctionType() == F->getFunctionType())
+ return ExistingF;
+
+ // The name already exists, but is not a function or has the wrong
+ // prototype. Make place for the new one by renaming the old version.
+ // Either this old version will be removed later on or the module is
+ // invalid and we'll get an error.
+ ExistingGV->setName(WantedName + ".renamed");
+ }
+ return Intrinsic::getDeclaration(F->getParent(), ID, ArgTys);
+ }();
+
NewDecl->setCallingConv(F->getCallingConv());
assert(NewDecl->getFunctionType() == F->getFunctionType() &&
"Shouldn't change the signature");
diff --git a/llvm/test/Assembler/remangle.ll b/llvm/test/Assembler/remangle.ll
new file mode 100644
index 0000000000000..80f33eed33bb5
--- /dev/null
+++ b/llvm/test/Assembler/remangle.ll
@@ -0,0 +1,60 @@
+; RUN: opt %s -S -o - | FileCheck %s
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; Note: this test mimics how the naming could have been after parsing in IR in
+; a LLVMContext where some types were already available; but before the
+; remangleIntrinsicFunctions has happened:
+; The @llvm.ssa.copy intrinsics will have to be remangled.
+; In certain cases (as shown here) the remangling can result in a name clash.
+; This is also related to the llvm/test/tools/llvm-linker/remangle.ll testcase that checks
+; a similar situation in the bitcode reader.
+
+%fum = type { %aab, i8, [7 x i8] }
+%aab = type { %aba }
+%aba = type { [8 x i8] }
+%fum.1 = type { %abb, i8, [7 x i8] }
+%abb = type { %abc }
+%abc = type { [4 x i8] }
+
+declare void @foo(%fum*)
+
+; Will be remagled to @"llvm.ssa.copy.p0p0s_fum.1s"
+declare %fum.1** @"llvm.ssa.copy.p0p0s_fums"(%fum.1**)
+
+; Will be remagled to @"llvm.ssa.copy.p0p0s_fums"
+declare %fum** @"llvm.ssa.copy.p0p0s_fum.1s"(%fum**)
+
+define void @foo1(%fum** %a, %fum.1 ** %b) {
+ %b.copy = call %fum.1** @"llvm.ssa.copy.p0p0s_fums"(%fum.1** %b)
+ %a.copy = call %fum** @"llvm.ssa.copy.p0p0s_fum.1s"(%fum** %a)
+ ret void
+}
+
+define void @foo2(%fum.1 ** %b, %fum** %a) {
+ %a.copy = call %fum** @"llvm.ssa.copy.p0p0s_fum.1s"(%fum** %a)
+ %b.copy = call %fum.1** @"llvm.ssa.copy.p0p0s_fums"(%fum.1** %b)
+ ret void
+}
+
+; CHECK-DAG: %fum = type { %aab, i8, [7 x i8] }
+; CHECK-DAG: %aab = type { %aba }
+; CHECK-DAG: %aba = type { [8 x i8] }
+; CHECK-DAG: %fum.1 = type { %abb, i8, [7 x i8] }
+; CHECK-DAG: %abb = type { %abc }
+; CHECK-DAG: %abc = type { [4 x i8] }
+
+; CHECK-LABEL: define void @foo1(%fum** %a, %fum.1** %b) {
+; CHECK-NEXT: %b.copy = call %fum.1** @llvm.ssa.copy.p0p0s_fum.1s(%fum.1** %b)
+; CHECK-NEXT: %a.copy = call %fum** @llvm.ssa.copy.p0p0s_fums(%fum** %a)
+; CHECK-NEXT: ret void
+
+; CHECK-LABEL: define void @foo2(%fum.1** %b, %fum** %a) {
+; CHECK-NEXT: %a.copy = call %fum** @llvm.ssa.copy.p0p0s_fums(%fum** %a)
+; CHECK-NEXT: %b.copy = call %fum.1** @llvm.ssa.copy.p0p0s_fum.1s(%fum.1** %b)
+; CHECK-NEXT: ret void
+
+; CHECK: declare %fum.1** @llvm.ssa.copy.p0p0s_fum.1s(%fum.1** returned)
+
+; CHECK: declare %fum** @llvm.ssa.copy.p0p0s_fums(%fum** returned)
diff --git a/llvm/test/tools/llvm-link/Inputs/remangle1.ll b/llvm/test/tools/llvm-link/Inputs/remangle1.ll
new file mode 100644
index 0000000000000..5c7f7c922aac3
--- /dev/null
+++ b/llvm/test/tools/llvm-link/Inputs/remangle1.ll
@@ -0,0 +1,10 @@
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+%aaa = type <{ %aab, i32, [4 x i8] }>
+%aab = type { i64 }
+%fum = type { %aac, i8, [7 x i8] }
+%aac = type { [8 x i8] }
+
+declare void @bar01(%aaa*)
+declare void @bar02(%fum*)
diff --git a/llvm/test/tools/llvm-link/Inputs/remangle2.ll b/llvm/test/tools/llvm-link/Inputs/remangle2.ll
new file mode 100644
index 0000000000000..b07be3c9cf03f
--- /dev/null
+++ b/llvm/test/tools/llvm-link/Inputs/remangle2.ll
@@ -0,0 +1,27 @@
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+%fum = type { %aab, i8, [7 x i8] }
+%aab = type { %aba }
+%aba = type { [8 x i8] }
+%fum.1 = type { %abb, i8, [7 x i8] }
+%abb = type { %abc }
+%abc = type { [4 x i8] }
+
+declare void @foo(%fum*)
+
+declare %fum.1** @"llvm.ssa.copy.p0p0s_fum.1s"(%fum.1**)
+
+declare %fum** @"llvm.ssa.copy.p0p0s_fums"(%fum**)
+
+define void @foo1(%fum** %a, %fum.1 ** %b) {
+ %b.copy = call %fum.1** @"llvm.ssa.copy.p0p0s_fum.1s"(%fum.1** %b)
+ %a.copy = call %fum** @"llvm.ssa.copy.p0p0s_fums"(%fum** %a)
+ ret void
+}
+
+define void @foo2(%fum.1 ** %b, %fum** %a) {
+ %a.copy = call %fum** @"llvm.ssa.copy.p0p0s_fums"(%fum** %a)
+ %b.copy = call %fum.1** @"llvm.ssa.copy.p0p0s_fum.1s"(%fum.1** %b)
+ ret void
+}
diff --git a/llvm/test/tools/llvm-link/remangle.ll b/llvm/test/tools/llvm-link/remangle.ll
new file mode 100644
index 0000000000000..f73fa117feb67
--- /dev/null
+++ b/llvm/test/tools/llvm-link/remangle.ll
@@ -0,0 +1,27 @@
+# RUN: llvm-as %S/Inputs/remangle1.ll -o %t.remangle1.bc
+# RUN: llvm-as %S/Inputs/remangle2.ll -o %t.remangle2.bc
+# RUN: llvm-link %t.remangle1.bc %t.remangle2.bc -o %t.remangle.linked.bc
+# RUN: llvm-dis %t.remangle.linked.bc -o - | FileCheck %s
+
+; CHECK-DAG: %fum.1 = type { %aab.0, i8, [7 x i8] }
+; CHECK-DAG: %aab.0 = type { %aba }
+; CHECK-DAG: %aba = type { [8 x i8] }
+; CHECK-DAG: %fum.1.2 = type { %abb, i8, [7 x i8] }
+; CHECK-DAG: %abb = type { %abc }
+; CHECK-DAG: %abc = type { [4 x i8] }
+
+; CHECK-LABEL: define void @foo1(%fum.1** %a, %fum.1.2** %b) {
+; CHECK-NEXT: %b.copy = call %fum.1.2** @llvm.ssa.copy.p0p0s_fum.1.2s(%fum.1.2** %b)
+; CHECK-NEXT: %a.copy = call %fum.1** @llvm.ssa.copy.p0p0s_fum.1s(%fum.1** %a)
+; CHECK-NEXT: ret void
+
+; CHECK: declare %fum.1.2** @llvm.ssa.copy.p0p0s_fum.1.2s(%fum.1.2** returned)
+
+; CHECK: declare %fum.1** @llvm.ssa.copy.p0p0s_fum.1s(%fum.1** returned)
+
+; CHECK-LABEL: define void @foo2(%fum.1.2** %b, %fum.1** %a) {
+; CHECK-NEXT: %a.copy = call %fum.1** @llvm.ssa.copy.p0p0s_fum.1s(%fum.1** %a)
+; CHECK-NEXT: %b.copy = call %fum.1.2** @llvm.ssa.copy.p0p0s_fum.1.2s(%fum.1.2** %b)
+; CHECK-NEXT: ret void
+
+
More information about the llvm-commits
mailing list