[Debuginfo][NFC] Unify error reporting routines inside DebugInfoDWARF.

[avl]

Summary

Error reporting in DebugInfoDWARF library currently done in three ways :

1. Direct calls to WithColor::error()/WithColor::warning()
2. ErrorPolicy defaultErrorHandler(Error E);
3. void dumpWarning(Error Warning);

additionally, other locations could have more variations:

lld/ELF/SyntheticSection.cpp

if (Error e = cu->tryExtractDIEsIfNeeded(false)) {
  error(toString(sec) + ": " + toString(std::move(e)));

DebugInfo/DWARF/DWARFUnit.cpp

if (Error e = tryExtractDIEsIfNeeded(CUDieOnly))
  WithColor::error() << toString(std::move(e));

Thus error reporting could look inconsistent. To have a consistent error
messages it is necessary to have a possibility to redefine error
reporting functions. This patch creates two handlers and allows to
redefine them. It also patches all places inside DebugInfoDWARF
to use these handlers.

The intention is always to use following handlers for error reporting
purposes inside DebugInfoDWARF:

DebugInfo/DWARF/DWARFContext.h

std::function<void(Error E)> RecoverableErrorHandler = WithColor::defaultErrorHandler;
std::function<void(Error E)> WarningHandler = WithColor::defaultWarningHandler;

This is last patch from series of patches: D74481, D74635, D75118.

[avl]

Unit tests pass. 62578 tests passed, 0 failed and 842 were skipped.

clang-tidy fail. clang-tidy found 0 errors and 1 warnings.

clang-format pass.

[jhenderson]

Overall, I like the principle of this change, but you should definitely hear what others have to say too.

llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h

353

Having a setter and getter for these seems a bit verbose, especially when you've got cases where you use the getter and then immediately call the return. Is there any particular reason to hide the handlers behind the interface?

llvm/lib/DWARFLinker/DWARFLinker.cpp

362 ↗

(On Diff #243503)

I know it's not related to your change, but since you are touching the line anyway, could you change 8 /* Indent */ to /*Indent=*/8, please, as clang-format plays well with that.

400 ↗

(On Diff #243503)

Ditto

llvm/lib/DebugInfo/DWARF/DWARFDie.cpp

642 ↗

(On Diff #243503)

Have you considered using DWARFContext::defaultErrorHandler here?

llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp

240 ↗

(On Diff #243503)

Same comment as elsewhere re. "indent"

llvm/tools/dsymutil/DwarfLinkerForBinary.cpp

162 ↗

(On Diff #243503)

Same comments re. using DWARFContext::defaultErrorHandler and "Indent" comment.

[avl]

llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h

353

There is no particular reason. Having getter/setter allows to add some code like checking for nullptr, or dumping, or something else... If it looks verbose I would remove it.

llvm/lib/DWARFLinker/DWARFLinker.cpp

362 ↗

(On Diff #243503)

Ok.

llvm/lib/DebugInfo/DWARF/DWARFDie.cpp

642 ↗

(On Diff #243503)

Ah, indeed. Would use it.

[dblaikie]

Overall it's a worthwhile direction - but please split this into smaller patches. General renamings separate, adding the functionality and using it in one place (preferably one that doesn't require function signature changes - one that already has the DWARFContext available, etc) & then a separate patch probably for each different error handling location that requires a function signature change. I think it'll be easier to review that way & make it clearer what changes were needed and how the error handling is being layered, etc.

llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h

89–91

Does this need to be wrapped in a lambda here?

353

@jhenderson : perhaps/did you mean "Is there any particular reason /not/ to hide the handlers behind the interface" - ie: I think you might've been suggesting that, rather than having "getRecoverableErrorHandler" the interface would have "emitRecoverableError(ERror E)" that calls the handler. Not exposing the handler itself to users of DWARFContext? Is that what you were/are suggesting? It sounds like a reasonable suggestion to me - at least worth asking/considering.

As for the setters - they seem to be uncalled right now? (just searching for them in this phab page doesn't show up any other instance of their names) - so probably best to leave them out until they're needed & we can discuss the merits of their use then?

llvm/include/llvm/DebugInfo/DWARF/DWARFDebugLine.h

307 ↗

(On Diff #243503)

Please separate this sort of renaming into a pre or post patch to make this review easier/more targeted.

[jhenderson]

llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h

353

I don't think it's what I meant when I wrote that comment, but I think your suggestion manifesting itself as a half-formed thought was what triggered me to write a comment at all! I agree that an "emitRecovarableError(Error E)" signature would make a lot of sense - all it would do would be to call the handler with the passed-in error.

[avl]

llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h

353

I did not clearly get idea of "emitRecoverableError(ERror E)". My understanting is that it allows to change this usage :

public:  std::function<void(Error E)> RecoverableErrorHandler; // field of DWARFContext
...
DWARFContext.RecoverableErrorHandler(E);  // emit error.

with this usage:

DWARFContext.emitRecoverableError(E);  // emit error, RecoverableErrorHandler is not seen

But what about this usage :

func (std::function<void(Error E)> ErrorHandler)  // some function
      { ErrorHandler(Error); }


func(DWARFContext.RecoverableErrorHandler);  // pass handler to function

How would this case be resolved with emitRecoverableError() ?

Something like this:

func([&](Error E ) { DWARFContext.emitRecoverableError(std::move(E)); }   // pass handler to function

?

[jhenderson]

llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h

353

I'm not sure I follow your comments I'm afraid. The suggestion is to replace getRecoverableErrorHandler() with the following function:

void emitRecoverableError(Error Err) {
  RecoverableErrorHandler(std::move(Err));
}

Usage would be either:

Context.emitRecoverableError(createStringError("some text"));

or

doSomething(/*Callback=*/Context.emitRecoverableError);
...
void doSomething(function_ref<void(Error E)> Handler) {
...
  Handler(createStringError("some error"));
}

depending on whether the handler needs to be passed into the function, or the Context can be accessed directly. Effectively, in the second case, the real error handler is wrapped in another function that calls it.

[avl]

llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h

353

Ah, OK, Thanks. I misinterpreted second usage ...

[avl]

llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h

353

But... it looks like it does not work that way :

doSomething(/*Callback=*/Context.emitRecoverableError);

error: reference to non-static member function must be called

It needs to be

doSomething(/*Callback=*/[&](Error Err){Context.emitRecoverableError(std::move(Err))});

[avl]

llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h

353

@dblaikie David, How emitRecoverableError() should be used for this scenario ? :

doSomething(/*Callback=*/Context.emitRecoverableError);
...
void doSomething(function_ref<void(Error E)> Handler) {
...
  Handler(createStringError("some error"));
}

Exactly this variant seems not working, since std::function/function_ref could not be assigned to member function. it is necessary to use proxy lambda function or use std::bind to make it working.

But that probably is not very convenient solution.

[dblaikie]

llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h

353

Yeah, fair enough - seems like there are enough places that want to pass the handler along to that don't have a DWARFContext directly accessible (& makes sense to just pass down the handlers, rather than the whole DWARFContext). Seems fine as-is (as an accessor for the handler), then. Maybe @jhenderson has other thoughts since it was his feedback originally.

Still seems like the setters should be removed if (as they appear to be) they are unused.

[avl]

llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h

353

Ok. I also think it would be fine to use an accessor here :

std::function<void(Error Err)> getRecoverableErrorHandler {
    return RecoverableErrorHandler;
}

Though @jhenderson asked for publicly visible member originally:

std::function<void(Error Err)> RecoverableErrorHandler;

will wait for opinions more...

[jhenderson]

llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h

353

Public accessor is fine. I don't have a strong opinion on it either way.

[avl]

As it was requested, D74481, D74635 patches already integrate
most of functionality. This final part adds the possibility
to redefine error handlers into constructor of DWARFContext
and DWARFContext::create() functions.

[jhenderson]

llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h

110

This can just be RecoverablErrorHandler. No need for the The - constructor arguments can be named the same as the thing they're initialising. Same goes throughout most of the other functions too.

354–355

objectFileErrorsHandler -> objectFileErrorHandler

357–362

I don't think you need to pass this in as well as a RecoverableErrorHandler. The two shouldn't need to diverge, I think. Rather, have ObjectFileErrorHandler be defined in terms of RecoverableErrorHandler, as suggested below.

llvm/lib/DebugInfo/DWARF/DWARFContext.cpp

1433–1434

Shouldn't this function use the provided error handler rather than the default one?

In fact, maybe this should just be an inline lambda now.

[avl]

llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h

110

Ok.

354–355

ok.

357–362

There are two problems here:

1. Using RecoverableErrorHandler for implementing ObjectFileErrorHandler.
   
   ObjectFileErrorHandler is a static function member, thus it could not be implemented using non-static function member RecoverableErrorHandler.

2. Passing one Error handler instead of two error Handlers. i.e.:
   
   instead of passing objectFileErrorsHandler and RecoverableErrorHandler into DWARFContext::create() pass only one handler.
   
   As far as I understand original logic for DWARFContext::create(), there should be possible to pass handler which would stop processing if error occurred. i.e. it would return ErrorPolicy::Halt. That behavior is important for the creation function. But for the other DWARFContext errors that policy is not supported. It could probably be incorrect to use handler which returns ErrorPolicy::Halt for other DWARFContext errors.
   
   Since error logic for create() method and for other places differs - it looks correct to use different handlers. What do you think?

llvm/lib/DebugInfo/DWARF/DWARFContext.cpp

1433–1434

since objectFileErrorsHandler is a static function - it could not use provided error handler(which is not-static member - RecoverableErrorHandler)

[avl]

llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h

357–362

@jhenderson James, what about passing two error handlers to "create" function?:

create(const object::ObjectFile &Obj, const LoadedObjectInfo *L = nullptr,
         function_ref<ErrorPolicy(Error)> ObjectFileErrorsHandler =
             objectFileErrorsHandler,
         std::string DWPName = "",
         std::function<void(Error)> RecoverableErrorHandler =
             WithColor::defaultErrorHandler,
         std::function<void(Error)> WarningHandler =
             WithColor::defaultWarningHandler);

Would it be OK ?

As an alternative objectFileErrorsHandler default implementation could be removed at all, with the price that every usage place would need to create it by itself:

create(const object::ObjectFile &Obj, const LoadedObjectInfo *L = nullptr,
         function_ref<ErrorPolicy(Error)> ObjectFileErrorsHandler =
             [](Error E)->ErrorPolicy {
               WithColor::defaultErrorHandler(std::move(E));
               return ErrorPolicy::Continue;
             },
         std::string DWPName = "",
         std::function<void(Error)> RecoverableErrorHandler =
             WithColor::defaultErrorHandler,
         std::function<void(Error)> WarningHandler =
             WithColor::defaultWarningHandler);

Context = DWARFContext::create(*Objects.second, nullptr,
                               [](Error E)->ErrorPolicy {
                                 WithColor::defaultErrorHandler(std::move(E));
                                 return ErrorPolicy::Continue;
                               }, Opts.DWPName);

[avl]

renamed parameters. objectFileErrorHandler is not addressed yet.

[avl]

@jhenderson I did not clearly get what should be done for handlers for create function. How following should be changed?

static ErrorPolicy objectFileErrorHandler(Error E);

ErrorPolicy DWARFContext::objectFileErrorHandler(Error E) {
  WithColor::defaultErrorHandler(std::move(E));
  return ErrorPolicy::Continue;
}
....
create(const object::ObjectFile &Obj, const LoadedObjectInfo *L = nullptr,
         function_ref<ErrorPolicy(Error)> ObjectFileErrorHandler = objectFileErrorHandler,
         std::string DWPName = "",
         std::function<void(Error)> RecoverableErrorHandler = WithColor::defaultErrorHandler,
         std::function<void(Error)> WarningHandler = WithColor::defaultWarningHandler);

.....
    Context = DWARFContext::create(*Objects.second, nullptr,
                                   DWARFContext::objectFileErrorHandler,
                                   Opts.DWPName);

[jhenderson]

Sorry for the delay in responding. It's been a busy few days. I just looked around the LLVM code base and could find no instance of the existing defaultErrorHandler being replaced with a different handler. Is it possible this is now dead code? It looks like @grimar originally added it 3 years ago.

[avl]

Aha, I see. Yes, it looks like it is not used currently. The only place is DWARFDebugInfoTest.cpp which checks for that feature. So it seems like that DWARFContext::defaultErrorHandler could be removed:

create(const object::ObjectFile &Obj, const LoadedObjectInfo *L = nullptr,
         std::string DWPName = "",
         std::function<void(Error)> RecoverableErrorHandler = WithColor::defaultErrorHandler,
         std::function<void(Error)> WarningHandler = WithColor::defaultWarningHandler);

Will wait for @grimar comments.

[grimar]

In D74308#1889316, @avl wrote:

Will wait for @grimar comments.

The error handler was implemented in D34328 and used in LLD: D34814.
It was needed because when an error happened during parsing a debug data, the code in lib/DebugInfo
just printed something to errs() and did not let LLD know about the error (see D34814).
LLD's implementation of .gdb_index (that is for what we are using lib/DebugInfo code) changed significantly since that time,
seems LLD does not use D34328 code anymore.

In D74308#1889097, @jhenderson wrote:

Is it possible this is now dead code? It looks like @grimar originally added it 3 years ago.

It probably might be usefull for people who might want to use DWARFObjInMemory class like we had in LLD before.
This error handler was the only good way to know on a caller side that something went wrong during DWARF parsing.

[jhenderson]

Right, so I think I see a path forward. I agree with @grimar that we need a way to pass the errors encountered during construction of DWARFObjInMemory back to the caller of the constructor (and therefore in turn to the caller of DWARFContext::create). However, I do not see a need for the existing continue/halt distinction. It seems harmless (aside from maybe slightly less performant) to continue in all cases. This means that we can change the signature of the callback to void(Error), like the other error callbacks, which in turn means we can use WithColor::defaultErrorHandler as the default error handling method instead of objectFileErrorHandler, and indeed, allows us therefore to remove the extra argument from the signature of create (because we can just use the RecoverableErrorHandler argument).

Does that make sense?

[grimar]

In D74308#1890911, @jhenderson wrote:

Right, so I think I see a path forward. I agree with @grimar that we need a way to pass the errors encountered during construction of DWARFObjInMemory back to the caller of the constructor (and therefore in turn to the caller of DWARFContext::create). However, I do not see a need for the existing continue/halt distinction. It seems harmless (aside from maybe slightly less performant) to continue in all cases. This means that we can change the signature of the callback to void(Error), like the other error callbacks, which in turn means we can use WithColor::defaultErrorHandler as the default error handling method instead of objectFileErrorHandler, and indeed, allows us therefore to remove the extra argument from the signature of create (because we can just use the RecoverableErrorHandler argument).

Does that make sense?

Sounds probably fine to me.

[avl]

Ok. just to make me sure: we are going to remove "enum class ErrorPolicy { Halt, Continue };" completetly ?

[grimar]

In D74308#1891014, @avl wrote:

Ok. just to make me sure: we are going to remove "enum class ErrorPolicy { Halt, Continue };" completetly ?

I believe this is what @jhenderson's suggestion does. There are no other users of ErrorPolicy I think, so yes.

[avl]

addressed comments: deleted handler with ErrorPolicy.

[jhenderson]

Looks good, but I'd pull the ErrorPolicy related changes into a separate patch.

llvm/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp

2532

"both two errors we know about" -> "the two errors we expect"

[avl]

updated after D75118 was extracted.

[jhenderson]

LGTM. Thanks for the work on this!

[avl]

@jhenderson Thanks!

llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h

class DWARFContext : public DIContext {
   };
   StringMap<std::weak_ptr<DWOFile>> DWOFiles;
   std::weak_ptr<DWOFile> DWP;
   bool CheckedForDWP = false;
   std::string DWPName;
 
   std::unique_ptr<MCRegisterInfo> RegInfo;
 
   std::function<void(Error)> RecoverableErrorHandler =
       WithColor::defaultErrorHandler;
   std::function<void(Error)> WarningHandler = WithColor::defaultWarningHandler;
 
   /// Read compile units from the debug_info section (if necessary)
   /// and type units from the debug_types sections (if necessary)
   /// and store them in NormalUnits.
   void parseNormalUnits();
 
   /// Read compile units from the debug_info.dwo section (if necessary)
   /// and type units from the debug_types.dwo section (if necessary)
   /// and store them in DWOUnits.
   /// If \p Lazy is true, set up to parse but don't actually parse them.
   enum { EagerParse = false, LazyParse = true };
   void parseDWOUnits(bool Lazy = false);
 
   std::unique_ptr<const DWARFObject> DObj;
 
 public:
   DWARFContext(std::unique_ptr<const DWARFObject> DObj,
-               std::string DWPName = "");
+               std::string DWPName = "",
+               std::function<void(Error)> RecoverableErrorHandler =
+                   WithColor::defaultErrorHandler,
+               std::function<void(Error)> WarningHandler =
+                   WithColor::defaultWarningHandler);
   ~DWARFContext();
 
   DWARFContext(DWARFContext &) = delete;
   DWARFContext &operator=(DWARFContext &) = delete;
 
   const DWARFObject &getDWARFObj() const { return *DObj; }
 
   static bool classof(const DIContext *DICtx) {
public:
   std::shared_ptr<DWARFContext> getDWOContext(StringRef AbsolutePath);
 
   const MCRegisterInfo *getRegisterInfo() const { return RegInfo.get(); }
 
   function_ref<void(Error)> getRecoverableErrorHandler() {
     return RecoverableErrorHandler;
   }
 
   function_ref<void(Error)> getWarningHandler() { return WarningHandler; }
 
   static std::unique_ptr<DWARFContext>
   create(const object::ObjectFile &Obj, const LoadedObjectInfo *L = nullptr,
-         function_ref<void(Error)> HandleError = WithColor::defaultErrorHandler,
-         std::string DWPName = "");
+         std::string DWPName = "",
+         std::function<void(Error)> RecoverableErrorHandler =
+             WithColor::defaultErrorHandler,
+         std::function<void(Error)> WarningHandler =
+             WithColor::defaultWarningHandler);
 
   static std::unique_ptr<DWARFContext>
   create(const StringMap<std::unique_ptr<MemoryBuffer>> &Sections,
-         uint8_t AddrSize, bool isLittleEndian = sys::IsLittleEndianHost);
+         uint8_t AddrSize, bool isLittleEndian = sys::IsLittleEndianHost,
+         std::function<void(Error)> RecoverableErrorHandler =
+             WithColor::defaultErrorHandler,
+         std::function<void(Error)> WarningHandler =
+             WithColor::defaultWarningHandler);
 
   /// Loads register info for the architecture of the provided object file.
   /// Improves readability of dumped DWARF expressions. Requires the caller to
   /// have initialized the relevant target descriptions.
   Error loadRegisterInfo(const object::ObjectFile &Obj);
 
   /// Get address size from CUs.
   /// TODO: refactor compile_units() to make this const.

llvm/lib/DebugInfo/DWARF/DWARFContext.cpp

 
 #define DEBUG_TYPE "dwarf"
 
 using DWARFLineTable = DWARFDebugLine::LineTable;
 using FileLineInfoKind = DILineInfoSpecifier::FileLineInfoKind;
 using FunctionNameKind = DILineInfoSpecifier::FunctionNameKind;
 
 DWARFContext::DWARFContext(std::unique_ptr<const DWARFObject> DObj,
-                           std::string DWPName)
-    : DIContext(CK_DWARF), DWPName(std::move(DWPName)), DObj(std::move(DObj)) {}
+                           std::string DWPName,
+                           std::function<void(Error)> RecoverableErrorHandler,
+                           std::function<void(Error)> WarningHandler)
+    : DIContext(CK_DWARF), DWPName(std::move(DWPName)),
+      RecoverableErrorHandler(RecoverableErrorHandler),
+      WarningHandler(WarningHandler), DObj(std::move(DObj)) {}
 
 DWARFContext::~DWARFContext() = default;
 
 /// Dump the UUID load command.
 static void dumpUUID(raw_ostream &OS, const ObjectFile &Obj) {
   auto *MachO = dyn_cast<MachOObjectFile>(&Obj);
   if (!MachO)
     return;
static bool isRelocScattered(const object::ObjectFile &Obj,
   const MachOObjectFile *MachObj = dyn_cast<MachOObjectFile>(&Obj);
   if (!MachObj)
     return false;
   // MachO also has relocations that point to sections and
   // scattered relocations.
   auto RelocInfo = MachObj->getRelocation(Reloc.getRawDataRefImpl());
   return MachObj->isRelocationScattered(RelocInfo);
 }
 
 namespace {
 struct DWARFSectionMap final : public DWARFSection {
   RelocAddrMap Relocs;
 };
 
 class DWARFObjInMemory final : public DWARFObject {
   bool IsLittleEndian;
   uint8_t AddressSize;
   StringRef FileName;
void forEachTypesSections(
     for (auto &P : TypesSections)
       F(P.second);
   }
 };
 } // namespace
 
 std::unique_ptr<DWARFContext>
 DWARFContext::create(const object::ObjectFile &Obj, const LoadedObjectInfo *L,
-                     function_ref<void(Error)> HandleError,
-                     std::string DWPName) {
-  auto DObj = std::make_unique<DWARFObjInMemory>(Obj, L, HandleError);
-  return std::make_unique<DWARFContext>(std::move(DObj), std::move(DWPName));
+                     std::string DWPName,
+                     std::function<void(Error)> RecoverableErrorHandler,
+                     std::function<void(Error)> WarningHandler) {
+  auto DObj =
+      std::make_unique<DWARFObjInMemory>(Obj, L, RecoverableErrorHandler);
+  return std::make_unique<DWARFContext>(std::move(DObj), std::move(DWPName),
+                                        RecoverableErrorHandler,
+                                        WarningHandler);
 }
 
 std::unique_ptr<DWARFContext>
 DWARFContext::create(const StringMap<std::unique_ptr<MemoryBuffer>> &Sections,
-                     uint8_t AddrSize, bool isLittleEndian) {
+                     uint8_t AddrSize, bool isLittleEndian,
+                     std::function<void(Error)> RecoverableErrorHandler,
+                     std::function<void(Error)> WarningHandler) {
   auto DObj =
       std::make_unique<DWARFObjInMemory>(Sections, AddrSize, isLittleEndian);
-  return std::make_unique<DWARFContext>(std::move(DObj), "");
+  return std::make_unique<DWARFContext>(
+      std::move(DObj), "", RecoverableErrorHandler, WarningHandler);
 }
 
 Error DWARFContext::loadRegisterInfo(const object::ObjectFile &Obj) {
   // Detect the architecture from the object file. We usually don't need OS
   // info to lookup a target and create register info.
   Triple TT;
   TT.setArch(Triple::ArchType(Obj.getArch()));
   TT.setVendor(Triple::UnknownVendor);

llvm/lib/DebugInfo/Symbolize/Symbolize.cpp

if (!EC && DebugInfo != nullptr && !PDBFileName.empty()) {
         Modules.emplace(ModuleName, std::unique_ptr<SymbolizableModule>());
         // Return along the PDB filename to provide more context
         return createFileError(PDBFileName, std::move(Err));
       }
       Context.reset(new PDBContext(*CoffObject, std::move(Session)));
     }
   }
   if (!Context)
-    Context = DWARFContext::create(
-        *Objects.second, nullptr, WithColor::defaultErrorHandler, Opts.DWPName);
+    Context = DWARFContext::create(*Objects.second, nullptr, Opts.DWPName);
   return createModuleInfo(Objects.first, std::move(Context), ModuleName);
 }
 
 namespace {
 
 // Undo these various manglings for Win32 extern "C" functions:
 // cdecl       - _foo
 // stdcall     - _foo@12

llvm/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp

TEST(DWARFDebugInfo, TestErrorReporting) {
   AP->OutStreamer->SwitchSection(
       MC->getELFSection(".zdebug_bar", 0 /*Type*/, 0 /*Flags*/));
   AP->OutStreamer->emitBytes("0");
 
   MemoryBufferRef FileBuffer(DG->generate(), "dwarf");
   auto Obj = object::ObjectFile::createObjectFile(FileBuffer);
   EXPECT_TRUE((bool)Obj);
 
   // DWARFContext parses whole file and finds the two errors we expect.
   int Errors = 0;
   std::unique_ptr<DWARFContext> Ctx1 =
-      DWARFContext::create(**Obj, nullptr, [&](Error E) {
+      DWARFContext::create(**Obj, nullptr, "", [&](Error E) {
         ++Errors;
         consumeError(std::move(E));
       });
   EXPECT_TRUE(Errors == 2);
 }
 
 TEST(DWARFDebugInfo, TestDwarfVerifyCURangesIncomplete) {
   // Create a single compile unit with a single function. The compile