[LTO] Fix error reporting when a file passed to libLTO is invalid or non-existent

[petpav01]

Summary

This patch addresses bug https://llvm.org/bugs/show_bug.cgi?id=26060 where lto_module_create() could return nullptr but lto_get_error_message() returned an empty string. Unfortunately, I could not find any place where lto_module_*() functions would be tested so I am not sure where to add a test for this problem.

Note that this patch would also fix the same problem that commit r257395 fixed yesterday in llvm-lto because the diagnostic handler in llvm-lto exits the program on error.

[• rafael]

To test this you should be able to delete the call to error in

ErrorOr<std::unique_ptr<LTOModule>> ModuleOrErr =
     LTOModule::createFromFile(Context, InputFilenames[i].c_str(), Options);
 error(ModuleOrErr, "error " + CurrentActivity);

in llvm-lto.cpp since now the error will be received via the diagnostic handler.

tools/lto/lto.cpp

84 ↗

(On Diff #44611)

Is this related?

[petpav01]

Thank you for looking at this patch. I will update the patch to delete the error() call after LTOModule::createFromFile() from llvm-lto.

tools/lto/lto.cpp

84 ↗

(On Diff #44611)

It is related because before r254721 function lto_get_error_message() would return "The file was not recognized as a valid object file" after calling lto_module_create() on an empty file. If only the emitError() calls are added then the returned error is "The file was not recognized as a valid object file\n" (i.e. it includes the newline character). This is problematic if someone had printf("LTO error: %s\n", lto_get_error_message()); in their code as it will now print an empty line after the error.

[petpav01]

Updated patch deletes the error() call after LTOModule::createFromFile() from llvm-lto because any error from this function should go through the diagnostic handler in llvm-lto which will exit the program.

[• rafael]

LGTM

llvm/trunk/lib/LTO/LTOModule.cpp

std::string LTOModule::getProducerString(MemoryBuffer *Buffer) {
   return getBitcodeProducerString(*BCOrErr, Context);
 }
 
 ErrorOr<std::unique_ptr<LTOModule>>
 LTOModule::createFromFile(LLVMContext &Context, const char *path,
                           TargetOptions options) {
   ErrorOr<std::unique_ptr<MemoryBuffer>> BufferOrErr =
       MemoryBuffer::getFile(path);
-  if (std::error_code EC = BufferOrErr.getError())
+  if (std::error_code EC = BufferOrErr.getError()) {
+    Context.emitError(EC.message());
     return EC;
+  }
   std::unique_ptr<MemoryBuffer> Buffer = std::move(BufferOrErr.get());
   return makeLTOModule(Buffer->getMemBufferRef(), options, &Context);
 }
 
 ErrorOr<std::unique_ptr<LTOModule>>
 LTOModule::createFromOpenFile(LLVMContext &Context, int fd, const char *path,
                               size_t size, TargetOptions options) {
   return createFromOpenFileSlice(Context, fd, path, size, 0, options);
 }
 
 ErrorOr<std::unique_ptr<LTOModule>>
 LTOModule::createFromOpenFileSlice(LLVMContext &Context, int fd,
                                    const char *path, size_t map_size,
                                    off_t offset, TargetOptions options) {
   ErrorOr<std::unique_ptr<MemoryBuffer>> BufferOrErr =
       MemoryBuffer::getOpenFileSlice(fd, path, map_size, offset);
-  if (std::error_code EC = BufferOrErr.getError())
+  if (std::error_code EC = BufferOrErr.getError()) {
+    Context.emitError(EC.message());
     return EC;
+  }
   std::unique_ptr<MemoryBuffer> Buffer = std::move(BufferOrErr.get());
   return makeLTOModule(Buffer->getMemBufferRef(), options, &Context);
 }
 
 ErrorOr<std::unique_ptr<LTOModule>>
 LTOModule::createFromBuffer(LLVMContext &Context, const void *mem,
                             size_t length, TargetOptions options,
                             StringRef path) {

 
 static ErrorOr<std::unique_ptr<Module>>
 parseBitcodeFileImpl(MemoryBufferRef Buffer, LLVMContext &Context,
                      bool ShouldBeLazy) {
 
   // Find the buffer.
   ErrorOr<MemoryBufferRef> MBOrErr =
       IRObjectFile::findBitcodeInMemBuffer(Buffer);
-  if (std::error_code EC = MBOrErr.getError())
+  if (std::error_code EC = MBOrErr.getError()) {
+    Context.emitError(EC.message());
     return EC;
+  }
 
   if (!ShouldBeLazy) {
     // Parse the full file.
     ErrorOr<std::unique_ptr<Module>> M = parseBitcodeFile(*MBOrErr, Context);
     if (std::error_code EC = M.getError())
       return EC;
     return std::move(*M);
   }

llvm/trunk/tools/llvm-lto/llvm-lto.cpp

for (unsigned i = 0; i < DSOSymbols.size(); ++i)
     DSOSymbolsSet.insert(DSOSymbols[i]);
 
   std::vector<std::string> KeptDSOSyms;
 
   for (unsigned i = BaseArg; i < InputFilenames.size(); ++i) {
     CurrentActivity = "loading file '" + InputFilenames[i] + "'";
     ErrorOr<std::unique_ptr<LTOModule>> ModuleOrErr =
         LTOModule::createFromFile(Context, InputFilenames[i].c_str(), Options);
-    error(ModuleOrErr, "error " + CurrentActivity);
     std::unique_ptr<LTOModule> &Module = *ModuleOrErr;
     CurrentActivity = "";
 
     unsigned NumSyms = Module->getSymbolCount();
     for (unsigned I = 0; I < NumSyms; ++I) {
       StringRef Name = Module->getSymbolName(I);
       if (!DSOSymbolsSet.count(Name))
         continue;

llvm/trunk/tools/lto/lto.cpp

if (DI.getSeverity() != DS_Error) {
     return;
   }
   sLastErrorString = "";
   {
     raw_string_ostream Stream(sLastErrorString);
     DiagnosticPrinterRawOStream DP(Stream);
     DI.print(DP);
   }
-  sLastErrorString += '\n';
 }
 
 // Initialize the configured targets if they have not been initialized.
 static void lto_initialize() {
   if (!initialized) {
 #ifdef LLVM_ON_WIN32
     // Dialog box on crash disabling doesn't work across DLL boundaries, so do
     // it here.
#endif
   }
 }
 
 namespace {
 
 static void handleLibLTODiagnostic(lto_codegen_diagnostic_severity_t Severity,
                                    const char *Msg, void *) {
   sLastErrorString = Msg;
-  sLastErrorString += "\n";
 }
 
 // This derived class owns the native object file. This helps implement the
 // libLTO API semantics, which require that the code generator owns the object
 // file.
 struct LibLTOCodeGenerator : LTOCodeGenerator {
   LibLTOCodeGenerator() : LTOCodeGenerator(*LTOContext) {
     setDiagnosticHandler(handleLibLTODiagnostic, nullptr); }