From 8c98210bcfcc4851624bccd9d9c7a64b55899e07 Mon Sep 17 00:00:00 2001 From: Teresa Johnson Date: Thu, 31 Jan 2019 17:18:11 +0000 Subject: [PATCH] Recommit "[ThinLTO] Rename COMDATs for COFF when promoting/renaming COMDAT leader" Recommit of r352763 with fix for use after free. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@352770 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../Transforms/Utils/FunctionImportUtils.h | 5 +++ lib/Transforms/Utils/FunctionImportUtils.cpp | 18 +++++++++++ .../FunctionImport/Inputs/comdat.ll | 10 ++++++ test/Transforms/FunctionImport/comdat.ll | 32 +++++++++++++++++++ 4 files changed, 65 insertions(+) create mode 100644 test/Transforms/FunctionImport/Inputs/comdat.ll create mode 100644 test/Transforms/FunctionImport/comdat.ll diff --git a/include/llvm/Transforms/Utils/FunctionImportUtils.h b/include/llvm/Transforms/Utils/FunctionImportUtils.h index ac69908d962..9c2a9ea531e 100644 --- a/include/llvm/Transforms/Utils/FunctionImportUtils.h +++ b/include/llvm/Transforms/Utils/FunctionImportUtils.h @@ -43,6 +43,11 @@ class FunctionImportGlobalProcessing { /// to promote any non-renamable values. SmallPtrSet Used; + /// Keep track of any COMDATs that require renaming (because COMDAT + /// leader was promoted and renamed). Maps from original COMDAT to one + /// with new name. + DenseMap RenamedComdats; + /// Check if we should promote the given local value to global scope. bool shouldPromoteLocalToGlobal(const GlobalValue *SGV); diff --git a/lib/Transforms/Utils/FunctionImportUtils.cpp b/lib/Transforms/Utils/FunctionImportUtils.cpp index a122b28e52e..e48fcbdeff4 100644 --- a/lib/Transforms/Utils/FunctionImportUtils.cpp +++ b/lib/Transforms/Utils/FunctionImportUtils.cpp @@ -248,6 +248,8 @@ void FunctionImportGlobalProcessing::processGlobalForThinLTO(GlobalValue &GV) { bool DoPromote = false; if (GV.hasLocalLinkage() && ((DoPromote = shouldPromoteLocalToGlobal(&GV)) || isPerformingImport())) { + // Save the original name string before we rename GV below. + auto Name = GV.getName().str(); // Once we change the name or linkage it is difficult to determine // again whether we should promote since shouldPromoteLocalToGlobal needs // to locate the summary (based on GUID from name and linkage). Therefore, @@ -256,6 +258,12 @@ void FunctionImportGlobalProcessing::processGlobalForThinLTO(GlobalValue &GV) { GV.setLinkage(getLinkage(&GV, DoPromote)); if (!GV.hasLocalLinkage()) GV.setVisibility(GlobalValue::HiddenVisibility); + + // If we are renaming a COMDAT leader, ensure that we record the COMDAT + // for later renaming as well. This is required for COFF. + if (const auto *C = GV.getComdat()) + if (C->getName() == Name) + RenamedComdats.try_emplace(C, M.getOrInsertComdat(GV.getName())); } else GV.setLinkage(getLinkage(&GV, /* DoPromote */ false)); @@ -280,6 +288,16 @@ void FunctionImportGlobalProcessing::processGlobalsForThinLTO() { processGlobalForThinLTO(SF); for (GlobalAlias &GA : M.aliases()) processGlobalForThinLTO(GA); + + // Replace any COMDATS that required renaming (because the COMDAT leader was + // promoted and renamed). + if (!RenamedComdats.empty()) + for (auto &GO : M.global_objects()) + if (auto *C = GO.getComdat()) { + auto Replacement = RenamedComdats.find(C); + if (Replacement != RenamedComdats.end()) + GO.setComdat(Replacement->second); + } } bool FunctionImportGlobalProcessing::run() { diff --git a/test/Transforms/FunctionImport/Inputs/comdat.ll b/test/Transforms/FunctionImport/Inputs/comdat.ll new file mode 100644 index 00000000000..1df6f25351e --- /dev/null +++ b/test/Transforms/FunctionImport/Inputs/comdat.ll @@ -0,0 +1,10 @@ +target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128" +target triple = "x86_64-pc-windows-msvc19.0.24215" + +define void @main() { +entry: + call i8* @lwt_fun() + ret void +} + +declare i8* @lwt_fun() diff --git a/test/Transforms/FunctionImport/comdat.ll b/test/Transforms/FunctionImport/comdat.ll new file mode 100644 index 00000000000..29e8cb538ab --- /dev/null +++ b/test/Transforms/FunctionImport/comdat.ll @@ -0,0 +1,32 @@ +; Test to ensure that comdat is renamed consistently when comdat leader is +; promoted and renamed due to an import. Required by COFF. + +; REQUIRES: x86-registered-target + +; RUN: opt -thinlto-bc -o %t1.bc %s +; RUN: opt -thinlto-bc -o %t2.bc %S/Inputs/comdat.ll +; RUN: llvm-lto2 run -save-temps -o %t3 %t1.bc %t2.bc \ +; RUN: -r %t1.bc,lwt_fun,plx \ +; RUN: -r %t2.bc,main,plx \ +; RUN: -r %t2.bc,lwt_fun, +; RUN: llvm-dis -o - %t3.1.3.import.bc | FileCheck %s + +target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128" +target triple = "x86_64-pc-windows-msvc19.0.24215" + +; CHECK: $lwt.llvm.[[HASH:[0-9]+]] = comdat any +$lwt = comdat any + +; CHECK: @lwt_aliasee = private unnamed_addr global {{.*}}, comdat($lwt.llvm.[[HASH]]) +@lwt_aliasee = private unnamed_addr global [1 x i8*] [i8* null], comdat($lwt) + +; CHECK: @lwt.llvm.[[HASH]] = hidden unnamed_addr alias +@lwt = internal unnamed_addr alias [1 x i8*], [1 x i8*]* @lwt_aliasee + +; Below function should get imported into other module, resulting in @lwt being +; promoted and renamed. +define i8* @lwt_fun() { + %1 = getelementptr inbounds [1 x i8*], [1 x i8*]* @lwt, i32 0, i32 0 + %2 = load i8*, i8** %1 + ret i8* %2 +} -- 2.50.1