Skip to content

Commit

Permalink
[llvm-profdata] Fixed various issue with Sample Profile Reader
Browse files Browse the repository at this point in the history
Fixed various undefind behaviors with current Sample Profile Reader when reading unusual input. Furthermore, add the following rule on allowing multiple name table sections (current Reader has conflicted code handling such case):

When a new name table section is read (in the order sections are read), the names in the previous name table are cleared. Any subsequent sections referring to function names will index into the most recent read name table.

Also changed name table index to uint64_t to be consistent since there's a mix of using uint32_t and uint64_t.

Reviewed By: snehasish, huangjd

Differential Revision: https://reviews.llvm.org/D146182
  • Loading branch information
huangjd committed Apr 13, 2023
1 parent 6f8360a commit 30037b7
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 14 deletions.
2 changes: 1 addition & 1 deletion llvm/include/llvm/ProfileData/SampleProf.h
Expand Up @@ -169,7 +169,7 @@ struct SecHdrTableEntry {
uint64_t Size;
// The index indicating the location of the current entry in
// SectionHdrLayout table.
uint32_t LayoutIndex;
uint64_t LayoutIndex;
};

// Flags common for all sections are defined here. In SecHdrTableEntry::Flags,
Expand Down
10 changes: 5 additions & 5 deletions llvm/include/llvm/ProfileData/SampleProfReader.h
Expand Up @@ -170,7 +170,7 @@
// Number of samples to get to the desrired percentile.
//
// NAME TABLE
// SIZE (uint32_t)
// SIZE (uint64_t)
// Number of entries in the name table.
// NAMES
// A NUL-separated list of SIZE strings.
Expand All @@ -182,7 +182,7 @@
// NOTE: This field should only be present for top-level functions
// (i.e., not inlined into any caller). Inlined function calls
// have no prologue, so they don't need this.
// NAME_IDX (uint32_t)
// NAME_IDX (uint64_t)
// Index into the name table indicating the function name.
// SAMPLES (uint64_t)
// Total number of samples collected in this function.
Expand All @@ -204,7 +204,7 @@
// represent all the actual functions called at runtime.
// CALL_TARGETS
// A list of NUM_CALLS entries for each called function:
// NAME_IDX (uint32_t)
// NAME_IDX (uint64_t)
// Index into the name table with the callee name.
// SAMPLES (uint64_t)
// Number of samples collected at the call site.
Expand Down Expand Up @@ -623,7 +623,7 @@ class SampleProfileReaderBinary : public SampleProfileReader {
ErrorOr<StringRef> readString();

/// Read the string index and check whether it overflows the table.
template <typename T> inline ErrorOr<uint32_t> readStringIndex(T &Table);
template <typename T> inline ErrorOr<size_t> readStringIndex(T &Table);

/// Return true if we've reached the end of file.
bool at_eof() const { return Data >= End; }
Expand Down Expand Up @@ -704,7 +704,7 @@ class SampleProfileReaderExtBinaryBase : public SampleProfileReaderBinary {

protected:
std::vector<SecHdrTableEntry> SecHdrTable;
std::error_code readSecHdrTableEntry(uint32_t Idx);
std::error_code readSecHdrTableEntry(uint64_t Idx);
std::error_code readSecHdrTable();

std::error_code readFuncMetadata(bool ProfileHasAttribute);
Expand Down
16 changes: 8 additions & 8 deletions llvm/lib/ProfileData/SampleProfReader.cpp
Expand Up @@ -515,9 +515,9 @@ ErrorOr<T> SampleProfileReaderBinary::readUnencodedNumber() {
}

template <typename T>
inline ErrorOr<uint32_t> SampleProfileReaderBinary::readStringIndex(T &Table) {
inline ErrorOr<size_t> SampleProfileReaderBinary::readStringIndex(T &Table) {
std::error_code EC;
auto Idx = readNumber<uint32_t>();
auto Idx = readNumber<size_t>();
if (std::error_code EC = Idx.getError())
return EC;
if (*Idx >= Table.size())
Expand Down Expand Up @@ -696,7 +696,7 @@ std::error_code SampleProfileReaderBinary::readImpl() {

ErrorOr<SampleContextFrames>
SampleProfileReaderExtBinaryBase::readContextFromTable() {
auto ContextIdx = readNumber<uint32_t>();
auto ContextIdx = readNumber<size_t>();
if (std::error_code EC = ContextIdx.getError())
return EC;
if (*ContextIdx >= CSNameTable->size())
Expand Down Expand Up @@ -1066,11 +1066,11 @@ SampleProfileReaderCompactBinary::verifySPMagic(uint64_t Magic) {
}

std::error_code SampleProfileReaderBinary::readNameTable() {
auto Size = readNumber<uint32_t>();
auto Size = readNumber<size_t>();
if (std::error_code EC = Size.getError())
return EC;
NameTable.reserve(*Size + NameTable.size());
for (uint32_t I = 0; I < *Size; ++I) {
for (size_t I = 0; I < *Size; ++I) {
auto Name(readString());
if (std::error_code EC = Name.getError())
return EC;
Expand Down Expand Up @@ -1121,14 +1121,14 @@ std::error_code SampleProfileReaderExtBinaryBase::readNameTableSec(bool IsMD5) {
// underlying raw function names that are stored in the name table, as well as
// a callsite identifier that only makes sense for non-leaf frames.
std::error_code SampleProfileReaderExtBinaryBase::readCSNameTableSec() {
auto Size = readNumber<uint32_t>();
auto Size = readNumber<size_t>();
if (std::error_code EC = Size.getError())
return EC;

std::vector<SampleContextFrameVector> *PNameVec =
new std::vector<SampleContextFrameVector>();
PNameVec->reserve(*Size);
for (uint32_t I = 0; I < *Size; ++I) {
for (size_t I = 0; I < *Size; ++I) {
PNameVec->emplace_back(SampleContextFrameVector());
auto ContextSize = readNumber<uint32_t>();
if (std::error_code EC = ContextSize.getError())
Expand Down Expand Up @@ -1249,7 +1249,7 @@ std::error_code SampleProfileReaderCompactBinary::readNameTable() {
}

std::error_code
SampleProfileReaderExtBinaryBase::readSecHdrTableEntry(uint32_t Idx) {
SampleProfileReaderExtBinaryBase::readSecHdrTableEntry(uint64_t Idx) {
SecHdrTableEntry Entry;
auto Type = readUnencodedNumber<uint64_t>();
if (std::error_code EC = Type.getError())
Expand Down

0 comments on commit 30037b7

Please sign in to comment.