jhuber6 created this revision. jhuber6 added reviewers: jdoerfert, ye-luo. Herald added a project: All. jhuber6 requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits.
This patch removes some uses of string savers that are no-longer needed. We also create a new string saver when linking bitcode files. It seems that occasionally the symbol string references can go out of scope when they are added to the LTO input so we need to save these names that are used for symbol resolution. Additionally, a previous patch added new logic for handling bitcode libraries, but failed to actually add them to the input. This bug has been fixed. Fixes #56445 Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D129383 Files: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp Index: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp =================================================================== --- clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp +++ clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp @@ -525,17 +525,15 @@ if (!TempFileOrErr) return TempFileOrErr.takeError(); - BumpPtrAllocator Alloc; - StringSaver Saver(Alloc); - SmallVector<StringRef, 16> CmdArgs; CmdArgs.push_back(*FatBinaryPath); CmdArgs.push_back(Triple.isArch64Bit() ? "-64" : "-32"); CmdArgs.push_back("--create"); CmdArgs.push_back(*TempFileOrErr); for (const auto &FileAndArch : InputFiles) - CmdArgs.push_back(Saver.save("--image=profile=" + std::get<1>(FileAndArch) + - ",file=" + std::get<0>(FileAndArch))); + CmdArgs.push_back( + Args.MakeArgString("--image=profile=" + std::get<1>(FileAndArch) + + ",file=" + std::get<0>(FileAndArch))); if (Error Err = executeCommands(*FatBinaryPath, CmdArgs)) return std::move(Err); @@ -808,6 +806,8 @@ SmallVector<OffloadFile, 4> BitcodeInputFiles; DenseSet<StringRef> UsedInRegularObj; DenseSet<StringRef> UsedInSharedLib; + BumpPtrAllocator Alloc; + StringSaver Saver(Alloc); // Search for bitcode files in the input and create an LTO input file. If it // is not a bitcode file, scan its symbol table for symbols we need to save. @@ -844,9 +844,9 @@ // Record if we've seen these symbols in any object or shared libraries. if ((*ObjFile)->isRelocatableObject()) - UsedInRegularObj.insert(*Name); + UsedInRegularObj.insert(Saver.save(*Name)); else - UsedInSharedLib.insert(*Name); + UsedInSharedLib.insert(Saver.save(*Name)); } continue; } @@ -908,7 +908,8 @@ // We will use this as the prevailing symbol definition in LTO unless // it is undefined or another definition has already been used. Res.Prevailing = - !Sym.isUndefined() && PrevailingSymbols.insert(Sym.getName()).second; + !Sym.isUndefined() && + PrevailingSymbols.insert(Saver.save(Sym.getName())).second; // We need LTO to preseve the following global symbols: // 1) Symbols used in regular objects. @@ -1193,8 +1194,6 @@ InputsForTarget[File].emplace_back(std::move(File)); LinkerInputFiles.clear(); - BumpPtrAllocator Alloc; - UniqueStringSaver Saver(Alloc); DenseMap<OffloadKind, SmallVector<OffloadingImage, 2>> Images; for (auto &InputForTarget : InputsForTarget) { SmallVector<OffloadFile, 4> &Input = InputForTarget.getSecond(); @@ -1395,6 +1394,7 @@ auto FileOrErr = getInputBitcodeLibrary(Library); if (!FileOrErr) reportError(FileOrErr.takeError()); + InputFiles.push_back(std::move(*FileOrErr)); } DenseSet<OffloadFile::TargetID> IsTargetUsed;
Index: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp =================================================================== --- clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp +++ clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp @@ -525,17 +525,15 @@ if (!TempFileOrErr) return TempFileOrErr.takeError(); - BumpPtrAllocator Alloc; - StringSaver Saver(Alloc); - SmallVector<StringRef, 16> CmdArgs; CmdArgs.push_back(*FatBinaryPath); CmdArgs.push_back(Triple.isArch64Bit() ? "-64" : "-32"); CmdArgs.push_back("--create"); CmdArgs.push_back(*TempFileOrErr); for (const auto &FileAndArch : InputFiles) - CmdArgs.push_back(Saver.save("--image=profile=" + std::get<1>(FileAndArch) + - ",file=" + std::get<0>(FileAndArch))); + CmdArgs.push_back( + Args.MakeArgString("--image=profile=" + std::get<1>(FileAndArch) + + ",file=" + std::get<0>(FileAndArch))); if (Error Err = executeCommands(*FatBinaryPath, CmdArgs)) return std::move(Err); @@ -808,6 +806,8 @@ SmallVector<OffloadFile, 4> BitcodeInputFiles; DenseSet<StringRef> UsedInRegularObj; DenseSet<StringRef> UsedInSharedLib; + BumpPtrAllocator Alloc; + StringSaver Saver(Alloc); // Search for bitcode files in the input and create an LTO input file. If it // is not a bitcode file, scan its symbol table for symbols we need to save. @@ -844,9 +844,9 @@ // Record if we've seen these symbols in any object or shared libraries. if ((*ObjFile)->isRelocatableObject()) - UsedInRegularObj.insert(*Name); + UsedInRegularObj.insert(Saver.save(*Name)); else - UsedInSharedLib.insert(*Name); + UsedInSharedLib.insert(Saver.save(*Name)); } continue; } @@ -908,7 +908,8 @@ // We will use this as the prevailing symbol definition in LTO unless // it is undefined or another definition has already been used. Res.Prevailing = - !Sym.isUndefined() && PrevailingSymbols.insert(Sym.getName()).second; + !Sym.isUndefined() && + PrevailingSymbols.insert(Saver.save(Sym.getName())).second; // We need LTO to preseve the following global symbols: // 1) Symbols used in regular objects. @@ -1193,8 +1194,6 @@ InputsForTarget[File].emplace_back(std::move(File)); LinkerInputFiles.clear(); - BumpPtrAllocator Alloc; - UniqueStringSaver Saver(Alloc); DenseMap<OffloadKind, SmallVector<OffloadingImage, 2>> Images; for (auto &InputForTarget : InputsForTarget) { SmallVector<OffloadFile, 4> &Input = InputForTarget.getSecond(); @@ -1395,6 +1394,7 @@ auto FileOrErr = getInputBitcodeLibrary(Library); if (!FileOrErr) reportError(FileOrErr.takeError()); + InputFiles.push_back(std::move(*FileOrErr)); } DenseSet<OffloadFile::TargetID> IsTargetUsed;
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits