Use the DiagnosticHandler to print diagnostics when reading bitcode.

The bitcode reading interface used std::error_code to report an error to the
callers and it is the callers job to print diagnostics.

This is not ideal for error handling or diagnostic reporting:

* For error handling, all that the callers care about is 3 possibilities:
  * It worked
  * The bitcode file is corrupted/invalid.
  * The file is not bitcode at all.

* For diagnostic, it is user friendly to include far more information
  about the invalid case so the user can find out what is wrong with the
  bitcode file. This comes up, for example, when a developer introduces a
  bug while extending the format.

The compromise we had was to have a lot of error codes.

With this patch we use the DiagnosticHandler to communicate with the
human and std::error_code to communicate with the caller.

This allows us to have far fewer error codes and adds the infrastructure to
print better diagnostics. This is so because the diagnostics are printed when
he issue is found. The code that detected the problem in alive in the stack and
can pass down as much context as needed. As an example the patch updates
test/Bitcode/invalid.ll.

Using a DiagnosticHandler also moves the fatal/non-fatal error decision to the
caller. A simple one like llvm-dis can just use fatal errors. The gold plugin
needs a bit more complex treatment because of being passed non-bitcode files. An
hypothetical interactive tool would make all bitcode errors non-fatal.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@225562 91177308-0d34-0410-b5e6-96231b3b80d8
This commit is contained in:
Rafael Espindola 2015-01-10 00:07:30 +00:00
parent f79e2d40b2
commit 68016e0a6e
11 changed files with 427 additions and 367 deletions

View File

