LowerTypeTests: Rename local functions to avoid collisions with identically named functions in ThinLTO modules.

[pcc]

Summary

Without this we can encounter link errors or incorrect behaviour
at runtime as a result of the wrong function being referenced.

Fixes https://github.com/ClangBuiltLinux/linux/issues/537

[dexonsmith]

llvm/lib/Transforms/IPO/LowerTypeTests.cpp

1890–1896 ↗

(On Diff #221452)

This looks like it could collide with an existing function. Clang handles this in a few places by looping until it finds an available name.

[pcc]

llvm/lib/Transforms/IPO/LowerTypeTests.cpp

1890–1896 ↗

(On Diff #221452)

I was under the impression that this was already handled by setName (see http://llvm-cs.pcc.me.uk/lib/IR/Value.cpp#282 -> http://llvm-cs.pcc.me.uk/lib/IR/ValueSymbolTable.cpp#112 -> http://llvm-cs.pcc.me.uk/lib/IR/ValueSymbolTable.cpp#61), although it may be worth adding a test for this case.

[dexonsmith]

llvm/lib/Transforms/IPO/LowerTypeTests.cpp

1890–1896 ↗

(On Diff #221452)

Ah, you're right. A test for that might be nice, but a comment might also be enough given that it's likely tested elsewhere.

[pcc]

• Add comment

[pcc]

llvm/lib/Transforms/IPO/LowerTypeTests.cpp

1890–1896 ↗

(On Diff #221452)

Okay, I've added a comment here.

[pcc]

eugenis: Ping.

[eugenis]

LGTM

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

if (CfiFunctionsMD) {
           P.first->second = {Linkage, FuncMD};
       }
 
       for (const auto &P : ExportedFunctions) {
         StringRef FunctionName = P.first;
         CfiFunctionLinkage Linkage = P.second.Linkage;
         MDNode *FuncMD = P.second.FuncMD;
         Function *F = M.getFunction(FunctionName);
+        if (F && F->hasLocalLinkage()) {
+          // Locally defined function that happens to have the same name as a
+          // function defined in a ThinLTO module. Rename it to move it out of
+          // the way of the external reference that we're about to create.
+          // Note that setName will find a unique name for the function, so even
+          // if there is an existing function with the suffix there won't be a
+          // name collision.
+          F->setName(F->getName() + ".1");
+          F = nullptr;
+        }
+
         if (!F)
           F = Function::Create(
               FunctionType::get(Type::getVoidTy(M.getContext()), false),
               GlobalVariable::ExternalLinkage,
               M.getDataLayout().getProgramAddressSpace(), FunctionName, &M);
 
         // If the function is available_externally, remove its definition so
         // that it is handled the same way as a declaration. Later we will try

llvm/trunk/test/Transforms/LowerTypeTests/export-rename-local.ll

+; RUN: opt -S %s -lowertypetests -lowertypetests-summary-action=export -lowertypetests-read-summary=%S/Inputs/exported-funcs.yaml | FileCheck %s
+
+; CHECK: define internal void @external_addrtaken.1()
+; CHECK: declare {{.*}} void @external_addrtaken.cfi()
+
+target triple = "x86_64-unknown-linux"
+
+define internal void @external_addrtaken() !type !1 {
+  ret void
+}
+
+!cfi.functions = !{!0}
+
+!0 = !{!"external_addrtaken", i8 0, !1}
+!1 = !{i64 0, !"typeid1"}