[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

Author: dobbelaj-snps

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

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");

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)

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*)

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
+}

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
+
+