[llvm] [llvm-cov] - Output better error message when the error kind is `coveragemap_error::malforme`. (PR #65264)
via llvm-commits
llvm-commits at lists.llvm.org
Mon Sep 4 07:17:26 PDT 2023
https://github.com/MaggieYingYi created https://github.com/llvm/llvm-project/pull/65264:
The current llvm-cov error message for kind `coveragemap_error::malforme`, just gives the issue kind without any reason for what caused the issue. This patch is aimed at improving the llvm-cov error message to help identify what caused the issue.
>From 0f36e2d264ff0da72f72b7aae3e98fa27d14a340 Mon Sep 17 00:00:00 2001
From: Ying Yi <maggieyi666 at gmail.com>
Date: Fri, 1 Sep 2023 14:18:46 +0100
Subject: [PATCH] - Output better error message when the error kind is
`coveragemap_error::malforme`.
The current llvm-cov error message for kind `coveragemap_error::malforme`, just gives the issue kind without any reason for what caused the issue. This patch is aimed at improving the llvm-cov error message to help identify what caused the issue.
---
.../ProfileData/Coverage/CoverageMapping.h | 8 +-
.../ProfileData/Coverage/CoverageMapping.cpp | 46 +++++---
.../Coverage/CoverageMappingReader.cpp | 107 +++++++++++++-----
.../ProfileData/CoverageMappingTest.cpp | 10 +-
4 files changed, 122 insertions(+), 49 deletions(-)
diff --git a/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h b/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
index 58f4fbd4953ba8b..b407fe277c543b8 100644
--- a/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
+++ b/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
@@ -75,7 +75,8 @@ inline std::error_code make_error_code(coveragemap_error E) {
class CoverageMapError : public ErrorInfo<CoverageMapError> {
public:
- CoverageMapError(coveragemap_error Err) : Err(Err) {
+ CoverageMapError(coveragemap_error Err, const Twine &ErrStr = Twine())
+ : Err(Err), Msg(ErrStr.str()) {
assert(Err != coveragemap_error::success && "Not an error");
}
@@ -88,11 +89,13 @@ class CoverageMapError : public ErrorInfo<CoverageMapError> {
}
coveragemap_error get() const { return Err; }
+ const std::string &getMessage() const { return Msg; }
static char ID;
private:
coveragemap_error Err;
+ std::string Msg;
};
/// A Counter is an abstract value that describes how to compute the
@@ -864,7 +867,8 @@ struct CovMapFunctionRecordV1 {
uint32_t NameS = support::endian::byte_swap<uint32_t, Endian>(NameSize);
FuncName = ProfileNames.getFuncName(NameRef, NameS);
if (NameS && FuncName.empty())
- return make_error<CoverageMapError>(coveragemap_error::malformed);
+ return make_error<CoverageMapError>(coveragemap_error::malformed,
+ "function name is empty");
return Error::success();
}
diff --git a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
index 849ee80bfaa3326..bd5af8237a826ec 100644
--- a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
+++ b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
@@ -237,7 +237,8 @@ Error CoverageMapping::loadFunctionRecord(
IndexedInstrProfReader &ProfileReader) {
StringRef OrigFuncName = Record.FunctionName;
if (OrigFuncName.empty())
- return make_error<CoverageMapError>(coveragemap_error::malformed);
+ return make_error<CoverageMapError>(coveragemap_error::malformed,
+ "record function name is empty");
if (Record.Filenames.empty())
OrigFuncName = getFuncNameWithoutPrefix(OrigFuncName);
@@ -342,7 +343,7 @@ static Error handleMaybeNoDataFoundError(Error E) {
std::move(E), [](const CoverageMapError &CME) {
if (CME.get() == coveragemap_error::no_data_found)
return static_cast<Error>(Error::success());
- return make_error<CoverageMapError>(CME.get());
+ return make_error<CoverageMapError>(CME.get(), CME.getMessage());
});
}
@@ -925,26 +926,45 @@ LineCoverageIterator &LineCoverageIterator::operator++() {
return *this;
}
-static std::string getCoverageMapErrString(coveragemap_error Err) {
+static std::string getCoverageMapErrString(coveragemap_error Err,
+ const std::string &ErrMsg = "") {
+ std::string Msg;
+ raw_string_ostream OS(Msg);
+
switch (Err) {
case coveragemap_error::success:
- return "Success";
+ OS << "Success";
+ break;
case coveragemap_error::eof:
- return "End of File";
+ OS << "End of File";
+ break;
case coveragemap_error::no_data_found:
- return "No coverage data found";
+ OS << "No coverage data found";
+ break;
case coveragemap_error::unsupported_version:
- return "Unsupported coverage format version";
+ OS << "Unsupported coverage format version";
+ break;
case coveragemap_error::truncated:
- return "Truncated coverage data";
+ OS << "Truncated coverage data";
+ break;
case coveragemap_error::malformed:
- return "Malformed coverage data";
+ OS << "Malformed coverage data";
+ break;
case coveragemap_error::decompression_failed:
- return "Failed to decompress coverage data (zlib)";
+ OS << "Failed to decompress coverage data (zlib)";
+ break;
case coveragemap_error::invalid_or_missing_arch_specifier:
- return "`-arch` specifier is invalid or missing for universal binary";
+ OS << "`-arch` specifier is invalid or missing for universal binary";
+ break;
+ default:
+ llvm_unreachable("Invalid coverage mapping error.");
}
- llvm_unreachable("A value of coveragemap_error has no message.");
+
+ // If optional error message is not empty, append it to the message.
+ if (!ErrMsg.empty())
+ OS << ": " << ErrMsg;
+
+ return Msg;
}
namespace {
@@ -962,7 +982,7 @@ class CoverageMappingErrorCategoryType : public std::error_category {
} // end anonymous namespace
std::string CoverageMapError::message() const {
- return getCoverageMapErrString(Err);
+ return getCoverageMapErrString(Err, Msg);
}
const std::error_category &llvm::coverage::coveragemap_category() {
diff --git a/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp b/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
index bee9c8d3fce7451..8480c892a3d6da5 100644
--- a/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
+++ b/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
@@ -68,7 +68,8 @@ Error RawCoverageReader::readULEB128(uint64_t &Result) {
unsigned N = 0;
Result = decodeULEB128(Data.bytes_begin(), &N);
if (N > Data.size())
- return make_error<CoverageMapError>(coveragemap_error::malformed);
+ return make_error<CoverageMapError>(coveragemap_error::malformed,
+ "the size of ULEB128 is too big");
Data = Data.substr(N);
return Error::success();
}
@@ -77,7 +78,9 @@ Error RawCoverageReader::readIntMax(uint64_t &Result, uint64_t MaxPlus1) {
if (auto Err = readULEB128(Result))
return Err;
if (Result >= MaxPlus1)
- return make_error<CoverageMapError>(coveragemap_error::malformed);
+ return make_error<CoverageMapError>(
+ coveragemap_error::malformed,
+ "the value of ULEB128 is greater than or equal to MaxPlus1");
return Error::success();
}
@@ -85,7 +88,8 @@ Error RawCoverageReader::readSize(uint64_t &Result) {
if (auto Err = readULEB128(Result))
return Err;
if (Result > Data.size())
- return make_error<CoverageMapError>(coveragemap_error::malformed);
+ return make_error<CoverageMapError>(coveragemap_error::malformed,
+ "the value of ULEB128 is too big");
return Error::success();
}
@@ -103,7 +107,8 @@ Error RawCoverageFilenamesReader::read(CovMapVersion Version) {
if (auto Err = readSize(NumFilenames))
return Err;
if (!NumFilenames)
- return make_error<CoverageMapError>(coveragemap_error::malformed);
+ return make_error<CoverageMapError>(coveragemap_error::malformed,
+ "number of filenames is zero");
if (Version < CovMapVersion::Version4)
return readUncompressed(Version, NumFilenames);
@@ -201,13 +206,15 @@ Error RawCoverageMappingReader::decodeCounter(unsigned Value, Counter &C) {
case CounterExpression::Add: {
auto ID = Value >> Counter::EncodingTagBits;
if (ID >= Expressions.size())
- return make_error<CoverageMapError>(coveragemap_error::malformed);
+ return make_error<CoverageMapError>(coveragemap_error::malformed,
+ "counter expression is invalid");
Expressions[ID].Kind = CounterExpression::ExprKind(Tag);
C = Counter::getExpression(ID);
break;
}
default:
- return make_error<CoverageMapError>(coveragemap_error::malformed);
+ return make_error<CoverageMapError>(coveragemap_error::malformed,
+ "counter expression kind is invalid");
}
return Error::success();
}
@@ -268,7 +275,8 @@ Error RawCoverageMappingReader::readMappingRegionsSubArray(
ExpandedFileID = EncodedCounterAndRegion >>
Counter::EncodingCounterTagAndExpansionRegionTagBits;
if (ExpandedFileID >= NumFileIDs)
- return make_error<CoverageMapError>(coveragemap_error::malformed);
+ return make_error<CoverageMapError>(coveragemap_error::malformed,
+ "ExpandedFileID is invalid");
} else {
switch (EncodedCounterAndRegion >>
Counter::EncodingCounterTagAndExpansionRegionTagBits) {
@@ -287,7 +295,8 @@ Error RawCoverageMappingReader::readMappingRegionsSubArray(
return Err;
break;
default:
- return make_error<CoverageMapError>(coveragemap_error::malformed);
+ return make_error<CoverageMapError>(coveragemap_error::malformed,
+ "region kind is incorrect");
}
}
}
@@ -300,7 +309,8 @@ Error RawCoverageMappingReader::readMappingRegionsSubArray(
if (auto Err = readULEB128(ColumnStart))
return Err;
if (ColumnStart > std::numeric_limits<unsigned>::max())
- return make_error<CoverageMapError>(coveragemap_error::malformed);
+ return make_error<CoverageMapError>(coveragemap_error::malformed,
+ "start column is too big");
if (auto Err = readIntMax(NumLines, std::numeric_limits<unsigned>::max()))
return Err;
if (auto Err = readIntMax(ColumnEnd, std::numeric_limits<unsigned>::max()))
@@ -341,7 +351,9 @@ Error RawCoverageMappingReader::readMappingRegionsSubArray(
LineStart, ColumnStart,
LineStart + NumLines, ColumnEnd, Kind);
if (CMR.startLoc() > CMR.endLoc())
- return make_error<CoverageMapError>(coveragemap_error::malformed);
+ return make_error<CoverageMapError>(
+ coveragemap_error::malformed,
+ "counter mapping region locations are incorrect");
MappingRegions.push_back(CMR);
}
return Error::success();
@@ -613,7 +625,9 @@ class VersionedCovMapFuncRecordReader : public CovMapFuncRecordReader {
using namespace support;
if (CovBuf + sizeof(CovMapHeader) > CovBufEnd)
- return make_error<CoverageMapError>(coveragemap_error::malformed);
+ return make_error<CoverageMapError>(
+ coveragemap_error::malformed,
+ "coverage mapping header section is greater than buffer size");
auto CovHeader = reinterpret_cast<const CovMapHeader *>(CovBuf);
uint32_t NRecords = CovHeader->getNRecords<Endian>();
uint32_t FilenamesSize = CovHeader->getFilenamesSize<Endian>();
@@ -634,7 +648,9 @@ class VersionedCovMapFuncRecordReader : public CovMapFuncRecordReader {
// Get the filenames.
if (CovBuf + FilenamesSize > CovBufEnd)
- return make_error<CoverageMapError>(coveragemap_error::malformed);
+ return make_error<CoverageMapError>(
+ coveragemap_error::malformed,
+ "filenames section is greater than buffer size");
size_t FilenamesBegin = Filenames.size();
StringRef FilenameRegion(CovBuf, FilenamesSize);
RawCoverageFilenamesReader Reader(FilenameRegion, Filenames,
@@ -673,12 +689,15 @@ class VersionedCovMapFuncRecordReader : public CovMapFuncRecordReader {
// coverage header).
const char *MappingBuf = CovBuf;
if (Version >= CovMapVersion::Version4 && CoverageSize != 0)
- return make_error<CoverageMapError>(coveragemap_error::malformed);
+ return make_error<CoverageMapError>(coveragemap_error::malformed,
+ "coverage mapping size is not zero");
CovBuf += CoverageSize;
const char *MappingEnd = CovBuf;
if (CovBuf > CovBufEnd)
- return make_error<CoverageMapError>(coveragemap_error::malformed);
+ return make_error<CoverageMapError>(
+ coveragemap_error::malformed,
+ "function records section is greater than buffer size");
if (Version < CovMapVersion::Version4) {
// Read each function record.
@@ -707,7 +726,9 @@ class VersionedCovMapFuncRecordReader : public CovMapFuncRecordReader {
CFR->template advanceByOne<Endian>(OutOfLineMappingBuf);
if (Version < CovMapVersion::Version4)
if (NextMappingBuf > OutOfLineMappingBufEnd)
- return make_error<CoverageMapError>(coveragemap_error::malformed);
+ return make_error<CoverageMapError>(
+ coveragemap_error::malformed,
+ "next mapping buffer is greater than buffer size");
// Look up the set of filenames associated with this function record.
std::optional<FilenameRange> FileRange;
@@ -717,7 +738,10 @@ class VersionedCovMapFuncRecordReader : public CovMapFuncRecordReader {
uint64_t FilenamesRef = CFR->template getFilenamesRef<Endian>();
auto It = FileRangeMap.find(FilenamesRef);
if (It == FileRangeMap.end())
- return make_error<CoverageMapError>(coveragemap_error::malformed);
+ return make_error<CoverageMapError>(
+ coveragemap_error::malformed,
+ "no filename found for function with hash=0x" +
+ Twine::utohexstr(FilenamesRef));
else
FileRange = It->getSecond();
}
@@ -728,7 +752,9 @@ class VersionedCovMapFuncRecordReader : public CovMapFuncRecordReader {
CFR->template getCoverageMapping<Endian>(OutOfLineMappingBuf);
if (Version >= CovMapVersion::Version4 &&
Mapping.data() + Mapping.size() > FuncRecBufEnd)
- return make_error<CoverageMapError>(coveragemap_error::malformed);
+ return make_error<CoverageMapError>(
+ coveragemap_error::malformed,
+ "coverage mapping data is greater than buffer size");
if (Error Err = insertFunctionRecordIfNeeded(CFR, Mapping, *FileRange))
return Err;
}
@@ -854,7 +880,9 @@ BinaryCoverageReader::createCoverageReaderFromBuffer(
Reader->MappingRecords, CompilationDir, Reader->Filenames))
return std::move(E);
} else
- return make_error<CoverageMapError>(coveragemap_error::malformed);
+ return make_error<CoverageMapError>(
+ coveragemap_error::malformed,
+ "not supported endianness or bytes in address");
return std::move(Reader);
}
@@ -866,7 +894,8 @@ loadTestingFormat(StringRef Data, StringRef CompilationDir) {
// Read the magic and version.
Data = Data.substr(sizeof(TestingFormatMagic));
if (Data.size() < sizeof(uint64_t))
- return make_error<CoverageMapError>(coveragemap_error::malformed);
+ return make_error<CoverageMapError>(coveragemap_error::malformed,
+ "the size of data is too small");
auto TestingVersion =
support::endian::byte_swap<uint64_t, support::endianness::little>(
*reinterpret_cast<const uint64_t *>(Data.data()));
@@ -878,17 +907,21 @@ loadTestingFormat(StringRef Data, StringRef CompilationDir) {
unsigned N = 0;
uint64_t ProfileNamesSize = decodeULEB128(Data.bytes_begin(), &N);
if (N > Data.size())
- return make_error<CoverageMapError>(coveragemap_error::malformed);
+ return make_error<CoverageMapError>(
+ coveragemap_error::malformed,
+ "the size of TestingFormatMagic is too big");
Data = Data.substr(N);
if (Data.empty())
return make_error<CoverageMapError>(coveragemap_error::truncated);
N = 0;
uint64_t Address = decodeULEB128(Data.bytes_begin(), &N);
if (N > Data.size())
- return make_error<CoverageMapError>(coveragemap_error::malformed);
+ return make_error<CoverageMapError>(coveragemap_error::malformed,
+ "the size of ULEB128 is too big");
Data = Data.substr(N);
if (Data.size() < ProfileNamesSize)
- return make_error<CoverageMapError>(coveragemap_error::malformed);
+ return make_error<CoverageMapError>(coveragemap_error::malformed,
+ "the size of ProfileNames is too big");
InstrProfSymtab ProfileNames;
if (Error E = ProfileNames.create(Data.substr(0, ProfileNamesSize), Address))
return std::move(E);
@@ -900,10 +933,13 @@ loadTestingFormat(StringRef Data, StringRef CompilationDir) {
N = 0;
CoverageMappingSize = decodeULEB128(Data.bytes_begin(), &N);
if (N > Data.size())
- return make_error<CoverageMapError>(coveragemap_error::malformed);
+ return make_error<CoverageMapError>(coveragemap_error::malformed,
+ "the size of ULEB128 is too big");
Data = Data.substr(N);
if (CoverageMappingSize < sizeof(CovMapHeader))
- return make_error<CoverageMapError>(coveragemap_error::malformed);
+ return make_error<CoverageMapError>(
+ coveragemap_error::malformed,
+ "the size of CoverageMapping is teoo small");
} else if (TestingVersion != uint64_t(TestingFormatVersion::Version1)) {
return make_error<CoverageMapError>(coveragemap_error::unsupported_version);
}
@@ -911,10 +947,13 @@ loadTestingFormat(StringRef Data, StringRef CompilationDir) {
// Skip the padding bytes because coverage map data has an alignment of 8.
auto Pad = offsetToAlignedAddr(Data.data(), Align(8));
if (Data.size() < Pad)
- return make_error<CoverageMapError>(coveragemap_error::malformed);
+ return make_error<CoverageMapError>(coveragemap_error::malformed,
+ "insufficient padding");
Data = Data.substr(Pad);
if (Data.size() < sizeof(CovMapHeader))
- return make_error<CoverageMapError>(coveragemap_error::malformed);
+ return make_error<CoverageMapError>(
+ coveragemap_error::malformed,
+ "coverage mapping header section is greater than data size");
auto const *CovHeader = reinterpret_cast<const CovMapHeader *>(
Data.substr(0, sizeof(CovMapHeader)).data());
auto Version =
@@ -937,13 +976,15 @@ loadTestingFormat(StringRef Data, StringRef CompilationDir) {
// Read the CoverageRecords data.
if (Version < CovMapVersion::Version4) {
if (!Data.empty())
- return make_error<CoverageMapError>(coveragemap_error::malformed);
+ return make_error<CoverageMapError>(coveragemap_error::malformed,
+ "data is not empty");
} else {
// Skip the padding bytes because coverage records data has an alignment
// of 8.
Pad = offsetToAlignedAddr(Data.data(), Align(8));
if (Data.size() < Pad)
- return make_error<CoverageMapError>(coveragemap_error::malformed);
+ return make_error<CoverageMapError>(coveragemap_error::malformed,
+ "insufficient padding");
Data = Data.substr(Pad);
}
BinaryCoverageReader::FuncRecordsStorage CoverageRecords =
@@ -1000,7 +1041,8 @@ loadBinaryFormat(std::unique_ptr<Binary> Bin, StringRef Arch,
return errorCodeToError(object_error::arch_not_found);
} else
// We can only handle object files.
- return make_error<CoverageMapError>(coveragemap_error::malformed);
+ return make_error<CoverageMapError>(coveragemap_error::malformed,
+ "binary is not an object file");
// The coverage uses native pointer sizes for the object it's written in.
uint8_t BytesInAddress = OF->getBytesInAddress();
@@ -1022,7 +1064,8 @@ loadBinaryFormat(std::unique_ptr<Binary> Bin, StringRef Arch,
return std::move(E);
std::vector<SectionRef> CoverageSectionRefs = *CoverageSection;
if (CoverageSectionRefs.size() != 1)
- return make_error<CoverageMapError>(coveragemap_error::malformed);
+ return make_error<CoverageMapError>(coveragemap_error::malformed,
+ "the size of name section is not one");
auto CoverageMappingOrErr = CoverageSectionRefs.back().getContents();
if (!CoverageMappingOrErr)
return CoverageMappingOrErr.takeError();
@@ -1031,7 +1074,9 @@ loadBinaryFormat(std::unique_ptr<Binary> Bin, StringRef Arch,
InstrProfSymtab ProfileNames;
std::vector<SectionRef> NamesSectionRefs = *NamesSection;
if (NamesSectionRefs.size() != 1)
- return make_error<CoverageMapError>(coveragemap_error::malformed);
+ return make_error<CoverageMapError>(
+ coveragemap_error::malformed,
+ "the size of coverage mapping section is not one");
if (Error E = ProfileNames.create(NamesSectionRefs.back()))
return std::move(E);
diff --git a/llvm/unittests/ProfileData/CoverageMappingTest.cpp b/llvm/unittests/ProfileData/CoverageMappingTest.cpp
index 884a76c83061198..873bc05b2ecc3f3 100644
--- a/llvm/unittests/ProfileData/CoverageMappingTest.cpp
+++ b/llvm/unittests/ProfileData/CoverageMappingTest.cpp
@@ -23,14 +23,17 @@ using namespace llvm;
using namespace coverage;
[[nodiscard]] static ::testing::AssertionResult
-ErrorEquals(coveragemap_error Expected, Error E) {
+ErrorEquals(Error E, coveragemap_error Expected_Err,
+ const std::string &Expected_Msg = std::string()) {
coveragemap_error Found;
+ std::string Msg;
std::string FoundMsg;
handleAllErrors(std::move(E), [&](const CoverageMapError &CME) {
Found = CME.get();
+ Msg = CME.getMessage();
FoundMsg = CME.message();
});
- if (Expected == Found)
+ if (Expected_Err == Found && Msg == Expected_Msg)
return ::testing::AssertionSuccess();
return ::testing::AssertionFailure() << "error: " << FoundMsg << "\n";
}
@@ -342,7 +345,8 @@ TEST_P(CoverageMappingTest, load_coverage_with_bogus_function_name) {
ProfileWriter.addRecord({"", 0x1234, {10}}, Err);
startFunction("", 0x1234);
addCMR(Counter::getCounter(0), "foo", 1, 1, 5, 5);
- EXPECT_TRUE(ErrorEquals(coveragemap_error::malformed, loadCoverageMapping()));
+ EXPECT_TRUE(ErrorEquals(loadCoverageMapping(), coveragemap_error::malformed,
+ "record function name is empty"));
}
TEST_P(CoverageMappingTest, load_coverage_for_several_functions) {
More information about the llvm-commits
mailing list