[llmv-pdbutil] Replace ExitOnError with explicit error handling.

[CarlosAlbertoEnciso]

Summary

[llmv-pdbutil] Replace ExitOnError with explicit error handling.

At Sony we are developing llvm-dva

https://lists.llvm.org/pipermail/llvm-dev/2020-August/144174.html

For its PDB support, it requires functionality already present
in llvm-pdbutil.

We intend to move that functionaly into the PDB library to be
shared by both tools. That change will be done in 2 steps, that
will be submitted as 2 patches:

(1) Replace 'ExitOnError' with explicit error handling.
(2) Move the intended shared code to the PDB library.

This patch is for step (1).

As 'ExitOnError' is intended to be used only in tool code, replace
all occurrences in the code that will be moved to the PDB library
with explicit error handling.

[aganea]

llvm/tools/llvm-pdbutil/DumpOutputStyle.cpp

455

The user would loose the context after removal of this line, if the message is printed on stderr. Can you move this line to L201, or replace it by an extra StringError if you plan on moving this entire file to the PDB library? It could be something like:

if (opts::dump::DumpSymbols) {
  auto EC = File.isPdb() ? dumpModuleSymsForPdb() : dumpModuleSymsForObj();
  if (EC)
    return joinErrors(createStringError(inconvertibleErrorCode(), "Unexpected error processing modules:"), EC);
}

681

You can early exit here.

llvm/tools/llvm-pdbutil/InputFile.cpp

40–43

Even though auto is already used throughout this file, I find it's hard for a person reading the code to understand what type the function returns. Also please see https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable

It's probably better if we wrote Expected<DbiStream &> DbiOrErr = ... and DbiStream &Dbi = ... below. Same applies for other places/usage of auto throughout this patch.

[rnk]

Essentially, this looks good to me. I have some minor concerns that as we migrate dumping functionality into library code, it will tend to increase the code size of production tools such as the linker. Maybe archive semantics and section GC paper over those problems. However, in the end I don't think it makes sense to worry about unmeasured, hypothetical code size problems. If there are problems, I guess it would make sense to factor out "dumping" functionanlity from the PDB and CodeView libraries into their own CVDump helper library or something. This is similar to how we pulled YAML writing out of lib/Object.

llvm/tools/llvm-pdbutil/InputFile.cpp

40–43

