diff --git a/llvm/include/llvm/ProfileData/InstrProf.h b/llvm/include/llvm/ProfileData/InstrProf.h --- a/llvm/include/llvm/ProfileData/InstrProf.h +++ b/llvm/include/llvm/ProfileData/InstrProf.h @@ -308,7 +308,8 @@ class InstrProfError : public ErrorInfo { 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 @@ } 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 @@ private: instrprof_error Err; + std::string Msg; }; class SoftInstrProfErrors { @@ -474,7 +477,8 @@ /// is used by the raw and text profile readers. Error addFuncName(StringRef FuncName) { if (FuncName.empty()) - return make_error(instrprof_error::malformed); + return make_error(instrprof_error::malformed, + "function name is empty"); auto Ins = NameTab.insert(FuncName); if (Ins.second) { MD5NameMap.push_back(std::make_pair( diff --git a/llvm/include/llvm/ProfileData/InstrProfReader.h b/llvm/include/llvm/ProfileData/InstrProfReader.h --- a/llvm/include/llvm/ProfileData/InstrProfReader.h +++ b/llvm/include/llvm/ProfileData/InstrProfReader.h @@ -71,6 +71,7 @@ /// format. Provides an iterator over NamedInstrProfRecords. class InstrProfReader { instrprof_error LastError = instrprof_error::success; + std::string LastErrorMsg; public: InstrProfReader() = default; @@ -114,14 +115,21 @@ std::unique_ptr 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(Err); + return make_error(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(LastError, LastErrorMsg); + } /// Clear the current error and return a successful one. Error success() { return error(instrprof_error::success); } @@ -136,7 +144,7 @@ /// Get the current error. Error getError() { if (hasError()) - return make_error(LastError); + return make_error(LastError, LastErrorMsg); return Error::success(); } 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 @@ -567,7 +567,8 @@ if (Error Err = CFR->template getFuncName(ProfileNames, FuncName)) return Err; if (FuncName.empty()) - return make_error(instrprof_error::malformed); + return make_error(instrprof_error::malformed, + "function name is empty"); ++CovMapNumUsedRecords; Records.emplace_back(Version, FuncName, FuncHash, Mapping, FileRange.StartingIndex, FileRange.Length); diff --git a/llvm/lib/ProfileData/InstrProf.cpp b/llvm/lib/ProfileData/InstrProf.cpp --- a/llvm/lib/ProfileData/InstrProf.cpp +++ b/llvm/lib/ProfileData/InstrProf.cpp @@ -74,53 +74,85 @@ 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 @@ } std::string InstrProfError::message() const { - return getInstrProfErrString(Err); + return getInstrProfErrString(Err, Msg); } char InstrProfError::ID = 0; @@ -878,18 +910,23 @@ Error ValueProfData::checkIntegrity() { if (NumValueKinds > IPVK_Last + 1) - return make_error(instrprof_error::malformed); - // Total size needs to be mulltiple of quadword size. + return make_error( + 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(instrprof_error::malformed); + return make_error( + 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(instrprof_error::malformed); + return make_error(instrprof_error::malformed, + "value kind is invalid"); VR = getValueProfRecordNext(VR); if ((char *)VR - (char *)this > (ptrdiff_t)TotalSize) - return make_error(instrprof_error::malformed); + return make_error( + instrprof_error::malformed, + "value profile address is greater than total size"); } return Error::success(); } diff --git a/llvm/lib/ProfileData/InstrProfReader.cpp b/llvm/lib/ProfileData/InstrProfReader.cpp --- a/llvm/lib/ProfileData/InstrProfReader.cpp +++ b/llvm/lib/ProfileData/InstrProfReader.cpp @@ -204,13 +204,15 @@ 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,16 @@ 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 invalid"); // 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 invalid"); 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 +289,7 @@ 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 +334,12 @@ // 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(instrprof_error::malformed); + return make_error(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(CurrentPos) % alignof(uint64_t)) - return make_error(instrprof_error::malformed); + return make_error(instrprof_error::malformed, + "padding is missing"); // The magic should have the same byte order as in the previous header. uint64_t Magic = *reinterpret_cast(CurrentPos); if (Magic != swap(RawInstrProf::getMagic())) @@ -435,7 +439,7 @@ 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"); auto *NamesStartAsCounter = reinterpret_cast(NamesStart); ptrdiff_t MaxNumCounters = NamesStartAsCounter - CountersStart; @@ -443,7 +447,7 @@ // 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 invalid"); // We need to compute the in-buffer counter offset from the in-memory address // distance. The initial CountersDelta is the in-memory address difference @@ -455,7 +459,7 @@ CountersDelta -= sizeof(*Data); if (CounterOffset < 0 || CounterOffset > MaxNumCounters || ((uint32_t)CounterOffset + NumCounters) > (uint32_t)MaxNumCounters) - return error(instrprof_error::malformed); + return error(instrprof_error::malformed, "counter offset is invalid"); auto RawCounts = makeArrayRef(getCounter(CounterOffset), NumCounters); @@ -544,19 +548,25 @@ // There should be enough left to read the binary ID size field. if (Remaining < sizeof(uint64_t)) - return make_error(instrprof_error::malformed); + return make_error( + instrprof_error::malformed, + "not enough space to read binary id length"); uint64_t BinaryIdLen = swap(*reinterpret_cast(BI)); // There should be enough left to read the binary ID size field, and the // binary ID. if (Remaining < sizeof(BinaryIdLen) + BinaryIdLen) - return make_error(instrprof_error::malformed); + return make_error( + instrprof_error::malformed, + "not enough space to read binary id data"); // Increment by binary id length data type size. BI += sizeof(BinaryIdLen); if (BI > (const uint8_t *)DataBuffer->getBufferEnd()) - return make_error(instrprof_error::malformed); + return make_error( + 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]); @@ -657,7 +667,8 @@ Data = (*Iter); if (Data.empty()) - return make_error(instrprof_error::malformed); + return make_error(instrprof_error::malformed, + "profile data is empty"); return Error::success(); } @@ -671,7 +682,8 @@ Data = *RecordIterator; if (Data.empty()) - return make_error(instrprof_error::malformed); + return make_error(instrprof_error::malformed, + "profile data is empty"); return Error::success(); } @@ -702,7 +714,7 @@ return Underlying.getRecords(FuncName, Data); } }; -} +} // namespace /// A remapper that applies remappings based on a symbol remapping file. template diff --git a/llvm/test/tools/llvm-profdata/large-binary-id-size.test b/llvm/test/tools/llvm-profdata/large-binary-id-size.test --- a/llvm/test/tools/llvm-profdata/large-binary-id-size.test +++ b/llvm/test/tools/llvm-profdata/large-binary-id-size.test @@ -17,4 +17,4 @@ RUN: printf '\0\1\2\3\0\0\0\0' >> %t.profraw // RUN: not llvm-profdata show --binary-ids %t.profraw 2>&1 | FileCheck %s -// CHECK: malformed instrumentation profile data +// CHECK: malformed instrumentation profile data : not enough space to read binary id data diff --git a/llvm/test/tools/llvm-profdata/malformed-not-space-for-another-header.test b/llvm/test/tools/llvm-profdata/malformed-not-space-for-another-header.test new file mode 100644 --- /dev/null +++ b/llvm/test/tools/llvm-profdata/malformed-not-space-for-another-header.test @@ -0,0 +1,51 @@ +// Header +// +// INSTR_PROF_RAW_HEADER(uint64_t, Magic, __llvm_profile_get_magic()) +// INSTR_PROF_RAW_HEADER(uint64_t, Version, __llvm_profile_get_version()) +// INSTR_PROF_RAW_HEADER(uint64_t, BinaryIdsSize, __llvm_write_binary_ids(NULL)) +// INSTR_PROF_RAW_HEADER(uint64_t, DataSize, DataSize) +// INSTR_PROF_RAW_HEADER(uint64_t, CountersSize, CountersSize) +// INSTR_PROF_RAW_HEADER(uint64_t, NamesSize, NamesSize) +// INSTR_PROF_RAW_HEADER(uint64_t, CountersDelta, (uintptr_t)CountersBegin) +// INSTR_PROF_RAW_HEADER(uint64_t, NamesDelta, (uintptr_t)NamesBegin) +// INSTR_PROF_RAW_HEADER(uint64_t, ValueKindLast, IPVK_Last) + +RUN: printf '\201rforpl\377' > %t.profraw +RUN: printf '\x8\0\0\0\0\0\0\0' >> %t.profraw +RUN: printf '\0\0\0\0\0\0\0\0' >> %t.profraw +RUN: printf '\1\0\0\0\0\0\0\0' >> %t.profraw +RUN: printf '\0\0\0\0\0\0\0\0' >> %t.profraw +RUN: printf '\1\0\0\0\0\0\0\0' >> %t.profraw +RUN: printf '\0\0\0\0\0\0\0\0' >> %t.profraw +RUN: printf '\xe\0\0\0\0\0\0\0' >> %t.profraw +RUN: printf '\xf8\xff\xff\xff\xff\xff\xff\xff' >> %t.profraw +RUN: printf '\x89\x7a\x40\x00\x00\x00\x00\x00' >> %t.profraw +RUN: printf '\1\0\0\0\0\0\0\0' >> %t.profraw + +// Data Section +// +// struct ProfData { +// #define INSTR_PROF_DATA(Type, LLVMType, Name, Initializer) \ +// Type Name; +// #include "llvm/ProfileData/InstrProfData.inc" +// }; + +RUN: printf '\xfa\xd5\x8d\xe7\x36\x64\x95\xdb' >> %t.profraw // NameRef +RUN: printf '\x18\0\0\0\0\0\0\0' >> %t.profraw // FuncHash +RUN: printf '\xf8\xff\xff\xff\xff\xff\xff\xff' >> %t.profraw // RelativeCounterPtr +RUN: printf '\0\0\0\0\0\0\0\0' >> %t.profraw // FunctionPointer +RUN: printf '\0\0\0\0\0\0\0\0' >> %t.profraw // Values +RUN: printf '\1\0\0\0\0\0\0\0' >> %t.profraw // NumCounters and NumValueSites + +// Counter section +RUN: printf '\1\0\0\0\0\0\0\0' >> %t.profraw + +// Name section (Name section is 14 bytes and 2 bytes padding is added) +RUN: printf '\x04\x0c\x78\xda\xcb\x4d\xcc\xcc' >> %t.profraw +RUN: printf '\x03\x00\x04\x1b\x01\xa6\x00\x00' >> %t.profraw + +// Write some garbage data at the end so we get "not enough space for another header" message +RUN: printf '\x03\x00\' >> %t.profraw + +RUN: not llvm-profdata show %t.profraw 2>&1 | FileCheck %s +CHECK: malformed instrumentation profile data : not enough space for another header diff --git a/llvm/test/tools/llvm-profdata/malformed-num-counters-zero.test b/llvm/test/tools/llvm-profdata/malformed-num-counters-zero.test new file mode 100644 --- /dev/null +++ b/llvm/test/tools/llvm-profdata/malformed-num-counters-zero.test @@ -0,0 +1,49 @@ +// Header +// +// INSTR_PROF_RAW_HEADER(uint64_t, Magic, __llvm_profile_get_magic()) +// INSTR_PROF_RAW_HEADER(uint64_t, Version, __llvm_profile_get_version()) +// INSTR_PROF_RAW_HEADER(uint64_t, BinaryIdsSize, __llvm_write_binary_ids(NULL)) +// INSTR_PROF_RAW_HEADER(uint64_t, DataSize, DataSize) +// INSTR_PROF_RAW_HEADER(uint64_t, CountersSize, CountersSize) +// INSTR_PROF_RAW_HEADER(uint64_t, NamesSize, NamesSize) +// INSTR_PROF_RAW_HEADER(uint64_t, CountersDelta, (uintptr_t)CountersBegin) +// INSTR_PROF_RAW_HEADER(uint64_t, NamesDelta, (uintptr_t)NamesBegin) +// INSTR_PROF_RAW_HEADER(uint64_t, ValueKindLast, IPVK_Last) + +RUN: printf '\201rforpl\377' > %t.profraw +RUN: printf '\x8\0\0\0\0\0\0\0' >> %t.profraw +RUN: printf '\0\0\0\0\0\0\0\0' >> %t.profraw +RUN: printf '\1\0\0\0\0\0\0\0' >> %t.profraw +RUN: printf '\0\0\0\0\0\0\0\0' >> %t.profraw +RUN: printf '\1\0\0\0\0\0\0\0' >> %t.profraw +RUN: printf '\0\0\0\0\0\0\0\0' >> %t.profraw +RUN: printf '\xe\0\0\0\0\0\0\0' >> %t.profraw +RUN: printf '\xf8\xff\xff\xff\xff\xff\xff\xff' >> %t.profraw +RUN: printf '\x89\x7a\x40\x00\x00\x00\x00\x00' >> %t.profraw +RUN: printf '\1\0\0\0\0\0\0\0' >> %t.profraw + +// Data Section +// +// struct ProfData { +// #define INSTR_PROF_DATA(Type, LLVMType, Name, Initializer) \ +// Type Name; +// #include "llvm/ProfileData/InstrProfData.inc" +// }; + +RUN: printf '\xfa\xd5\x8d\xe7\x36\x64\x95\xdb' >> %t.profraw // NameRef +RUN: printf '\x18\0\0\0\0\0\0\0' >> %t.profraw // FuncHash +RUN: printf '\xf8\xff\xff\xff\xff\xff\xff\xff' >> %t.profraw // RelativeCounterPtr +RUN: printf '\0\0\0\0\0\0\0\0' >> %t.profraw // FunctionPointer +RUN: printf '\0\0\0\0\0\0\0\0' >> %t.profraw // Values +// Make NumCounters = 0 so that we get "number of counters is zero" error message +RUN: printf '\0\0\0\0\0\0\0\0' >> %t.profraw // NumCounters and NumValueSites + +// Counter section +RUN: printf '\1\0\0\0\0\0\0\0' >> %t.profraw + +// Name section (Name section is 14 bytes and 2 bytes padding is added) +RUN: printf '\x04\x0c\x78\xda\xcb\x4d\xcc\xcc' >> %t.profraw +RUN: printf '\x03\x00\x04\x1b\x01\xa6\x00\x00' >> %t.profraw + +RUN: not llvm-profdata show %t.profraw 2>&1 | FileCheck %s +CHECK: malformed instrumentation profile data : number of counters is zero diff --git a/llvm/test/tools/llvm-profdata/malformed-ptr-to-counter-array.test b/llvm/test/tools/llvm-profdata/malformed-ptr-to-counter-array.test --- a/llvm/test/tools/llvm-profdata/malformed-ptr-to-counter-array.test +++ b/llvm/test/tools/llvm-profdata/malformed-ptr-to-counter-array.test @@ -53,5 +53,5 @@ RUN: printf '\3\0bar\0\0\0' >> %t.profraw RUN: not llvm-profdata merge -o /dev/null %t.profraw 2>&1 | FileCheck %s -CHECK: warning: {{.+}}: malformed instrumentation profile data +CHECK: warning: {{.+}}: malformed instrumentation profile data : counter offset is invalid CHECK: error: no profile can be merged