diff --git a/llvm/include/llvm/DebugInfo/Symbolize/MarkupFilter.h b/llvm/include/llvm/DebugInfo/Symbolize/MarkupFilter.h --- a/llvm/include/llvm/DebugInfo/Symbolize/MarkupFilter.h +++ b/llvm/include/llvm/DebugInfo/Symbolize/MarkupFilter.h @@ -15,13 +15,12 @@ #ifndef LLVM_DEBUGINFO_SYMBOLIZE_MARKUPFILTER_H #define LLVM_DEBUGINFO_SYMBOLIZE_MARKUPFILTER_H -#include "Markup.h" - -#include - #include "llvm/ADT/DenseMap.h" +#include "llvm/DebugInfo/Symbolize/Markup.h" +#include "llvm/Object/BuildID.h" #include "llvm/Support/WithColor.h" #include "llvm/Support/raw_ostream.h" +#include namespace llvm { namespace symbolize { @@ -116,7 +115,7 @@ std::optional parseAddr(StringRef Str) const; std::optional parseModuleID(StringRef Str) const; std::optional parseSize(StringRef Str) const; - std::optional> parseBuildID(StringRef Str) const; + object::BuildID parseBuildID(StringRef Str) const; std::optional parseMode(StringRef Str) const; std::optional parsePCType(StringRef Str) const; std::optional parseFrameNumber(StringRef Str) const; diff --git a/llvm/include/llvm/Object/BuildID.h b/llvm/include/llvm/Object/BuildID.h --- a/llvm/include/llvm/Object/BuildID.h +++ b/llvm/include/llvm/Object/BuildID.h @@ -29,8 +29,11 @@ class ObjectFile; +/// Parses a build ID from a hex string. +BuildID parseBuildID(StringRef Str); + /// Returns the build ID, if any, contained in the given object file. -std::optional getBuildID(const ObjectFile *Obj); +BuildIDRef getBuildID(const ObjectFile *Obj); /// BuildIDFetcher searches local cache directories for debug info. class BuildIDFetcher { diff --git a/llvm/lib/DebugInfo/Symbolize/MarkupFilter.cpp b/llvm/lib/DebugInfo/Symbolize/MarkupFilter.cpp --- a/llvm/lib/DebugInfo/Symbolize/MarkupFilter.cpp +++ b/llvm/lib/DebugInfo/Symbolize/MarkupFilter.cpp @@ -513,8 +513,9 @@ } if (!checkNumFields(Element, 4)) return std::nullopt; - ASSIGN_OR_RETURN_NONE(SmallVector, BuildID, - parseBuildID(Element.Fields[3])); + SmallVector BuildID = parseBuildID(Element.Fields[3]); + if (BuildID.empty()) + return std::nullopt; return Module{ID, Name.str(), std::move(BuildID)}; } @@ -597,16 +598,11 @@ } // Parse a build ID (%x in the spec). -std::optional> -MarkupFilter::parseBuildID(StringRef Str) const { - std::string Bytes; - if (Str.empty() || Str.size() % 2 || !tryGetFromHex(Str, Bytes)) { +object::BuildID MarkupFilter::parseBuildID(StringRef Str) const { + object::BuildID BID = llvm::object::parseBuildID(Str); + if (BID.empty()) reportTypeError(Str, "build ID"); - return std::nullopt; - } - ArrayRef BuildID(reinterpret_cast(Bytes.data()), - Bytes.size()); - return SmallVector(BuildID.begin(), BuildID.end()); + return BID; } // Parses the mode string for an mmap element. diff --git a/llvm/lib/DebugInfo/Symbolize/Symbolize.cpp b/llvm/lib/DebugInfo/Symbolize/Symbolize.cpp --- a/llvm/lib/DebugInfo/Symbolize/Symbolize.cpp +++ b/llvm/lib/DebugInfo/Symbolize/Symbolize.cpp @@ -363,12 +363,10 @@ const ELFObjectFileBase *Obj, const std::string &ArchName) { auto BuildID = getBuildID(Obj); - if (!BuildID) - return nullptr; - if (BuildID->size() < 2) + if (BuildID.size() < 2) return nullptr; std::string DebugBinaryPath; - if (!getOrFindDebugBinary(*BuildID, DebugBinaryPath)) + if (!getOrFindDebugBinary(BuildID, DebugBinaryPath)) return nullptr; auto DbgObjOrErr = getOrCreateObject(DebugBinaryPath, ArchName); if (!DbgObjOrErr) { diff --git a/llvm/lib/Debuginfod/Debuginfod.cpp b/llvm/lib/Debuginfod/Debuginfod.cpp --- a/llvm/lib/Debuginfod/Debuginfod.cpp +++ b/llvm/lib/Debuginfod/Debuginfod.cpp @@ -412,11 +412,11 @@ if (!Object) continue; - std::optional ID = getBuildID(Object); - if (!ID) + BuildIDRef ID = getBuildID(Object); + if (ID.empty()) continue; - std::string IDString = buildIDToString(*ID); + std::string IDString = buildIDToString(ID); if (Object->hasDebugInfo()) { std::lock_guard DebugBinariesGuard(DebugBinariesMutex); (void)DebugBinaries.try_emplace(IDString, std::move(FilePath)); diff --git a/llvm/lib/Object/BuildID.cpp b/llvm/lib/Object/BuildID.cpp --- a/llvm/lib/Object/BuildID.cpp +++ b/llvm/lib/Object/BuildID.cpp @@ -18,13 +18,12 @@ #include "llvm/Support/FileSystem.h" #include "llvm/Support/Path.h" -namespace llvm { -namespace object { +using namespace llvm; +using namespace llvm::object; namespace { -template -std::optional getBuildID(const ELFFile &Obj) { +template BuildIDRef getBuildID(const ELFFile &Obj) { auto PhdrsOrErr = Obj.program_headers(); if (!PhdrsOrErr) { consumeError(PhdrsOrErr.takeError()); @@ -45,15 +44,24 @@ } // namespace -std::optional getBuildID(const ObjectFile *Obj) { +BuildID llvm::object::parseBuildID(StringRef Str) { + std::string Bytes; + if (!tryGetFromHex(Str, Bytes)) + return {}; + ArrayRef BuildID(reinterpret_cast(Bytes.data()), + Bytes.size()); + return SmallVector(BuildID.begin(), BuildID.end()); +} + +BuildIDRef llvm::object::getBuildID(const ObjectFile *Obj) { if (auto *O = dyn_cast>(Obj)) - return getBuildID(O->getELFFile()); + return ::getBuildID(O->getELFFile()); if (auto *O = dyn_cast>(Obj)) - return getBuildID(O->getELFFile()); + return ::getBuildID(O->getELFFile()); if (auto *O = dyn_cast>(Obj)) - return getBuildID(O->getELFFile()); + return ::getBuildID(O->getELFFile()); if (auto *O = dyn_cast>(Obj)) - return getBuildID(O->getELFFile()); + return ::getBuildID(O->getELFFile()); return std::nullopt; } @@ -88,6 +96,3 @@ } return std::nullopt; } - -} // namespace object -} // namespace llvm diff --git a/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp b/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp --- a/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp +++ b/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp @@ -955,7 +955,7 @@ static Expected> loadBinaryFormat(std::unique_ptr Bin, StringRef Arch, StringRef CompilationDir = "", - std::optional *BinaryID = nullptr) { + object::BuildIDRef *BinaryID = nullptr) { std::unique_ptr OF; if (auto *Universal = dyn_cast(Bin.get())) { // If we have a universal binary, try to look up the object for the @@ -1151,14 +1151,14 @@ return std::move(Readers); } - std::optional BinaryID; + object::BuildIDRef BinaryID; auto ReaderOrErr = loadBinaryFormat(std::move(Bin), Arch, CompilationDir, BinaryIDs ? &BinaryID : nullptr); if (!ReaderOrErr) return ReaderOrErr.takeError(); Readers.push_back(std::move(ReaderOrErr.get())); - if (BinaryID) - BinaryIDs->push_back(*BinaryID); + if (!BinaryID.empty()) + BinaryIDs->push_back(BinaryID); return std::move(Readers); } diff --git a/llvm/lib/ProfileData/RawMemProfReader.cpp b/llvm/lib/ProfileData/RawMemProfReader.cpp --- a/llvm/lib/ProfileData/RawMemProfReader.cpp +++ b/llvm/lib/ProfileData/RawMemProfReader.cpp @@ -337,12 +337,11 @@ Error RawMemProfReader::setupForSymbolization() { auto *Object = cast(Binary.getBinary()); - auto BuildIdOr = object::getBuildID(Object); - if (!BuildIdOr.has_value()) + object::BuildIDRef BinaryId = object::getBuildID(Object); + if (BinaryId.empty()) return make_error(Twine("No build id found in binary ") + Binary.getBinary()->getFileName(), inconvertibleErrorCode()); - llvm::ArrayRef BinaryId = BuildIdOr.value(); int NumMatched = 0; for (const auto &Entry : SegmentInfo) { diff --git a/llvm/test/DebugInfo/symbolize-filter-markup-parse-fields.test b/llvm/test/DebugInfo/symbolize-filter-markup-parse-fields.test --- a/llvm/test/DebugInfo/symbolize-filter-markup-parse-fields.test +++ b/llvm/test/DebugInfo/symbolize-filter-markup-parse-fields.test @@ -2,25 +2,25 @@ RUN: llvm-symbolizer --filter-markup < %t/log 2> %t.err RUN: FileCheck %s -input-file=%t.err --match-full-lines -CHECK-NOT: '0x4f' -CHECK-NOT: '00' -CHECK: error: expected address; found '' -CHECK: error: expected address; found '42' -CHECK: error: expected address; found '0xgg' +CHECK-NOT: error: expected address; found '0x4f' +CHECK-NOT: error: expected address; found '00' +CHECK: error: expected address; found '' +CHECK: error: expected address; found '42' +CHECK: error: expected address; found '0xgg' -CHECK-NOT: '0' -CHECK: error: expected module ID; found '' -CHECK: error: expected module ID; found '-1' -CHECK-NOT: '077' -CHECK: error: expected module ID; found '079' -CHECK-NOT: '0xff' -CHECK: error: expected module ID; found '0xfg' -CHECK: error: expected module ID; found '0x' +CHECK-NOT: error: expected module ID; found '0' +CHECK: error: expected module ID; found '' +CHECK: error: expected module ID; found '-1' +CHECK-NOT: error: expected module ID; found '077' +CHECK: error: expected module ID; found '079' +CHECK-NOT: error: expected module ID; found '0xff' +CHECK: error: expected module ID; found '0xfg' +CHECK: error: expected module ID; found '0x' -CHECK: error: expected build ID; found '' -CHECK: error: expected build ID; found '0' -CHECK-NOT: '0xff' -CHECK: error: expected build ID; found 'fg' +CHECK: error: expected build ID; found '' +CHECK-NOT: error: expected build ID; found '0' +CHECK-NOT: error: expected build ID; found '0xff' +CHECK: error: expected build ID; found 'fg' ;--- log {{{mmap:0x4f:1:unknown}}} diff --git a/llvm/tools/llvm-objdump/llvm-objdump.cpp b/llvm/tools/llvm-objdump/llvm-objdump.cpp --- a/llvm/tools/llvm-objdump/llvm-objdump.cpp +++ b/llvm/tools/llvm-objdump/llvm-objdump.cpp @@ -1285,10 +1285,10 @@ // Build ID. Returns std::nullopt if nothing was found. static std::optional> fetchBinaryByBuildID(const ObjectFile &Obj) { - std::optional BuildID = getBuildID(&Obj); - if (!BuildID) + object::BuildIDRef BuildID = getBuildID(&Obj); + if (BuildID.empty()) return std::nullopt; - std::optional Path = BIDFetcher->fetch(*BuildID); + std::optional Path = BIDFetcher->fetch(BuildID); if (!Path) return std::nullopt; Expected> DebugBinary = createBinary(*Path); @@ -2943,13 +2943,11 @@ static object::BuildID parseBuildIDArg(const opt::Arg *A) { StringRef V(A->getValue()); - std::string Bytes; - if (!tryGetFromHex(V, Bytes)) + object::BuildID BID = parseBuildID(V); + if (BID.empty()) reportCmdLineError(A->getSpelling() + ": expected a build ID, but got '" + V + "'"); - ArrayRef BuildID(reinterpret_cast(Bytes.data()), - Bytes.size()); - return object::BuildID(BuildID.begin(), BuildID.end()); + return BID; } void objdump::invalidArgValue(const opt::Arg *A) { diff --git a/llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp b/llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp --- a/llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp +++ b/llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp @@ -125,15 +125,6 @@ HTTPClient::initialize(); } -static object::BuildID parseBuildID(StringRef Str) { - std::string Bytes; - if (!tryGetFromHex(Str, Bytes)) - return {}; - ArrayRef BuildID(reinterpret_cast(Bytes.data()), - Bytes.size()); - return object::BuildID(BuildID.begin(), BuildID.end()); -} - static bool parseCommand(StringRef BinaryName, bool IsAddr2Line, StringRef InputString, Command &Cmd, std::string &ModuleName, object::BuildID &BuildID,