From f1f60632b06de61a15f7362002ef0fe3399a2d54 Mon Sep 17 00:00:00 2001 From: Teresa Johnson Date: Sat, 21 Nov 2015 21:55:48 +0000 Subject: [PATCH] [ThinLTO] Handle bitcode without function summary sections gracefully Summary: Several fixes to the handling of bitcode files without function summary sections so that they are skipped during ThinLTO processing in llvm-lto and the gold plugin when appropriate instead of aborting. 1 Don't assert when trying to add a FunctionInfo that doesn't have a summary attached. 2 Skip FunctionInfo structures that don't have attached function summary sections when trying to create the combined function summary. 3 In both llvm-lto and gold-plugin, check whether a bitcode file has a function summary section before trying to parse the index, and skip the bitcode file if it does not. 4 Fix hasFunctionSummaryInMemBuffer in BitcodeReader, which had a bug where we returned to early while looking for the summary section. Also added llvm-lto and gold-plugin based tests for cases where we don't have function summaries in the bitcode file. I verified that either the first couple fixes described above are enough to avoid the crashes, or fixes 1,3,4. But have combined them all here for added robustness. Reviewers: joker.eph Subscribers: llvm-commits, joker.eph Differential Revision: http://reviews.llvm.org/D14903 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@253796 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/IR/FunctionInfo.h | 6 +++-- lib/Bitcode/Reader/BitcodeReader.cpp | 8 ++++--- lib/IR/FunctionInfo.cpp | 4 ++++ test/Linker/funcimport.ll | 7 ++++++ test/tools/gold/X86/thinlto.ll | 8 +++++++ tools/gold/gold-plugin.cpp | 12 ++++++++++ tools/llvm-lto/llvm-lto.cpp | 35 +++++++++++++++++----------- 7 files changed, 61 insertions(+), 19 deletions(-) diff --git a/include/llvm/IR/FunctionInfo.h b/include/llvm/IR/FunctionInfo.h index e265ad37096..55f9ed88f1c 100644 --- a/include/llvm/IR/FunctionInfo.h +++ b/include/llvm/IR/FunctionInfo.h @@ -196,8 +196,10 @@ public: /// Add a function info for a function of the given name. void addFunctionInfo(StringRef FuncName, std::unique_ptr Info) { - if (ExportingModule) { - assert(Info->functionSummary()); + // Update the HasExportedFunctions flag, but only if we had a function + // summary (i.e. we aren't parsing them lazily or have a bitcode file + // without a function summary section). + if (ExportingModule && Info->functionSummary()) { if (ExportingModule->getModuleIdentifier() == Info->functionSummary()->modulePath()) HasExportedFunctions = true; diff --git a/lib/Bitcode/Reader/BitcodeReader.cpp b/lib/Bitcode/Reader/BitcodeReader.cpp index 9d907773cb3..11c9b131da7 100644 --- a/lib/Bitcode/Reader/BitcodeReader.cpp +++ b/lib/Bitcode/Reader/BitcodeReader.cpp @@ -5566,12 +5566,14 @@ std::error_code FunctionIndexBitcodeReader::parseModule() { case BitstreamEntry::SubBlock: if (CheckFuncSummaryPresenceOnly) { - if (Entry.ID == bitc::FUNCTION_SUMMARY_BLOCK_ID) + if (Entry.ID == bitc::FUNCTION_SUMMARY_BLOCK_ID) { SeenFuncSummary = true; + // No need to parse the rest since we found the summary. + return std::error_code(); + } if (Stream.SkipBlock()) return error("Invalid record"); - // No need to parse the rest since we found the summary. - return std::error_code(); + continue; } switch (Entry.ID) { default: // Skip unknown content. diff --git a/lib/IR/FunctionInfo.cpp b/lib/IR/FunctionInfo.cpp index d26c7a4e9a0..17a67bcf047 100644 --- a/lib/IR/FunctionInfo.cpp +++ b/lib/IR/FunctionInfo.cpp @@ -31,6 +31,10 @@ void FunctionInfoIndex::mergeFrom(std::unique_ptr Other, assert(List.size() == 1); std::unique_ptr Info = std::move(List.front()); + // Skip if there was no function summary section. + if (!Info->functionSummary()) + continue; + // Add the module path string ref for this module if we haven't already // saved a reference to it. if (ModPath.empty()) diff --git a/test/Linker/funcimport.ll b/test/Linker/funcimport.ll index dad9b95bb9f..d6aa0502f26 100644 --- a/test/Linker/funcimport.ll +++ b/test/Linker/funcimport.ll @@ -1,3 +1,10 @@ +; First ensure that the ThinLTO handling in llvm-link and llvm-lto handles +; bitcode without function summary sections gracefully. +; RUN: llvm-as %s -o %t.bc +; RUN: llvm-as %p/Inputs/funcimport.ll -o %t2.bc +; RUN: llvm-link %t.bc -functionindex=%t.bc -S +; RUN: llvm-lto -thinlto -o %t3 %t.bc %t2.bc + ; Do setup work for all below tests: generate bitcode and combined index ; RUN: llvm-as -function-summary %s -o %t.bc ; RUN: llvm-as -function-summary %p/Inputs/funcimport.ll -o %t2.bc diff --git a/test/tools/gold/X86/thinlto.ll b/test/tools/gold/X86/thinlto.ll index b24e4af80bf..97def3d7a14 100644 --- a/test/tools/gold/X86/thinlto.ll +++ b/test/tools/gold/X86/thinlto.ll @@ -1,3 +1,11 @@ +; First ensure that the ThinLTO handling in the gold plugin handles +; bitcode without function summary sections gracefully. +; RUN: llvm-as %s -o %t.o +; RUN: llvm-as %p/Inputs/thinlto.ll -o %t2.o +; RUN: %gold -plugin %llvmshlibdir/LLVMgold.so \ +; RUN: --plugin-opt=thinlto \ +; RUN: -shared %t.o %t2.o -o %t3 + ; RUN: llvm-as -function-summary %s -o %t.o ; RUN: llvm-as -function-summary %p/Inputs/thinlto.ll -o %t2.o diff --git a/tools/gold/gold-plugin.cpp b/tools/gold/gold-plugin.cpp index 77e4b83223c..672c4e3580d 100644 --- a/tools/gold/gold-plugin.cpp +++ b/tools/gold/gold-plugin.cpp @@ -620,6 +620,13 @@ getFunctionIndexForFile(claimed_file &F, ld_plugin_input_file &Info) { MemoryBufferRef BufferRef(StringRef((const char *)View, Info.filesize), Info.name); + + // Don't bother trying to build an index if there is no summary information + // in this bitcode file. + if (!object::FunctionIndexObjectFile::hasFunctionSummaryInMemBuffer( + BufferRef, diagnosticHandler)) + return std::unique_ptr(nullptr); + ErrorOr> ObjOrErr = object::FunctionIndexObjectFile::create(BufferRef, diagnosticHandler); @@ -911,6 +918,11 @@ static ld_plugin_status allSymbolsReadHook(raw_fd_ostream *ApiFile) { std::unique_ptr Index = getFunctionIndexForFile(F, File); + + // Skip files without a function summary. + if (!Index) + continue; + CombinedIndex.mergeFrom(std::move(Index), ++NextModuleId); } diff --git a/tools/llvm-lto/llvm-lto.cpp b/tools/llvm-lto/llvm-lto.cpp index aac82d31a36..e580a5df578 100644 --- a/tools/llvm-lto/llvm-lto.cpp +++ b/tools/llvm-lto/llvm-lto.cpp @@ -192,24 +192,27 @@ static int listSymbols(StringRef Command, const TargetOptions &Options) { /// Parse the function index out of an IR file and return the function /// index object if found, or nullptr if not. -static std::unique_ptr -getFunctionIndexForFile(StringRef Path, std::string &Error, +static ErrorOr> +getFunctionIndexForFile(StringRef Path, DiagnosticHandlerFunction DiagnosticHandler) { std::unique_ptr Buffer; ErrorOr> BufferOrErr = MemoryBuffer::getFile(Path); - if (std::error_code EC = BufferOrErr.getError()) { - Error = EC.message(); - return nullptr; - } + if (std::error_code EC = BufferOrErr.getError()) + return EC; Buffer = std::move(BufferOrErr.get()); + + // Don't bother trying to build an index if there is no summary information + // in this bitcode file. + if (!object::FunctionIndexObjectFile::hasFunctionSummaryInMemBuffer( + Buffer->getMemBufferRef(), DiagnosticHandler)) + return std::unique_ptr(nullptr); + ErrorOr> ObjOrErr = object::FunctionIndexObjectFile::create(Buffer->getMemBufferRef(), DiagnosticHandler); - if (std::error_code EC = ObjOrErr.getError()) { - Error = EC.message(); - return nullptr; - } + if (std::error_code EC = ObjOrErr.getError()) + return EC; return (*ObjOrErr)->takeIndex(); } @@ -221,14 +224,18 @@ static int createCombinedFunctionIndex(StringRef Command) { FunctionInfoIndex CombinedIndex; uint64_t NextModuleId = 0; for (auto &Filename : InputFilenames) { - std::string Error; - std::unique_ptr Index = - getFunctionIndexForFile(Filename, Error, diagnosticHandler); - if (!Index) { + ErrorOr> IndexOrErr = + getFunctionIndexForFile(Filename, diagnosticHandler); + if (std::error_code EC = IndexOrErr.getError()) { + std::string Error = EC.message(); errs() << Command << ": error loading file '" << Filename << "': " << Error << "\n"; return 1; } + std::unique_ptr Index = std::move(IndexOrErr.get()); + // Skip files without a function summary. + if (!Index) + continue; CombinedIndex.mergeFrom(std::move(Index), ++NextModuleId); } std::error_code EC; -- 2.34.1