@ -14,6 +14,7 @@
#ifndef LLVM_BITCODE_READERWRITER_H #ifndef LLVM_BITCODE_READERWRITER_H
#define LLVM_BITCODE_READERWRITER_H #define LLVM_BITCODE_READERWRITER_H
#include "llvm/IR/DiagnosticInfo.h"
#include "llvm/Support/ErrorOr.h" #include "llvm/Support/ErrorOr.h"
#include "llvm/Support/MemoryBuffer.h" #include "llvm/Support/MemoryBuffer.h"
#include <memory> #include <memory>
@ -30,24 +31,28 @@ namespace llvm {
/// Read the header of the specified bitcode buffer and prepare for lazy /// Read the header of the specified bitcode buffer and prepare for lazy
/// deserialization of function bodies. If successful, this moves Buffer. On /// deserialization of function bodies. If successful, this moves Buffer. On
/// error, this *does not* move Buffer. /// error, this *does not* move Buffer.
ErrorOr<Module *> getLazyBitcodeModule(std::unique_ptr<MemoryBuffer> &&Buffer, ErrorOr<Module *>
LLVMContext &Context); getLazyBitcodeModule(std::unique_ptr<MemoryBuffer> &&Buffer,
LLVMContext &Context,
DiagnosticHandlerFunction DiagnosticHandler = nullptr);
/// Read the header of the specified stream and prepare for lazy /// Read the header of the specified stream and prepare for lazy
/// deserialization and streaming of function bodies. /// deserialization and streaming of function bodies.
ErrorOr<std::unique_ptr<Module>> ErrorOr<std::unique_ptr<Module>> getStreamedBitcodeModule(
getStreamedBitcodeModule(StringRef Name, DataStreamer *Streamer, StringRef Name, DataStreamer *Streamer, LLVMContext &Context,
LLVMContext &Context); DiagnosticHandlerFunction DiagnosticHandler = nullptr);
/// Read the header of the specified bitcode buffer and extract just the /// Read the header of the specified bitcode buffer and extract just the
/// triple information. If successful, this returns a string. On error, this /// triple information. If successful, this returns a string. On error, this
/// returns "". /// returns "".
std::string getBitcodeTargetTriple(MemoryBufferRef Buffer, std::string
LLVMContext &Context); getBitcodeTargetTriple(MemoryBufferRef Buffer, LLVMContext &Context,
DiagnosticHandlerFunction DiagnosticHandler = nullptr);
/// Read the specified bitcode file, returning the module. /// Read the specified bitcode file, returning the module.
ErrorOr<Module *> parseBitcodeFile(MemoryBufferRef Buffer, ErrorOr<Module *>
LLVMContext &Context); parseBitcodeFile(MemoryBufferRef Buffer, LLVMContext &Context,
DiagnosticHandlerFunction DiagnosticHandler = nullptr);
/// WriteBitcodeToFile - Write the specified module to the specified /// WriteBitcodeToFile - Write the specified module to the specified
/// raw output stream. For streams where it matters, the given stream /// raw output stream. For streams where it matters, the given stream
@ -138,32 +143,26 @@ namespace llvm {
} }
const std::error_category &BitcodeErrorCategory(); const std::error_category &BitcodeErrorCategory();
enum class BitcodeError { enum class BitcodeError { InvalidBitcodeSignature, CorruptedBitcode };
ConflictingMETADATA_KINDRecords,
CouldNotFindFunctionInStream,
ExpectedConstant,
InsufficientFunctionProtos,
InvalidBitcodeSignature,
InvalidBitcodeWrapperHeader,
InvalidConstantReference,
InvalidID, // A read identifier is not found in the table it should be in.
InvalidInstructionWithNoBB,
InvalidRecord, // A read record doesn't have the expected size or structure
InvalidTypeForValue, // Type read OK, but is invalid for its use
InvalidTYPETable,
InvalidType, // We were unable to read a type
MalformedBlock, // We are unable to advance in the stream.
MalformedGlobalInitializerSet,
InvalidMultipleBlocks, // We found multiple blocks of a kind that should
// have only one
NeverResolvedValueFoundInFunction,
NeverResolvedFunctionFromBlockAddress,
InvalidValue // Invalid version, inst number, attr number, etc
};
inline std::error_code make_error_code(BitcodeError E) { inline std::error_code make_error_code(BitcodeError E) {
return std::error_code(static_cast<int>(E), BitcodeErrorCategory()); return std::error_code(static_cast<int>(E), BitcodeErrorCategory());
} }
class BitcodeDiagnosticInfo : public DiagnosticInfo {
const Twine &Msg;
std::error_code EC;
public:
BitcodeDiagnosticInfo(std::error_code EC, DiagnosticSeverity Severity,
const Twine &Msg);
void print(DiagnosticPrinter &DP) const override;
std::error_code getError() const { return EC; };
static bool classof(const DiagnosticInfo *DI) {
return DI->getKind() == DK_Bitcode;
}
};
} // End llvm namespace } // End llvm namespace
namespace std { namespace std {

View File

@ -45,6 +45,7 @@ enum DiagnosticSeverity {
/// \brief Defines the different supported kind of a diagnostic. /// \brief Defines the different supported kind of a diagnostic.
/// This enum should be extended with a new ID for each added concrete subclass. /// This enum should be extended with a new ID for each added concrete subclass.
enum DiagnosticKind { enum DiagnosticKind {
DK_Bitcode,
DK_InlineAsm, DK_InlineAsm,
DK_StackSize, DK_StackSize,
DK_Linker, DK_Linker,
@ -97,6 +98,8 @@ public:
virtual void print(DiagnosticPrinter &DP) const = 0; virtual void print(DiagnosticPrinter &DP) const = 0;
}; };
typedef std::function<void(const DiagnosticInfo &)> DiagnosticHandlerFunction;
/// Diagnostic information for inline asm reporting. /// Diagnostic information for inline asm reporting.
/// This is basically a message and an optional location. /// This is basically a message and an optional location.
class DiagnosticInfoInlineAsm : public DiagnosticInfo { class DiagnosticInfoInlineAsm : public DiagnosticInfo {

View File

@ -13,11 +13,9 @@
#include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/DenseMap.h" #include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/DenseSet.h" #include "llvm/ADT/DenseSet.h"
#include "llvm/IR/DiagnosticInfo.h"
#include <functional>
namespace llvm { namespace llvm {
class DiagnosticInfo;
class Module; class Module;
class StructType; class StructType;
class Type; class Type;
@ -28,8 +26,6 @@ class Type;
/// something with it after the linking. /// something with it after the linking.
class Linker { class Linker {
public: public:
typedef std::function<void(const DiagnosticInfo &)> DiagnosticHandlerFunction;
struct StructTypeKeyInfo { struct StructTypeKeyInfo {
struct KeyTy { struct KeyTy {
ArrayRef<Type *> ETypes; ArrayRef<Type *> ETypes;

File diff suppressed because it is too large Load Diff

View File

@ -132,6 +132,7 @@ public:
class BitcodeReader : public GVMaterializer { class BitcodeReader : public GVMaterializer {
LLVMContext &Context; LLVMContext &Context;
DiagnosticHandlerFunction DiagnosticHandler;
Module *TheModule; Module *TheModule;
std::unique_ptr<MemoryBuffer> Buffer; std::unique_ptr<MemoryBuffer> Buffer;
std::unique_ptr<BitstreamReader> StreamFile; std::unique_ptr<BitstreamReader> StreamFile;
@ -210,18 +211,14 @@ class BitcodeReader : public GVMaterializer {
SmallPtrSet<const Function *, 4> BlockAddressesTaken; SmallPtrSet<const Function *, 4> BlockAddressesTaken;
public: public:
std::error_code Error(BitcodeError E) { return make_error_code(E); } std::error_code Error(BitcodeError E, const Twine &Message);
std::error_code Error(BitcodeError E);
std::error_code Error(const Twine &Message);
explicit BitcodeReader(MemoryBuffer *buffer, LLVMContext &C) explicit BitcodeReader(MemoryBuffer *buffer, LLVMContext &C,
: Context(C), TheModule(nullptr), Buffer(buffer), LazyStreamer(nullptr), DiagnosticHandlerFunction DiagnosticHandler);
NextUnreadBit(0), SeenValueSymbolTable(false), ValueList(C), explicit BitcodeReader(DataStreamer *streamer, LLVMContext &C,
MDValueList(C), SeenFirstFunctionBody(false), UseRelativeIDs(false), DiagnosticHandlerFunction DiagnosticHandler);
WillMaterializeAllForwardRefs(false) {}
explicit BitcodeReader(DataStreamer *streamer, LLVMContext &C)
: Context(C), TheModule(nullptr), Buffer(nullptr), LazyStreamer(streamer),
NextUnreadBit(0), SeenValueSymbolTable(false), ValueList(C),
MDValueList(C), SeenFirstFunctionBody(false), UseRelativeIDs(false),
WillMaterializeAllForwardRefs(false) {}
~BitcodeReader() { FreeState(); } ~BitcodeReader() { FreeState(); }
std::error_code materializeForwardReferencedFunctions(); std::error_code materializeForwardReferencedFunctions();

View File

@ -17,6 +17,7 @@
#include "llvm/Bitcode/ReaderWriter.h" #include "llvm/Bitcode/ReaderWriter.h"
#include "llvm/CodeGen/Analysis.h" #include "llvm/CodeGen/Analysis.h"
#include "llvm/IR/Constants.h" #include "llvm/IR/Constants.h"
#include "llvm/IR/DiagnosticPrinter.h"
#include "llvm/IR/LLVMContext.h" #include "llvm/IR/LLVMContext.h"
#include "llvm/IR/Metadata.h" #include "llvm/IR/Metadata.h"
#include "llvm/IR/Module.h" #include "llvm/IR/Module.h"
@ -146,23 +147,42 @@ LTOModule *LTOModule::createInContext(const void *mem, size_t length,
return makeLTOModule(Buffer, options, errMsg, Context); return makeLTOModule(Buffer, options, errMsg, Context);
} }
static ErrorOr<Module *> parseBitcodeFileImpl(MemoryBufferRef Buffer, static Module *parseBitcodeFileImpl(MemoryBufferRef Buffer,
LLVMContext &Context, LLVMContext &Context, bool ShouldBeLazy,
bool ShouldBeLazy) { std::string &ErrMsg) {
// Find the buffer. // Find the buffer.
ErrorOr<MemoryBufferRef> MBOrErr = ErrorOr<MemoryBufferRef> MBOrErr =
IRObjectFile::findBitcodeInMemBuffer(Buffer); IRObjectFile::findBitcodeInMemBuffer(Buffer);
if (std::error_code EC = MBOrErr.getError()) if (std::error_code EC = MBOrErr.getError()) {
return EC; ErrMsg = EC.message();
return nullptr;
}
if (!ShouldBeLazy) std::function<void(const DiagnosticInfo &)> DiagnosticHandler =
[&ErrMsg](const DiagnosticInfo &DI) {
raw_string_ostream Stream(ErrMsg);
DiagnosticPrinterRawOStream DP(Stream);
DI.print(DP);
};
if (!ShouldBeLazy) {
// Parse the full file. // Parse the full file.
return parseBitcodeFile(*MBOrErr, Context); ErrorOr<Module *> M =
parseBitcodeFile(*MBOrErr, Context, DiagnosticHandler);
if (!M)
return nullptr;
return *M;
}
// Parse lazily. // Parse lazily.
std::unique_ptr<MemoryBuffer> LightweightBuf = std::unique_ptr<MemoryBuffer> LightweightBuf =
MemoryBuffer::getMemBuffer(*MBOrErr, false); MemoryBuffer::getMemBuffer(*MBOrErr, false);
return getLazyBitcodeModule(std::move(LightweightBuf), Context); ErrorOr<Module *> M = getLazyBitcodeModule(std::move(LightweightBuf), Context,
DiagnosticHandler);
if (!M)
return nullptr;
return *M;
} }
LTOModule *LTOModule::makeLTOModule(MemoryBufferRef Buffer, LTOModule *LTOModule::makeLTOModule(MemoryBufferRef Buffer,
@ -176,13 +196,11 @@ LTOModule *LTOModule::makeLTOModule(MemoryBufferRef Buffer,
// If we own a context, we know this is being used only for symbol // If we own a context, we know this is being used only for symbol
// extraction, not linking. Be lazy in that case. // extraction, not linking. Be lazy in that case.
ErrorOr<Module *> MOrErr = parseBitcodeFileImpl( std::unique_ptr<Module> M(parseBitcodeFileImpl(
Buffer, *Context, /* ShouldBeLazy */ static_cast<bool>(OwnedContext)); Buffer, *Context,
if (std::error_code EC = MOrErr.getError()) { /* ShouldBeLazy */ static_cast<bool>(OwnedContext), errMsg));
errMsg = EC.message(); if (!M)
return nullptr; return nullptr;
}
std::unique_ptr<Module> M(MOrErr.get());
std::string TripleStr = M->getTargetTriple(); std::string TripleStr = M->getTargetTriple();
if (TripleStr.empty()) if (TripleStr.empty())

View File

@ -420,11 +420,11 @@ class ModuleLinker {
/// Functions that have replaced other functions. /// Functions that have replaced other functions.
SmallPtrSet<const Function *, 16> OverridingFunctions; SmallPtrSet<const Function *, 16> OverridingFunctions;
Linker::DiagnosticHandlerFunction DiagnosticHandler; DiagnosticHandlerFunction DiagnosticHandler;
public: public:
ModuleLinker(Module *dstM, Linker::IdentifiedStructTypeSet &Set, Module *srcM, ModuleLinker(Module *dstM, Linker::IdentifiedStructTypeSet &Set, Module *srcM,
Linker::DiagnosticHandlerFunction DiagnosticHandler) DiagnosticHandlerFunction DiagnosticHandler)
: DstM(dstM), SrcM(srcM), TypeMap(Set), : DstM(dstM), SrcM(srcM), TypeMap(Set),
ValMaterializer(TypeMap, DstM, LazilyLinkGlobalValues), ValMaterializer(TypeMap, DstM, LazilyLinkGlobalValues),
DiagnosticHandler(DiagnosticHandler) {} DiagnosticHandler(DiagnosticHandler) {}

View File

@ -1,6 +1,6 @@
; RUN: not llvm-dis < %s.bc 2>&1 | FileCheck %s ; RUN: not llvm-dis < %s.bc 2>&1 | FileCheck %s
; CHECK: llvm-dis{{(\.EXE|\.exe)?}}: Invalid value ; CHECK: llvm-dis{{(\.EXE|\.exe)?}}: Unknown attribute kind (48)
; invalid.ll.bc has an invalid attribute number. ; invalid.ll.bc has an invalid attribute number.
; The test checks that LLVM reports the error and doesn't access freed memory ; The test checks that LLVM reports the error and doesn't access freed memory

View File

@ -1,4 +1,4 @@
; RUN: not llvm-lto %S/../Bitcode/invalid.ll.bc 2>&1 | FileCheck %s ; RUN: not llvm-lto %S/../Bitcode/invalid.ll.bc 2>&1 | FileCheck %s
; CHECK: llvm-lto{{.*}}: error loading file '{{.*}}/../Bitcode/invalid.ll.bc': Invalid value ; CHECK: llvm-lto{{.*}}: error loading file '{{.*}}/../Bitcode/invalid.ll.bc': Unknown attribute kind (48)

View File

@ -19,6 +19,8 @@
#include "llvm/CodeGen/Analysis.h" #include "llvm/CodeGen/Analysis.h"
#include "llvm/CodeGen/CommandFlags.h" #include "llvm/CodeGen/CommandFlags.h"
#include "llvm/IR/Constants.h" #include "llvm/IR/Constants.h"
#include "llvm/IR/DiagnosticInfo.h"
#include "llvm/IR/DiagnosticPrinter.h"
#include "llvm/IR/LLVMContext.h" #include "llvm/IR/LLVMContext.h"
#include "llvm/IR/Module.h" #include "llvm/IR/Module.h"
#include "llvm/IR/Verifier.h" #include "llvm/IR/Verifier.h"
@ -269,6 +271,23 @@ static bool shouldSkip(uint32_t Symflags) {
return false; return false;
} }
static void diagnosticHandler(const DiagnosticInfo &DI, void *Context) {
assert(DI.getSeverity() == DS_Error && "Only expecting errors");
const auto &BDI = cast<BitcodeDiagnosticInfo>(DI);
std::error_code EC = BDI.getError();
if (EC == BitcodeError::InvalidBitcodeSignature)
return;
std::string ErrStorage;
{
raw_string_ostream OS(ErrStorage);
DiagnosticPrinterRawOStream DP(OS);
DI.print(DP);
}
message(LDPL_FATAL, "LLVM gold plugin has failed to create LTO module: %s",
ErrStorage.c_str());
}
/// Called by gold to see whether this file is one that our plugin can handle. /// Called by gold to see whether this file is one that our plugin can handle.
/// We'll try to open it and register all the symbols with add_symbol if /// We'll try to open it and register all the symbols with add_symbol if
/// possible. /// possible.
@ -302,11 +321,11 @@ static ld_plugin_status claim_file_hook(const ld_plugin_input_file *file,
BufferRef = Buffer->getMemBufferRef(); BufferRef = Buffer->getMemBufferRef();
} }
Context.setDiagnosticHandler(diagnosticHandler);
ErrorOr<std::unique_ptr<object::IRObjectFile>> ObjOrErr = ErrorOr<std::unique_ptr<object::IRObjectFile>> ObjOrErr =
object::IRObjectFile::create(BufferRef, Context); object::IRObjectFile::create(BufferRef, Context);
std::error_code EC = ObjOrErr.getError(); std::error_code EC = ObjOrErr.getError();
if (EC == BitcodeError::InvalidBitcodeSignature || if (EC == object::object_error::invalid_file_type ||
EC == object::object_error::invalid_file_type ||
EC == object::object_error::bitcode_section_not_found) EC == object::object_error::bitcode_section_not_found)
return LDPS_OK; return LDPS_OK;

View File

@ -20,6 +20,8 @@
#include "llvm/Bitcode/ReaderWriter.h" #include "llvm/Bitcode/ReaderWriter.h"
#include "llvm/IR/AssemblyAnnotationWriter.h" #include "llvm/IR/AssemblyAnnotationWriter.h"
#include "llvm/IR/DebugInfo.h" #include "llvm/IR/DebugInfo.h"
#include "llvm/IR/DiagnosticInfo.h"
#include "llvm/IR/DiagnosticPrinter.h"
#include "llvm/IR/IntrinsicInst.h" #include "llvm/IR/IntrinsicInst.h"
#include "llvm/IR/Module.h" #include "llvm/IR/Module.h"
#include "llvm/IR/Type.h" #include "llvm/IR/Type.h"
@ -112,6 +114,17 @@ public:
} // end anon namespace } // end anon namespace
static void diagnosticHandler(const DiagnosticInfo &DI, void *Context) {
assert(DI.getSeverity() == DS_Error && "Only expecting errors");
raw_ostream &OS = errs();
OS << (char *)Context << ": ";
DiagnosticPrinterRawOStream DP(OS);
DI.print(DP);
OS << '\n';
exit(1);
}
int main(int argc, char **argv) { int main(int argc, char **argv) {
// Print a stack trace if we signal out. // Print a stack trace if we signal out.
sys::PrintStackTraceOnErrorSignal(); sys::PrintStackTraceOnErrorSignal();
@ -120,6 +133,7 @@ int main(int argc, char **argv) {
LLVMContext &Context = getGlobalContext(); LLVMContext &Context = getGlobalContext();
llvm_shutdown_obj Y; // Call llvm_shutdown() on exit. llvm_shutdown_obj Y; // Call llvm_shutdown() on exit.
Context.setDiagnosticHandler(diagnosticHandler, argv[0]);
cl::ParseCommandLineOptions(argc, argv, "llvm .bc -> .ll disassembler\n"); cl::ParseCommandLineOptions(argc, argv, "llvm .bc -> .ll disassembler\n");
@ -136,25 +150,8 @@ int main(int argc, char **argv) {
DisplayFilename = InputFilename; DisplayFilename = InputFilename;
ErrorOr<std::unique_ptr<Module>> MOrErr = ErrorOr<std::unique_ptr<Module>> MOrErr =
getStreamedBitcodeModule(DisplayFilename, Streamer, Context); getStreamedBitcodeModule(DisplayFilename, Streamer, Context);
if (std::error_code EC = MOrErr.getError()) M = std::move(*MOrErr);
ErrorMessage = EC.message(); M->materializeAllPermanently();
else
M = std::move(*MOrErr);
if(M.get()) {
if (std::error_code EC = M->materializeAllPermanently()) {
ErrorMessage = EC.message();
M.reset();
}
}
}
if (!M.get()) {
errs() << argv[0] << ": ";
if (ErrorMessage.size())
errs() << ErrorMessage << "\n";
else
errs() << "bitcode didn't read correctly.\n";
return 1;
} }
// Just use stdout. We won't actually print anything on it. // Just use stdout. We won't actually print anything on it.