[compiler-rt] Add more diagnostic to InstrProfError

If profile data is malformed for any kind of reason, we generate
an error that only reports "malformed instrumentation profile data"
without any further information. This patch extends InstrProfError
class to receive an optional error message argument, so that we can
do better error reporting.

Differential Revision: https://reviews.llvm.org/D108942

Author: gulfemsavrun

llvm/include/llvm/ProfileData/InstrProf.h

@@ -308,7 +308,8 @@ inline std::error_code make_error_code(instrprof_error E) {
 
 class InstrProfError : public ErrorInfo<InstrProfError> {
 public:
-  InstrProfError(instrprof_error Err) : Err(Err) {
+  InstrProfError(instrprof_error Err, const Twine &ErrStr = Twine())
+      : Err(Err), Msg(ErrStr.str()) {
     assert(Err != instrprof_error::success && "Not an error");
   }
 
@@ -321,6 +322,7 @@ class InstrProfError : public ErrorInfo<InstrProfError> {
   }
 
   instrprof_error get() const { return Err; }
+  const std::string &getMessage() const { return Msg; }
 
   /// Consume an Error and return the raw enum value contained within it. The
   /// Error must either be a success value, or contain a single InstrProfError.
@@ -337,6 +339,7 @@ class InstrProfError : public ErrorInfo<InstrProfError> {
 
 private:
   instrprof_error Err;
+  std::string Msg;
 };
 
 class SoftInstrProfErrors {
@@ -474,7 +477,8 @@ class InstrProfSymtab {
   /// is used by the raw and text profile readers.
   Error addFuncName(StringRef FuncName) {
     if (FuncName.empty())
-      return make_error<InstrProfError>(instrprof_error::malformed);
+      return make_error<InstrProfError>(instrprof_error::malformed,
+                                        "function name is empty");
     auto Ins = NameTab.insert(FuncName);
     if (Ins.second) {
       MD5NameMap.push_back(std::make_pair(

llvm/include/llvm/ProfileData/InstrProfReader.h

@@ -71,6 +71,7 @@ class InstrProfIterator {
 /// format. Provides an iterator over NamedInstrProfRecords.
 class InstrProfReader {
   instrprof_error LastError = instrprof_error::success;
+  std::string LastErrorMsg;
 
 public:
   InstrProfReader() = default;
@@ -114,14 +115,21 @@ class InstrProfReader {
   std::unique_ptr<InstrProfSymtab> Symtab;
 
   /// Set the current error and return same.
-  Error error(instrprof_error Err) {
+  Error error(instrprof_error Err, const std::string &ErrMsg = "") {
     LastError = Err;
+    LastErrorMsg = ErrMsg;
     if (Err == instrprof_error::success)
       return Error::success();
-    return make_error<InstrProfError>(Err);
+    return make_error<InstrProfError>(Err, ErrMsg);
   }
 
-  Error error(Error &&E) { return error(InstrProfError::take(std::move(E))); }
+  Error error(Error &&E) {
+    handleAllErrors(std::move(E), [&](const InstrProfError &IPE) {
+      LastError = IPE.get();
+      LastErrorMsg = IPE.getMessage();
+    });
+    return make_error<InstrProfError>(LastError, LastErrorMsg);
+  }
 
   /// Clear the current error and return a successful one.
   Error success() { return error(instrprof_error::success); }
@@ -136,7 +144,7 @@ class InstrProfReader {
   /// Get the current error.
   Error getError() {
     if (hasError())
-      return make_error<InstrProfError>(LastError);
+      return make_error<InstrProfError>(LastError, LastErrorMsg);
     return Error::success();
   }
 

llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp

@@ -567,7 +567,8 @@ class VersionedCovMapFuncRecordReader : public CovMapFuncRecordReader {
       if (Error Err = CFR->template getFuncName<Endian>(ProfileNames, FuncName))
         return Err;
       if (FuncName.empty())
-        return make_error<InstrProfError>(instrprof_error::malformed);
+        return make_error<InstrProfError>(instrprof_error::malformed,
+                                          "function name is empty");
       ++CovMapNumUsedRecords;
       Records.emplace_back(Version, FuncName, FuncHash, Mapping,
                            FileRange.StartingIndex, FileRange.Length);

llvm/lib/ProfileData/InstrProf.cpp

@@ -74,53 +74,85 @@ static cl::opt<unsigned> StaticFuncStripDirNamePrefix(
     cl::desc("Strip specified level of directory name from source path in "
              "the profile counter name for static functions."));
 
-static std::string getInstrProfErrString(instrprof_error Err) {
+static std::string getInstrProfErrString(instrprof_error Err,
+                                         const std::string &ErrMsg = "") {
+  std::string Msg;
+  raw_string_ostream OS(Msg);
+
   switch (Err) {
   case instrprof_error::success:
-    return "success";
+    OS << "success";
+    break;
   case instrprof_error::eof:
-    return "end of File";
+    OS << "end of File";
+    break;
   case instrprof_error::unrecognized_format:
-    return "unrecognized instrumentation profile encoding format";
+    OS << "unrecognized instrumentation profile encoding format";
+    break;
   case instrprof_error::bad_magic:
-    return "invalid instrumentation profile data (bad magic)";
+    OS << "invalid instrumentation profile data (bad magic)";
+    break;
   case instrprof_error::bad_header:
-    return "invalid instrumentation profile data (file header is corrupt)";
+    OS << "invalid instrumentation profile data (file header is corrupt)";
+    break;
   case instrprof_error::unsupported_version:
-    return "unsupported instrumentation profile format version";
+    OS << "unsupported instrumentation profile format version";
+    break;
   case instrprof_error::unsupported_hash_type:
-    return "unsupported instrumentation profile hash type";
+    OS << "unsupported instrumentation profile hash type";
+    break;
   case instrprof_error::too_large:
-    return "too much profile data";
+    OS << "too much profile data";
+    break;
   case instrprof_error::truncated:
-    return "truncated profile data";
+    OS << "truncated profile data";
+    break;
   case instrprof_error::malformed:
-    return "malformed instrumentation profile data";
+    OS << "malformed instrumentation profile data";
+    break;
   case instrprof_error::invalid_prof:
-    return "invalid profile created. Please file a bug "
-           "at: " BUG_REPORT_URL
-           " and include the profraw files that caused this error.";
+    OS << "invalid profile created. Please file a bug "
+          "at: " BUG_REPORT_URL
+          " and include the profraw files that caused this error.";
+    break;
   case instrprof_error::unknown_function:
-    return "no profile data available for function";
+    OS << "no profile data available for function";
+    break;
   case instrprof_error::hash_mismatch:
-    return "function control flow change detected (hash mismatch)";
+    OS << "function control flow change detected (hash mismatch)";
+    break;
   case instrprof_error::count_mismatch:
-    return "function basic block count change detected (counter mismatch)";
+    OS << "function basic block count change detected (counter mismatch)";
+    break;
   case instrprof_error::counter_overflow:
-    return "counter overflow";
+    OS << "counter overflow";
+    break;
   case instrprof_error::value_site_count_mismatch:
-    return "function value site count change detected (counter mismatch)";
+    OS << "function value site count change detected (counter mismatch)";
+    break;
   case instrprof_error::compress_failed:
-    return "failed to compress data (zlib)";
+    OS << "failed to compress data (zlib)";
+    break;
   case instrprof_error::uncompress_failed:
-    return "failed to uncompress data (zlib)";
+    OS << "failed to uncompress data (zlib)";
+    break;
   case instrprof_error::empty_raw_profile:
-    return "empty raw profile file";
+    OS << "empty raw profile file";
+    break;
   case instrprof_error::zlib_unavailable:
-    return "profile uses zlib compression but the profile reader was built "
-           "without zlib support";
+    OS << "profile uses zlib compression but the profile reader was built "
+          "without zlib support";
+    break;
+  default:
+    llvm_unreachable("A value of instrprof_error has no message.");
+    break;
   }
-  llvm_unreachable("A value of instrprof_error has no message.");
+
+  // If optional error message is not empty, append it to the message.
+  if (!ErrMsg.empty())
+    OS << ": '" << ErrMsg << "'";
+
+  return OS.str();
 }
 
 namespace {
@@ -217,7 +249,7 @@ void SoftInstrProfErrors::addError(instrprof_error IE) {
 }
 
 std::string InstrProfError::message() const {
-  return getInstrProfErrString(Err);
+  return getInstrProfErrString(Err, Msg);
 }
 
 char InstrProfError::ID = 0;
@@ -878,18 +910,23 @@ static std::unique_ptr<ValueProfData> allocValueProfData(uint32_t TotalSize) {
 
 Error ValueProfData::checkIntegrity() {
   if (NumValueKinds > IPVK_Last + 1)
-    return make_error<InstrProfError>(instrprof_error::malformed);
-  // Total size needs to be mulltiple of quadword size.
+    return make_error<InstrProfError>(
+        instrprof_error::malformed, "number of value profile kinds is invalid");
+  // Total size needs to be multiple of quadword size.
   if (TotalSize % sizeof(uint64_t))
-    return make_error<InstrProfError>(instrprof_error::malformed);
+    return make_error<InstrProfError>(
+        instrprof_error::malformed, "total size is not multiples of quardword");
 
   ValueProfRecord *VR = getFirstValueProfRecord(this);
   for (uint32_t K = 0; K < this->NumValueKinds; K++) {
     if (VR->Kind > IPVK_Last)
-      return make_error<InstrProfError>(instrprof_error::malformed);
+      return make_error<InstrProfError>(instrprof_error::malformed,
+                                        "value kind is invalid");
     VR = getValueProfRecordNext(VR);
     if ((char *)VR - (char *)this > (ptrdiff_t)TotalSize)
-      return make_error<InstrProfError>(instrprof_error::malformed);
+      return make_error<InstrProfError>(
+          instrprof_error::malformed,
+          "value profile address is greater than total size");
   }
   return Error::success();
 }

llvm/lib/ProfileData/InstrProfReader.cpp

@@ -204,13 +204,15 @@ TextInstrProfReader::readValueProfileData(InstrProfRecord &Record) {
     return success();
   }
   if (NumValueKinds == 0 || NumValueKinds > IPVK_Last + 1)
-    return error(instrprof_error::malformed);
+    return error(instrprof_error::malformed,
+                 "number of value kinds is invalid");
   Line++;
 
   for (uint32_t VK = 0; VK < NumValueKinds; VK++) {
     VP_READ_ADVANCE(ValueKind);
     if (ValueKind > IPVK_Last)
-      return error(instrprof_error::malformed);
+      return error(instrprof_error::malformed, "value kind is invalid");
+    ;
     VP_READ_ADVANCE(NumValueSites);
     if (!NumValueSites)
       continue;
@@ -268,16 +270,18 @@ Error TextInstrProfReader::readNextRecord(NamedInstrProfRecord &Record) {
   if (Line.is_at_end())
     return error(instrprof_error::truncated);
   if ((Line++)->getAsInteger(0, Record.Hash))
-    return error(instrprof_error::malformed);
+    return error(instrprof_error::malformed,
+                 "function hash is not a valid integer");
 
   // Read the number of counters.
   uint64_t NumCounters;
   if (Line.is_at_end())
     return error(instrprof_error::truncated);
   if ((Line++)->getAsInteger(10, NumCounters))
-    return error(instrprof_error::malformed);
+    return error(instrprof_error::malformed,
+                 "number of counters is not a valid integer");
   if (NumCounters == 0)
-    return error(instrprof_error::malformed);
+    return error(instrprof_error::malformed, "number of counters is zero");
 
   // Read each counter and fill our internal storage with the values.
   Record.Clear();
@@ -287,7 +291,7 @@ Error TextInstrProfReader::readNextRecord(NamedInstrProfRecord &Record) {
       return error(instrprof_error::truncated);
     uint64_t Count;
     if ((Line++)->getAsInteger(10, Count))
-      return error(instrprof_error::malformed);
+      return error(instrprof_error::malformed, "count is invalid");
     Record.Counts.push_back(Count);
   }
 
@@ -332,10 +336,12 @@ Error RawInstrProfReader<IntPtrT>::readNextHeader(const char *CurrentPos) {
   // If there isn't enough space for another header, this is probably just
   // garbage at the end of the file.
   if (CurrentPos + sizeof(RawInstrProf::Header) > End)
-    return make_error<InstrProfError>(instrprof_error::malformed);
+    return make_error<InstrProfError>(instrprof_error::malformed,
+                                      "not enough space for another header");
   // The writer ensures each profile is padded to start at an aligned address.
   if (reinterpret_cast<size_t>(CurrentPos) % alignof(uint64_t))
-    return make_error<InstrProfError>(instrprof_error::malformed);
+    return make_error<InstrProfError>(instrprof_error::malformed,
+                                      "insufficient padding");
   // The magic should have the same byte order as in the previous header.
   uint64_t Magic = *reinterpret_cast<const uint64_t *>(CurrentPos);
   if (Magic != swap(RawInstrProf::getMagic<IntPtrT>()))
@@ -426,21 +432,39 @@ template <class IntPtrT>
 Error RawInstrProfReader<IntPtrT>::readRawCounts(
     InstrProfRecord &Record) {
   uint32_t NumCounters = swap(Data->NumCounters);
-  IntPtrT CounterPtr = Data->CounterPtr;
   if (NumCounters == 0)
-    return error(instrprof_error::malformed);
+    return error(instrprof_error::malformed, "number of counters is zero");
 
+  IntPtrT CounterPtr = Data->CounterPtr;
   auto *NamesStartAsCounter = reinterpret_cast<const uint64_t *>(NamesStart);
   ptrdiff_t MaxNumCounters = NamesStartAsCounter - CountersStart;
 
   // Check bounds. Note that the counter pointer embedded in the data record
   // may itself be corrupt.
   if (MaxNumCounters < 0 || NumCounters > (uint32_t)MaxNumCounters)
-    return error(instrprof_error::malformed);
+    return error(instrprof_error::malformed,
+                 "counter pointer is out of bounds");
   ptrdiff_t CounterOffset = getCounterOffset(CounterPtr);
-  if (CounterOffset < 0 || CounterOffset > MaxNumCounters ||
-      ((uint32_t)CounterOffset + NumCounters) > (uint32_t)MaxNumCounters)
-    return error(instrprof_error::malformed);
+  if (CounterOffset < 0)
+    return error(
+        instrprof_error::malformed,
+        ("counter offset(" + Twine(CounterOffset) + ")" + " is < 0").str());
+
+  if (CounterOffset > MaxNumCounters)
+    return error(instrprof_error::malformed,
+                 ("counter offset(" + Twine(CounterOffset) + ")" + " > " +
+                  "max number of counters(" + Twine((uint32_t)MaxNumCounters) +
+                  ")")
+                     .str());
+
+  if (((uint32_t)CounterOffset + NumCounters) > (uint32_t)MaxNumCounters)
+    return error(instrprof_error::malformed,
+                 ("number of counters is out of bounds(counter offset(" +
+                  Twine((uint32_t)CounterOffset) + ") + " +
+                  "number of counters(" + Twine(NumCounters) + ") > " +
+                  "max number of counters(" + Twine((uint32_t)MaxNumCounters) +
+                  "))")
+                     .str());
 
   auto RawCounts = makeArrayRef(getCounter(CounterOffset), NumCounters);
 
@@ -524,7 +548,9 @@ Error RawInstrProfReader<IntPtrT>::printBinaryIds(raw_ostream &OS) {
     // Increment by binary id length data type size.
     BI += sizeof(BinaryIdLen);
     if (BI > (const uint8_t *)DataBuffer->getBufferEnd())
-      return make_error<InstrProfError>(instrprof_error::malformed);
+      return make_error<InstrProfError>(
+          instrprof_error::malformed,
+          "binary id that is read is bigger than buffer size");
 
     for (uint64_t I = 0; I < BinaryIdLen; I++)
       OS << format("%02x", BI[I]);
@@ -624,7 +650,8 @@ Error InstrProfReaderIndex<HashTableImpl>::getRecords(
 
   Data = (*Iter);
   if (Data.empty())
-    return make_error<InstrProfError>(instrprof_error::malformed);
+    return make_error<InstrProfError>(instrprof_error::malformed,
+                                      "profile data is empty");
 
   return Error::success();
 }
@@ -638,7 +665,8 @@ Error InstrProfReaderIndex<HashTableImpl>::getRecords(
   Data = *RecordIterator;
 
   if (Data.empty())
-    return make_error<InstrProfError>(instrprof_error::malformed);
+    return make_error<InstrProfError>(instrprof_error::malformed,
+                                      "profile data is empty");
 
   return Error::success();
 }
@@ -669,7 +697,7 @@ class InstrProfReaderNullRemapper : public InstrProfReaderRemapper {
     return Underlying.getRecords(FuncName, Data);
   }
 };
-}
+} // namespace
 
 /// A remapper that applies remappings based on a symbol remapping file.
 template <typename HashTableImpl>