I wanted to ask if we can write Expected<auto>, but no, that is not valid C++. :(

I don't think I can make a strong argument for either direction. Fully writing out all Expected types seems onerous, but this also seems like an overuse of auto. The type that the reader cares about most is probably DbiStream.

[dblaikie]

llvm/tools/llvm-pdbutil/InputFile.cpp

40–43

I'm mostly OK with using auto for Expected - especially if you're going to name the extracted value, as is done here (Dbi) - if you want to include the type name for that for readability, you could include it there (DbiStream &Dbi = *DbiOrErr). To me - the Expected's pratical use/scope is 4 lines, and the variable name and its 3 uses all communicate, to me at least, pretty clearly what it is/how it's being used.

40–43

But I should say I don't feel super strongly either way - as @rnk said.

[aganea]

llvm/tools/llvm-pdbutil/InputFile.cpp

40–43

Agreed - the most important is to have DbiStream &Dbi = *DbiOrErr; below.

[CarlosAlbertoEnciso]

As stated, we intend to move some functionality into the PDB library:

Patch (1) Replace 'ExitOnError' with explicit error handling.
Patch (2) Move the intended shared code to the PDB library.

With the moved functionality, we are able to:

• Open a PDB file.
• Get module debug stream.
• Traverse module sections.
• Traverse module subsections.

The second patch only moves InputFile.cpp[h], LinePrinter.cpp[h] and FormatUtil.cpp[h] files.

Most of the needed functionality is in InputFile, but there are dependencies from LinePrinter and FormatUtil.

Some other functionality is in the following functions in DumpOutputStyle.cpp file:

• iterateModuleSubsections
• getModuleDebugStream
• iterateOneModule
• iterateSymbolGroups
• iterateModuleSubsections

Only these specific functions from DumpOutputStyle are moved to the PDB library.

[CarlosAlbertoEnciso]

• Added details about the intended second patch, that includes: files and functions being moved into the PDB library.
• Replaced the 'auto' for the specific: Expected<DbiStream &> and DbiStream.
• Added an early exit.

The only remainding comment not addressed is from @aganea, in relation to the

ExitOnError Err("Unexpected error processing modules: ");

[CarlosAlbertoEnciso]

llvm/tools/llvm-pdbutil/DumpOutputStyle.cpp

455

Very valid point.

The following functions are affected:

Error DumpOutputStyle::dumpModules();
Error DumpOutputStyle::dumpModuleFiles();
Error DumpOutputStyle::dumpSymbolStats();
Error DumpOutputStyle::dumpModuleSymsForPdb();

What I am suggesting is to pass an additional parameter to the functions that are intended to be moved to the PDB library that now are returning an Error and add your suggested code to those functions.

if (EC)
    return joinErrors(createStringError(inconvertibleErrorCode(), ExtraMessage), EC);

681

Added the early exit.

[aganea]

LGTM, thanks!

In D121801#3386781, @rnk wrote:

Essentially, this looks good to me. I have some minor concerns that as we migrate dumping functionality into library code, it will tend to increase the code size of production tools such as the linker. Maybe archive semantics and section GC paper over those problems. However, in the end I don't think it makes sense to worry about unmeasured, hypothetical code size problems. If there are problems, I guess it would make sense to factor out "dumping" functionanlity from the PDB and CodeView libraries into their own CVDump helper library or something. This is similar to how we pulled YAML writing out of lib/Object.

@CarlosAlbertoEnciso Would you be able to provide some numbers please along with patch (2)? At least we'll be settled if this code move should go in a new lib or not.

llvm/tools/llvm-pdbutil/DumpOutputStyle.cpp

455

I guess you can also move the ExitOnError closer to the caller site:

Error DumpOutputStyle::dump() {
  ...
  if (opts::dump::DumpModules) {
    ExitOnError Err("Unexpected error processing modules: ");
    Err(dumpModules());
  }

[CarlosAlbertoEnciso]

llvm/tools/llvm-pdbutil/DumpOutputStyle.cpp

455

Added your suggested change to the caller sites that are missing the ExitOnError.

[CarlosAlbertoEnciso]

Addressed the remainding comment from @aganea, in relation to the

ExitOnError Err("Unexpected error processing modules: ");

[CarlosAlbertoEnciso]

@aganea, @dblaikie, @rnk thanks for your reviews.

llvm/tools/llvm-pdbutil/DumpOutputStyle.cpp

Error DumpOutputStyle::dump() {
 
   if (opts::dump::DumpStreams) {
     if (auto EC = dumpStreamSummary())
       return EC;
     P.NewLine();
   }
 
   if (opts::dump::DumpSymbolStats) {
-    if (auto EC = dumpSymbolStats())
-      return EC;
+    ExitOnError Err("Unexpected error processing module stats: ");
+    Err(dumpSymbolStats());
     P.NewLine();
   }
 
   if (opts::dump::DumpUdtStats) {
     if (auto EC = dumpUdtStats())
       return EC;
     P.NewLine();
   }
Error DumpOutputStyle::dump() {
 
   if (opts::dump::DumpStringTable || opts::dump::DumpStringTableDetails) {
     if (auto EC = dumpStringTable())
       return EC;
     P.NewLine();
   }
 
   if (opts::dump::DumpModules) {
-    if (auto EC = dumpModules())
-      return EC;
+    ExitOnError Err("Unexpected error processing modules: ");
+    Err(dumpModules());
   }
 
   if (opts::dump::DumpModuleFiles) {
-    if (auto EC = dumpModuleFiles())
-      return EC;
+    ExitOnError Err("Unexpected error processing files: ");
+    Err(dumpModuleFiles());
   }
 
   if (opts::dump::DumpLines) {
-    if (auto EC = dumpLines())
-      return EC;
+    ExitOnError Err("Unexpected error processing lines: ");
+    Err(dumpLines());
   }
 
   if (opts::dump::DumpInlineeLines) {
-    if (auto EC = dumpInlineeLines())
-      return EC;
+    ExitOnError Err("Unexpected error processing inlinee lines: ");
+    Err(dumpInlineeLines());
   }
 
   if (opts::dump::DumpXmi) {
-    if (auto EC = dumpXmi())
-      return EC;
+    ExitOnError Err("Unexpected error processing cross module imports: ");
+    Err(dumpXmi());
   }
 
   if (opts::dump::DumpXme) {
-    if (auto EC = dumpXme())
-      return EC;
+    ExitOnError Err("Unexpected error processing cross module exports: ");
+    Err(dumpXme());
   }
 
   if (opts::dump::DumpFpo) {
     if (auto EC = dumpFpo())
       return EC;
   }
 
   if (File.isObj()) {
Error DumpOutputStyle::dump() {
   }
 
   if (opts::dump::DumpPublics) {
     if (auto EC = dumpPublics())
       return EC;
   }
 
   if (opts::dump::DumpSymbols) {
-    auto EC = File.isPdb() ? dumpModuleSymsForPdb() : dumpModuleSymsForObj();
-    if (EC)
-      return EC;
+    ExitOnError Err("Unexpected error processing symbols: ");
+    Err(File.isPdb() ? dumpModuleSymsForPdb() : dumpModuleSymsForObj());
   }
 
   if (opts::dump::DumpTypeRefStats) {
     if (auto EC = dumpTypeRefStats())
       return EC;
   }
 
   if (opts::dump::DumpSectionHeaders) {
Error DumpOutputStyle::dumpFileSummary() {
   P.formatLine("GUID: {0}", fmt_guid(PS.getGuid().Guid));
   P.formatLine("Features: {0:x+}", static_cast<uint32_t>(PS.getFeatures()));
   P.formatLine("Has Debug Info: {0}", getPdb().hasPDBDbiStream());
   P.formatLine("Has Types: {0}", getPdb().hasPDBTpiStream());
   P.formatLine("Has IDs: {0}", getPdb().hasPDBIpiStream());
   P.formatLine("Has Globals: {0}", getPdb().hasPDBGlobalsStream());
   P.formatLine("Has Publics: {0}", getPdb().hasPDBPublicsStream());
   if (getPdb().hasPDBDbiStream()) {
-    auto &DBI = Err(getPdb().getPDBDbiStream());
+    DbiStream &DBI = Err(getPdb().getPDBDbiStream());
     P.formatLine("Is incrementally linked: {0}", DBI.isIncrementallyLinked());
     P.formatLine("Has conflicting types: {0}", DBI.hasCTypes());
     P.formatLine("Is stripped: {0}", DBI.isStripped());
   }
 
   return Error::success();
 }
 
for (uint32_t StreamIdx = 0; StreamIdx < StreamCount; ++StreamIdx) {
     }
   }
 
   return Error::success();
 }
 
 static Expected<ModuleDebugStreamRef> getModuleDebugStream(PDBFile &File,
                                                            uint32_t Index) {
-  ExitOnError Err("Unexpected error: ");
-
-  auto &Dbi = Err(File.getPDBDbiStream());
+  Expected<DbiStream &> DbiOrErr = File.getPDBDbiStream();
+  if (!DbiOrErr)
+    return DbiOrErr.takeError();
+  DbiStream &Dbi = *DbiOrErr;
   const auto &Modules = Dbi.modules();
   auto Modi = Modules.getModuleDescriptor(Index);
 
   uint16_t ModiStream = Modi.getModuleStreamIndex();
   if (ModiStream == kInvalidStreamIndex)
     return make_error<RawError>(raw_error_code::no_stream,
                                 "Module stream not present");
 
   auto ModStreamData = File.createIndexedStream(ModiStream);
 
   ModuleDebugStreamRef ModS(Modi, std::move(ModStreamData));
   if (auto EC = ModS.reload())
     return make_error<RawError>(raw_error_code::corrupt_file,
                                 "Invalid module stream");
 
   return std::move(ModS);
 }
 
 template <typename CallbackT>
-static void
+static Error
 iterateOneModule(InputFile &File, const Optional<PrintScope> &HeaderScope,
                  const SymbolGroup &SG, uint32_t Modi, CallbackT Callback) {
   if (HeaderScope) {
     HeaderScope->P.formatLine(
         "Mod {0:4} | `{1}`: ",
         fmt_align(Modi, AlignStyle::Right, HeaderScope->LabelWidth), SG.name());
   }
 
   AutoIndent Indent(HeaderScope);
-  Callback(Modi, SG);
+  return Callback(Modi, SG);
 }
 
 template <typename CallbackT>
-static void iterateSymbolGroups(InputFile &Input,
+static Error iterateSymbolGroups(InputFile &Input,
                                  const Optional<PrintScope> &HeaderScope,
                                  CallbackT Callback) {
   AutoIndent Indent(HeaderScope);
 
-  ExitOnError Err("Unexpected error processing modules: ");
-
   if (opts::dump::DumpModi.getNumOccurrences() > 0) {
     assert(opts::dump::DumpModi.getNumOccurrences() == 1);
     uint32_t Modi = opts::dump::DumpModi;
     SymbolGroup SG(&Input, Modi);
-    iterateOneModule(Input, withLabelWidth(HeaderScope, NumDigits(Modi)), SG,
-                     Modi, Callback);
-    return;
+    return iterateOneModule(Input, withLabelWidth(HeaderScope, NumDigits(Modi)),
+                            SG, Modi, Callback);
   }
 
   uint32_t I = 0;
 
   for (const auto &SG : Input.symbol_groups()) {
     if (shouldDumpSymbolGroup(I, SG))
-      iterateOneModule(Input, withLabelWidth(HeaderScope, NumDigits(I)), SG, I,
-                       Callback);
+      if (auto Err =
+              iterateOneModule(Input, withLabelWidth(HeaderScope, NumDigits(I)),
+                               SG, I, Callback))
+        return Err;
 
     ++I;
   }
+  return Error::success();
 }
 
 template <typename SubsectionT>
-static void iterateModuleSubsections(
+static Error iterateModuleSubsections(
     InputFile &File, const Optional<PrintScope> &HeaderScope,
-    llvm::function_ref<void(uint32_t, const SymbolGroup &, SubsectionT &)>
+    llvm::function_ref<Error(uint32_t, const SymbolGroup &, SubsectionT &)>
         Callback) {
 
-  iterateSymbolGroups(File, HeaderScope,
-                      [&](uint32_t Modi, const SymbolGroup &SG) {
-                        for (const auto &SS : SG.getDebugSubsections()) {
-                          SubsectionT Subsection;
+  return iterateSymbolGroups(
+      File, HeaderScope, [&](uint32_t Modi, const SymbolGroup &SG) -> Error {
 
-                          if (SS.kind() != Subsection.kind())
-                            continue;
 
-                          BinaryStreamReader Reader(SS.getRecordData());
-                          if (auto EC = Subsection.initialize(Reader))
-                            continue;
-                          Callback(Modi, SG, Subsection);
-                        }
-                      });
+          if (auto Err = Subsection.initialize(Reader))
+          if (auto Err = Callback(Modi, SG, Subsection))
+            return Err;
+        return Error::success();
 }
 
 static Expected<std::pair<std::unique_ptr<MappedBlockStream>,
                           ArrayRef<llvm::object::coff_section>>>
 loadSectionHeaders(PDBFile &File, DbgHeaderType Type) {
   if (!File.hasPDBDbiStream())
     return make_error<StringError>(
         "Section headers require a DBI Stream, which could not be loaded",
         inconvertibleErrorCode());
 
-  auto &Dbi = cantFail(File.getPDBDbiStream());
+  DbiStream &Dbi = cantFail(File.getPDBDbiStream());
   uint32_t SI = Dbi.getDebugStreamIndex(Type);
 
   if (SI == kInvalidStreamIndex)
     return make_error<StringError>(
         "PDB does not contain the requested image section header type",
         inconvertibleErrorCode());
 
   auto Stream = File.createIndexedStream(SI);
Error DumpOutputStyle::dumpModules() {
   }
 
   if (!getPdb().hasPDBDbiStream()) {
     printStreamNotPresent("DBI");
     return Error::success();
   }
 
   AutoIndent Indent(P);
-  ExitOnError Err("Unexpected error processing modules: ");
 
-  auto &Stream = Err(getPdb().getPDBDbiStream());
+  Expected<DbiStream &> StreamOrErr = getPdb().getPDBDbiStream();
+  if (!StreamOrErr)
+    return StreamOrErr.takeError();
+  DbiStream &Stream = *StreamOrErr;
 
   const DbiModuleList &Modules = Stream.modules();
-  iterateSymbolGroups(
-      File, PrintScope{P, 11}, [&](uint32_t Modi, const SymbolGroup &Strings) {
+  return iterateSymbolGroups(
+      File, PrintScope{P, 11},
+      [&](uint32_t Modi, const SymbolGroup &Strings) -> Error {
         auto Desc = Modules.getModuleDescriptor(Modi);
         if (opts::dump::DumpSectionContribs) {
           std::vector<std::string> Sections = getSectionNames(getPdb());
           dumpSectionContrib(P, Desc.getSectionContrib(), Sections, 0);
         }
         P.formatLine("Obj: `{0}`: ", Desc.getObjFileName());
         P.formatLine("debug stream: {0}, # files: {1}, has ec info: {2}",
                      Desc.getModuleStreamIndex(), Desc.getNumberOfFiles(),
                      Desc.hasECInfo());
-        StringRef PdbFilePath =
-            Err(Stream.getECName(Desc.getPdbFilePathNameIndex()));
-        StringRef SrcFilePath =
-            Err(Stream.getECName(Desc.getSourceFileNameIndex()));
+
+        auto PdbPathOrErr = Stream.getECName(Desc.getPdbFilePathNameIndex());
+        if (!PdbPathOrErr)
+          return PdbPathOrErr.takeError();
+        StringRef PdbFilePath = *PdbPathOrErr;
+
+        auto SrcPathOrErr = Stream.getECName(Desc.getSourceFileNameIndex());
+        if (!SrcPathOrErr)
+          return SrcPathOrErr.takeError();
+        StringRef SrcFilePath = *SrcPathOrErr;
+
         P.formatLine("pdb file ni: {0} `{1}`, src file ni: {2} `{3}`",
                      Desc.getPdbFilePathNameIndex(), PdbFilePath,
                      Desc.getSourceFileNameIndex(), SrcFilePath);
-      });
         return Error::success();
+      });
 }
 
 Error DumpOutputStyle::dumpModuleFiles() {
   printHeader(P, "Files");
 
   if (File.isObj()) {
     printStreamNotValidForObj();
     return Error::success();
   }
 
   if (!getPdb().hasPDBDbiStream()) {
     printStreamNotPresent("DBI");
     return Error::success();
   }
 
-  ExitOnError Err("Unexpected error processing modules: ");
-
-  iterateSymbolGroups(File, PrintScope{P, 11},
-                      [this, &Err](uint32_t Modi, const SymbolGroup &Strings) {
-                        auto &Stream = Err(getPdb().getPDBDbiStream());
+  return iterateSymbolGroups(
+      File, PrintScope{P, 11},
+      [this](uint32_t Modi, const SymbolGroup &Strings) -> Error {
+        Expected<DbiStream &> StreamOrErr = getPdb().getPDBDbiStream();
+        if (!StreamOrErr)
+          return StreamOrErr.takeError();
+        DbiStream &Stream = *StreamOrErr;
 
-                        const DbiModuleList &Modules = Stream.modules();
-                        for (const auto &F : Modules.source_files(Modi)) {
-                          Strings.formatFromFileName(P, F);
-                        }
-                      });
         return Error::success();
+      });
 }
 
 Error DumpOutputStyle::dumpSymbolStats() {
   printHeader(P, "Module Stats");
 
   if (File.isPdb() && !getPdb().hasPDBDbiStream()) {
     printStreamNotPresent("DBI");
     return Error::success();
   }
 
-  ExitOnError Err("Unexpected error processing modules: ");
-
   StatCollection SymStats;
   StatCollection ChunkStats;
 
   Optional<PrintScope> Scope;
   if (File.isPdb())
     Scope.emplace(P, 2);
 
-  iterateSymbolGroups(File, Scope, [&](uint32_t Modi, const SymbolGroup &SG) {
+  if (Error Err = iterateSymbolGroups(
+          File, Scope, [&](uint32_t Modi, const SymbolGroup &SG) -> Error {
             StatCollection SS = getSymbolStats(SG, SymStats);
             StatCollection CS = getChunkStats(SG, ChunkStats);
 
-    if (SG.getFile().isPdb()) {
+            if (!SG.getFile().isPdb())
+              return Error::success();
+
             AutoIndent Indent(P);
             auto Modules = cantFail(File.pdb().getPDBDbiStream()).modules();
             uint32_t ModCount = Modules.getModuleCount();
             DbiModuleDescriptor Desc = Modules.getModuleDescriptor(Modi);
             uint32_t StreamIdx = Desc.getModuleStreamIndex();
 
             if (StreamIdx == kInvalidStreamIndex) {
-        P.formatLine("Mod {0} (debug info not present): [{1}]",
-                     fmt_align(Modi, AlignStyle::Right, NumDigits(ModCount)),
-                     Desc.getModuleName());
-        return;
+              P.formatLine(
+                  "Mod {0} (debug info not present): [{1}]",
+              return Error::success();
             }
             P.formatLine("Stream {0}, {1} bytes", StreamIdx,
                          getPdb().getStreamByteSize(StreamIdx));
 
             printModuleDetailStats<SymbolKind>(P, "Symbols", SS);
             printModuleDetailStats<DebugSubsectionKind>(P, "Chunks", CS);
-    }
-  });
+
+            return Error::success();
+          }))
+    return Err;
 
   if (SymStats.Totals.Count > 0) {
     P.printLine("  Summary |");
     AutoIndent Indent(P, 4);
     printModuleDetailStats<SymbolKind>(P, "Symbols", SymStats);
     printModuleDetailStats<DebugSubsectionKind>(P, "Chunks", ChunkStats);
   }
 
Error DumpOutputStyle::dumpLines() {
 
   if (File.isPdb() && !getPdb().hasPDBDbiStream()) {
     printStreamNotPresent("DBI");
     return Error::success();
   }
 
   uint32_t LastModi = UINT32_MAX;
   uint32_t LastNameIndex = UINT32_MAX;
-  iterateModuleSubsections<DebugLinesSubsectionRef>(
+  return iterateModuleSubsections<DebugLinesSubsectionRef>(
       File, PrintScope{P, 4},
-      [this, &LastModi, &LastNameIndex](uint32_t Modi,
-                                        const SymbolGroup &Strings,
-                                        DebugLinesSubsectionRef &Lines) {
+      [this, &LastModi,
+       &LastNameIndex](uint32_t Modi, const SymbolGroup &Strings,
+                       DebugLinesSubsectionRef &Lines) -> Error {
         uint16_t Segment = Lines.header()->RelocSegment;
         uint32_t Begin = Lines.header()->RelocOffset;
         uint32_t End = Begin + Lines.header()->CodeSize;
         for (const auto &Block : Lines) {
           if (LastModi != Modi || LastNameIndex != Block.NameIndex) {
             LastModi = Modi;
             LastNameIndex = Block.NameIndex;
             Strings.formatFromChecksumsOffset(P, Block.NameIndex);
           }
 
           AutoIndent Indent(P, 2);
           P.formatLine("{0:X-4}:{1:X-8}-{2:X-8}, ", Segment, Begin, End);
           uint32_t Count = Block.LineNumbers.size();
           if (Lines.hasColumnInfo())
             P.format("line/column/addr entries = {0}", Count);
           else
             P.format("line/addr entries = {0}", Count);
 
           P.NewLine();
           typesetLinesAndColumns(P, Begin, Block);
         }
-      });
-
         return Error::success();
+      });
 }
 
 Error DumpOutputStyle::dumpInlineeLines() {
   printHeader(P, "Inlinee Lines");
 
   if (File.isPdb() && !getPdb().hasPDBDbiStream()) {
     printStreamNotPresent("DBI");
     return Error::success();
   }
 
-  iterateModuleSubsections<DebugInlineeLinesSubsectionRef>(
+  return iterateModuleSubsections<DebugInlineeLinesSubsectionRef>(
       File, PrintScope{P, 2},
       [this](uint32_t Modi, const SymbolGroup &Strings,
-             DebugInlineeLinesSubsectionRef &Lines) {
+             DebugInlineeLinesSubsectionRef &Lines) -> Error {
         P.formatLine("{0,+8} | {1,+5} | {2}", "Inlinee", "Line", "Source File");
         for (const auto &Entry : Lines) {
           P.formatLine("{0,+8} | {1,+5} | ", Entry.Header->Inlinee,
                        fmtle(Entry.Header->SourceLineNum));
           Strings.formatFromChecksumsOffset(P, Entry.Header->FileID, true);
           for (const auto &ExtraFileID : Entry.ExtraFiles) {
             P.formatLine("                   ");
             Strings.formatFromChecksumsOffset(P, ExtraFileID, true);
           }
         }
         P.NewLine();
-      });
-
         return Error::success();
+      });
 }
 
 Error DumpOutputStyle::dumpXmi() {
   printHeader(P, "Cross Module Imports");
 
   if (File.isPdb() && !getPdb().hasPDBDbiStream()) {
     printStreamNotPresent("DBI");
     return Error::success();
   }
 
-  iterateModuleSubsections<DebugCrossModuleImportsSubsectionRef>(
+  return iterateModuleSubsections<DebugCrossModuleImportsSubsectionRef>(
       File, PrintScope{P, 2},
       [this](uint32_t Modi, const SymbolGroup &Strings,
-             DebugCrossModuleImportsSubsectionRef &Imports) {
+             DebugCrossModuleImportsSubsectionRef &Imports) -> Error {
         P.formatLine("{0,=32} | {1}", "Imported Module", "Type IDs");
 
         for (const auto &Xmi : Imports) {
           auto ExpectedModule =
               Strings.getNameFromStringTable(Xmi.Header->ModuleNameOffset);
           StringRef Module;
           SmallString<32> ModuleStorage;
           if (!ExpectedModule) {
             Module = "(unknown module)";
             consumeError(ExpectedModule.takeError());
           } else
             Module = *ExpectedModule;
           if (Module.size() > 32) {
             ModuleStorage = "...";
             ModuleStorage += Module.take_back(32 - 3);
             Module = ModuleStorage;
           }
           std::vector<std::string> TIs;
           for (const auto I : Xmi.Imports)
             TIs.push_back(std::string(formatv("{0,+10:X+}", fmtle(I))));
           std::string Result =
               typesetItemList(TIs, P.getIndentLevel() + 35, 12, " ");
           P.formatLine("{0,+32} | {1}", Module, Result);
         }
-      });
-
         return Error::success();
+      });
 }
 
 Error DumpOutputStyle::dumpXme() {
   printHeader(P, "Cross Module Exports");
 
   if (File.isPdb() && !getPdb().hasPDBDbiStream()) {
     printStreamNotPresent("DBI");
     return Error::success();
   }
 
-  iterateModuleSubsections<DebugCrossModuleExportsSubsectionRef>(
+  return iterateModuleSubsections<DebugCrossModuleExportsSubsectionRef>(
       File, PrintScope{P, 2},
       [this](uint32_t Modi, const SymbolGroup &Strings,
-             DebugCrossModuleExportsSubsectionRef &Exports) {
+             DebugCrossModuleExportsSubsectionRef &Exports) -> Error {
         P.formatLine("{0,-10} | {1}", "Local ID", "Global ID");
         for (const auto &Export : Exports) {
           P.formatLine("{0,+10:X+} | {1}", TypeIndex(Export.Local),
                        TypeIndex(Export.Global));
         }
-      });
-
         return Error::success();
+      });
 }
 
 std::string formatFrameType(object::frame_type FT) {
   switch (FT) {
   case object::frame_type::Fpo:
     return "FPO";
   case object::frame_type::NonFpo:
     return "Non-FPO";
   case object::frame_type::Trap:
     return "Trap";
   case object::frame_type::Tss:
     return "TSS";
   }
   return "<unknown>";
 }
 
 Error DumpOutputStyle::dumpOldFpo(PDBFile &File) {
   printHeader(P, "Old FPO Data");
 
   ExitOnError Err("Error dumping old fpo data:");
-  auto &Dbi = Err(File.getPDBDbiStream());
+  DbiStream &Dbi = Err(File.getPDBDbiStream());
 
   if (!Dbi.hasOldFpoRecords()) {
     printStreamNotPresent("FPO");
     return Error::success();
   }
 
   const FixedStreamArray<object::FpoData>& Records = Dbi.getOldFpoRecords();
 
Error DumpOutputStyle::dumpOldFpo(PDBFile &File) {
   }
   return Error::success();
 }
 
 Error DumpOutputStyle::dumpNewFpo(PDBFile &File) {
   printHeader(P, "New FPO Data");
 
   ExitOnError Err("Error dumping new fpo data:");
-  auto &Dbi = Err(File.getPDBDbiStream());
+  DbiStream &Dbi = Err(File.getPDBDbiStream());
 
   if (!Dbi.hasNewFpoRecords()) {
     printStreamNotPresent("New FPO");
     return Error::success();
   }
 
   const DebugFrameDataSubsectionRef& FDS = Dbi.getNewFpoRecords();
 
P.NewLine();
                      uint32_t(Entry.value()));
       P.formatLine("Name Count: {0}", IS->getNameCount());
     }
   }
   return Error::success();
 }
 
 Error DumpOutputStyle::dumpStringTableFromObj() {
-  iterateModuleSubsections<DebugStringTableSubsectionRef>(
+  return iterateModuleSubsections<DebugStringTableSubsectionRef>(
       File, PrintScope{P, 4},
       [&](uint32_t Modi, const SymbolGroup &Strings,
-          DebugStringTableSubsectionRef &Strings2) {
+          DebugStringTableSubsectionRef &Strings2) -> Error {
         BinaryStreamRef StringTableBuffer = Strings2.getBuffer();
         BinaryStreamReader Reader(StringTableBuffer);
         while (Reader.bytesRemaining() > 0) {
           StringRef Str;
           uint32_t Offset = Reader.getOffset();
           cantFail(Reader.readCString(Str));
           if (Str.empty())
             continue;
 
           P.formatLine("{0} | {1}", fmt_align(Offset, AlignStyle::Right, 4),
                        Str);
         }
-      });
         return Error::success();
+      });
 }
 
 Error DumpOutputStyle::dumpNamedStreams() {
   printHeader(P, "Named Streams");
 
   if (File.isObj()) {
     printStreamNotValidForObj();
     return Error::success();
Error DumpOutputStyle::dumpTpiStream(uint32_t StreamIdx) {
   return Error::success();
 }
 
 Error DumpOutputStyle::dumpModuleSymsForObj() {
   printHeader(P, "Symbols");
 
   AutoIndent Indent(P);
 
-  ExitOnError Err("Unexpected error processing symbols: ");
-
   auto &Types = File.types();
 
   SymbolVisitorCallbackPipeline Pipeline;
   SymbolDeserializer Deserializer(nullptr, CodeViewContainer::ObjectFile);
   MinimalSymbolDumper Dumper(P, opts::dump::DumpSymRecordBytes, Types, Types);
 
   Pipeline.addCallbackToPipeline(Deserializer);
   Pipeline.addCallbackToPipeline(Dumper);
   CVSymbolVisitor Visitor(Pipeline);
 
-  std::unique_ptr<llvm::Error> SymbolError;
-
-  iterateModuleSubsections<DebugSymbolsSubsectionRef>(
+  return iterateModuleSubsections<DebugSymbolsSubsectionRef>(
       File, PrintScope{P, 2},
       [&](uint32_t Modi, const SymbolGroup &Strings,
-          DebugSymbolsSubsectionRef &Symbols) {
+          DebugSymbolsSubsectionRef &Symbols) -> Error {
         Dumper.setSymbolGroup(&Strings);
         for (auto Symbol : Symbols) {
           if (auto EC = Visitor.visitSymbolRecord(Symbol)) {
-            SymbolError = std::make_unique<Error>(std::move(EC));
-            return;
+            return EC;
           }
         }
-      });
-
-  if (SymbolError)
-    return std::move(*SymbolError);
-
         return Error::success();
+      });
 }
 
 Error DumpOutputStyle::dumpModuleSymsForPdb() {
   printHeader(P, "Symbols");
 
   if (File.isPdb() && !getPdb().hasPDBDbiStream()) {
     printStreamNotPresent("DBI");
     return Error::success();
   }
 
   AutoIndent Indent(P);
-  ExitOnError Err("Unexpected error processing symbols: ");
 
   auto &Ids = File.ids();
   auto &Types = File.types();
 
-  iterateSymbolGroups(
-      File, PrintScope{P, 2}, [&](uint32_t I, const SymbolGroup &Strings) {
+  return iterateSymbolGroups(
+      File, PrintScope{P, 2},
+      [&](uint32_t I, const SymbolGroup &Strings) -> Error {
         auto ExpectedModS = getModuleDebugStream(File.pdb(), I);
         if (!ExpectedModS) {
           P.formatLine("Error loading module stream {0}.  {1}", I,
                        toString(ExpectedModS.takeError()));
-          return;
+          return Error::success();
         }
 
         ModuleDebugStreamRef &ModS = *ExpectedModS;
 
         SymbolVisitorCallbackPipeline Pipeline;
         SymbolDeserializer Deserializer(nullptr, CodeViewContainer::Pdb);
         MinimalSymbolDumper Dumper(P, opts::dump::DumpSymRecordBytes, Strings,
                                    Ids, Types);
 
         Pipeline.addCallbackToPipeline(Deserializer);
         Pipeline.addCallbackToPipeline(Dumper);
         CVSymbolVisitor Visitor(Pipeline);
         auto SS = ModS.getSymbolsSubstream();
         if (auto EC =
                 Visitor.visitSymbolStream(ModS.getSymbolArray(), SS.Offset)) {
           P.formatLine("Error while processing symbol records.  {0}",
                        toString(std::move(EC)));
-          return;
+          return EC;
         }
-      });
         return Error::success();
+      });
 }
 
 Error DumpOutputStyle::dumpTypeRefStats() {
   printHeader(P, "Type Reference Statistics");
   AutoIndent Indent(P);
 
   // Sum the byte size of all type records, and the size and count of all
   // referenced records.
Error DumpOutputStyle::dumpSectionContribs() {
   if (!getPdb().hasPDBDbiStream()) {
     printStreamNotPresent("DBI");
     return Error::success();
   }
 
   AutoIndent Indent(P);
   ExitOnError Err("Error dumping section contributions: ");
 
-  auto &Dbi = Err(getPdb().getPDBDbiStream());
+  DbiStream &Dbi = Err(getPdb().getPDBDbiStream());
 
   class Visitor : public ISectionContribVisitor {
   public:
     Visitor(LinePrinter &P, ArrayRef<std::string> Names) : P(P), Names(Names) {
       auto Max = std::max_element(
           Names.begin(), Names.end(),
           [](StringRef S1, StringRef S2) { return S1.size() < S2.size(); });
       MaxNameLen = (Max == Names.end() ? 0 : Max->size());
Error DumpOutputStyle::dumpSectionMap() {
   if (!getPdb().hasPDBDbiStream()) {
     printStreamNotPresent("DBI");
     return Error::success();
   }
 
   AutoIndent Indent(P);
   ExitOnError Err("Error dumping section map: ");
 
-  auto &Dbi = Err(getPdb().getPDBDbiStream());
+  DbiStream &Dbi = Err(getPdb().getPDBDbiStream());
 
   uint32_t I = 0;
   for (auto &M : Dbi.getSectionMap()) {
     P.formatLine(
         "Section {0:4} | ovl = {1}, group = {2}, frame = {3}, name = {4}", I,
         fmtle(M.Ovl), fmtle(M.Group), fmtle(M.Frame), fmtle(M.SecName));
     P.formatLine("               class = {0}, offset = {1}, size = {2}",
                  fmtle(M.ClassName), fmtle(M.Offset), fmtle(M.SecByteLength));
     P.formatLine("               flags = {0}",
                  formatSegMapDescriptorFlag(
                      P.getIndentLevel() + 13,
                      static_cast<OMFSegDescFlags>(uint16_t(M.Flags))));
     ++I;
   }
   return Error::success();
 }

llvm/tools/llvm-pdbutil/InputFile.cpp

 using namespace llvm::object;
 using namespace llvm::pdb;
 
 InputFile::InputFile() {}
 InputFile::~InputFile() {}
 
 static Expected<ModuleDebugStreamRef>
 getModuleDebugStream(PDBFile &File, StringRef &ModuleName, uint32_t Index) {
-  ExitOnError Err("Unexpected error: ");
-
-  auto &Dbi = Err(File.getPDBDbiStream());
+  Expected<DbiStream &> DbiOrErr = File.getPDBDbiStream();
+  if (!DbiOrErr)
+    return DbiOrErr.takeError();
+  DbiStream &Dbi = *DbiOrErr;
   const auto &Modules = Dbi.modules();
   if (Index >= Modules.getModuleCount())
     return make_error<RawError>(raw_error_code::index_out_of_bounds,
                                 "Invalid module index");
 
   auto Modi = Modules.getModuleDescriptor(Index);
 
   ModuleName = Modi.getModuleName();
while (++Iter != End) {
     return;
   }
 }
 
 bool SymbolGroupIterator::isEnd() const {
   if (!Value.File)
     return true;
   if (Value.File->isPdb()) {
-    auto &Dbi = cantFail(Value.File->pdb().getPDBDbiStream());
+    DbiStream &Dbi = cantFail(Value.File->pdb().getPDBDbiStream());
     uint32_t Count = Dbi.modules().getModuleCount();
     assert(Index <= Count);
     return Index == Count;
   }
 
   assert(SectionIter.hasValue());
   return *SectionIter == Value.File->obj().section_end();
 }