[llvm-profdata] Emit Error when Invalid MemOpSize Section is Created by llvm-profdata

Under certain (currently unknown) conditions, llvm-profdata is outputting
profiles that have two consecutive entries in the MemOPSize section for the
value 0. This causes the PGOMemOPSizeOpt pass to output an invalid switch
instruction with two cases for 0. As mentioned, we’re not quite sure what’s
causing this to happen, but this patch prevents llvm-profdata from outputting a
profile that has this problem and gives an error with a request for a
reproducible.

Differential Revision: https://reviews.llvm.org/D92074
This commit is contained in:
Matthew Voss 2021-01-14 15:31:32 -08:00
parent f51b3de4e8
commit 6da7d31416
10 changed files with 217 additions and 18 deletions

View File

@ -291,6 +291,7 @@ enum class instrprof_error {
truncated,
malformed,
unknown_function,
invalid_prof,
hash_mismatch,
count_mismatch,
counter_overflow,

View File

@ -64,11 +64,13 @@ public:
function_ref<void(Error)> Warn);
/// Write the profile to \c OS
void write(raw_fd_ostream &OS);
Error write(raw_fd_ostream &OS);
/// Write the profile in text format to \c OS
Error writeText(raw_fd_ostream &OS);
Error validateRecord(const InstrProfRecord &Func);
/// Write \c Record in text format to \c OS
static void writeRecordInText(StringRef Name, uint64_t Hash,
const InstrProfRecord &Counters,
@ -114,7 +116,8 @@ private:
void addRecord(StringRef Name, uint64_t Hash, InstrProfRecord &&I,
uint64_t Weight, function_ref<void(Error)> Warn);
bool shouldEncodeData(const ProfilingData &PD);
void writeImpl(ProfOStream &OS);
Error writeImpl(ProfOStream &OS);
};
} // end namespace llvm

View File

@ -18,6 +18,7 @@
#include "llvm/ADT/StringExtras.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/Triple.h"
#include "llvm/Config/config.h"
#include "llvm/IR/Constant.h"
#include "llvm/IR/Constants.h"
#include "llvm/IR/Function.h"
@ -95,6 +96,10 @@ static std::string getInstrProfErrString(instrprof_error Err) {
return "Truncated profile data";
case instrprof_error::malformed:
return "Malformed instrumentation profile data";
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.";
case instrprof_error::unknown_function:
return "No profile data available for function";
case instrprof_error::hash_mismatch:

View File

@ -285,7 +285,7 @@ static void setSummary(IndexedInstrProf::Summary *TheSummary,
TheSummary->setEntry(I, Res[I]);
}
void InstrProfWriter::writeImpl(ProfOStream &OS) {
Error InstrProfWriter::writeImpl(ProfOStream &OS) {
using namespace IndexedInstrProf;
OnDiskChainedHashTableGenerator<InstrProfRecordWriterTrait> Generator;
@ -376,12 +376,19 @@ void InstrProfWriter::writeImpl(ProfOStream &OS) {
(int)CSSummarySize}};
OS.patch(PatchItems, sizeof(PatchItems) / sizeof(*PatchItems));
for (const auto &I : FunctionData)
for (const auto &F : I.getValue())
if (Error E = validateRecord(F.second))
return E;
return Error::success();
}
void InstrProfWriter::write(raw_fd_ostream &OS) {
Error InstrProfWriter::write(raw_fd_ostream &OS) {
// Write the hash table.
ProfOStream POS(OS);
writeImpl(POS);
return writeImpl(POS);
}
std::unique_ptr<MemoryBuffer> InstrProfWriter::writeBuffer() {
@ -389,7 +396,8 @@ std::unique_ptr<MemoryBuffer> InstrProfWriter::writeBuffer() {
raw_string_ostream OS(Data);
ProfOStream POS(OS);
// Write the hash table.
writeImpl(POS);
if (Error E = writeImpl(POS))
return nullptr;
// Return this in an aligned memory buffer.
return MemoryBuffer::getMemBufferCopy(Data);
}
@ -399,6 +407,27 @@ static const char *ValueProfKindStr[] = {
#include "llvm/ProfileData/InstrProfData.inc"
};
Error InstrProfWriter::validateRecord(const InstrProfRecord &Func) {
for (uint32_t VK = 0; VK <= IPVK_Last; VK++) {
uint32_t NS = Func.getNumValueSites(VK);
if (!NS)
continue;
for (uint32_t S = 0; S < NS; S++) {
uint32_t ND = Func.getNumValueDataForSite(VK, S);
std::unique_ptr<InstrProfValueData[]> VD = Func.getValueForSite(VK, S);
bool WasZero = false;
for (uint32_t I = 0; I < ND; I++)
if ((VK != IPVK_IndirectCallTarget) && (VD[I].Value == 0)) {
if (WasZero)
return make_error<InstrProfError>(instrprof_error::invalid_prof);
WasZero = true;
}
}
}
return Error::success();
}
void InstrProfWriter::writeRecordInText(StringRef Name, uint64_t Hash,
const InstrProfRecord &Func,
InstrProfSymtab &Symtab,
@ -473,5 +502,11 @@ Error InstrProfWriter::writeText(raw_fd_ostream &OS) {
writeRecordInText(Name, Func.first, Func.second, Symtab, OS);
}
for (const auto &record : OrderedFuncData) {
const FuncPair &Func = record.second;
if (Error E = validateRecord(Func.second))
return E;
}
return Error::success();
}

View File

@ -46,6 +46,7 @@
#include "llvm/Support/Debug.h"
#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/MathExtras.h"
#include "llvm/Support/WithColor.h"
#include "llvm/Transforms/Instrumentation.h"
#include "llvm/Transforms/Instrumentation/PGOInstrumentation.h"
#include "llvm/Transforms/Utils/BasicBlockUtils.h"
@ -338,6 +339,7 @@ bool MemOPSizeOpt::perform(MemOp MO) {
SmallVector<uint64_t, 16> CaseCounts;
uint64_t MaxCount = 0;
unsigned Version = 0;
int64_t LastV = -1;
// Default case is in the front -- save the slot here.
CaseCounts.push_back(0);
for (auto &VD : VDs) {
@ -354,6 +356,15 @@ bool MemOPSizeOpt::perform(MemOp MO) {
if (!isProfitable(C, RemainCount))
break;
if (V == LastV) {
LLVM_DEBUG(dbgs() << "Invalid Profile Data in Function " << Func.getName()
<< ": Two consecutive, identical values in MemOp value"
"counts.\n");
return false;
}
LastV = V;
SizeIds.push_back(V);
CaseCounts.push_back(C);
if (C > MaxCount)

View File

@ -0,0 +1,47 @@
# IR level Instrumentation Flag
:ir
foo
# Func Hash:
687116424982578944
# Num Counters:
3
# Counter Values:
523
20
1
# Num Value Kinds:
1
# ValueKind = IPVK_MemOPSize:
1
# NumValueSites:
3
9
0:99
0:88
3:77
9:72
4:66
5:55
6:44
7:33
8:22
9
7:33
2:88
9:72
4:66
1:99
5:55
6:44
3:77
8:22
9
7:33
2:88
9:72
4:66
1:99
5:55
6:44
3:77
8:22

View File

@ -0,0 +1,58 @@
; RUN: llvm-profdata merge %S/Inputs/consecutive-zeros.proftext -o %t.profdata
; RUN: opt < %s -debug -pgo-instr-use -pgo-memop-opt -pgo-memop-count-threshold=0 -pgo-memop-percent-threshold=0 -pgo-test-profile-file=%t.profdata -S 2>&1 | FileCheck %s
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"
define void @foo(i8* %dst, i8* %src, i32* %a, i32 %n) {
; CHECK: Invalid Profile
entry:
br label %for.cond
for.cond:
%i.0 = phi i32 [ 0, %entry ], [ %inc5, %for.inc4 ]
%cmp = icmp slt i32 %i.0, %n
br i1 %cmp, label %for.body, label %for.end6
for.body:
br label %for.cond1
for.cond1:
%j.0 = phi i32 [ 0, %for.body ], [ %inc, %for.inc ]
%idx.ext = sext i32 %i.0 to i64
%add.ptr = getelementptr inbounds i32, i32* %a, i64 %idx.ext
%0 = load i32, i32* %add.ptr, align 4
%cmp2 = icmp slt i32 %j.0, %0
br i1 %cmp2, label %for.body3, label %for.end
for.body3:
%add = add nsw i32 %i.0, 1
%conv = sext i32 %add to i64
call void @llvm.memcpy.p0i8.p0i8.i64(i8* %dst, i8* %src, i64 %conv, i1 false)
%memcmp = call i32 @memcmp(i8* %dst, i8* %src, i64 %conv)
%bcmp = call i32 @bcmp(i8* %dst, i8* %src, i64 %conv)
br label %for.inc
for.inc:
%inc = add nsw i32 %j.0, 1
br label %for.cond1
for.end:
br label %for.inc4
for.inc4:
%inc5 = add nsw i32 %i.0, 1
br label %for.cond
for.end6:
ret void
}
declare void @llvm.lifetime.start(i64, i8* nocapture)
declare void @llvm.memcpy.p0i8.p0i8.i64(i8* nocapture writeonly, i8* nocapture readonly, i64, i1)
declare i32 @memcmp(i8*, i8*, i64)
declare i32 @bcmp(i8*, i8*, i64)
declare void @llvm.lifetime.end(i64, i8* nocapture)

View File

@ -37,20 +37,20 @@ for.body3:
br label %for.inc
; MEMOP_OPT: switch i64 %conv, label %[[DEFAULT_LABEL:.*]] [
; MEMOP_OPT: i64 1, label %[[CASE_1_LABEL:.*]]
; MEMOP_OPT: i64 0, label %[[CASE_1_LABEL:.*]]
; MEMOP_OPT: ], !prof [[SWITCH_BW:![0-9]+]]
; MEMOP_OPT: [[CASE_1_LABEL]]:
; MEMOP_OPT: call void @llvm.memcpy.p0i8.p0i8.i64(i8* %dst, i8* %src, i64 1, i1 false)
; MEMOP_OPT: call void @llvm.memcpy.p0i8.p0i8.i64(i8* %dst, i8* %src, i64 0, i1 false)
; MEMOP_OPT: br label %[[MERGE_LABEL:.*]]
; MEMOP_OPT: [[DEFAULT_LABEL]]:
; MEMOP_OPT: call void @llvm.memcpy.p0i8.p0i8.i64(i8* %dst, i8* %src, i64 %conv, i1 false), !prof [[NEWVP:![0-9]+]]
; MEMOP_OPT: br label %[[MERGE_LABEL]]
; MEMOP_OPT: [[MERGE_LABEL]]:
; MEMOP_OPT: switch i64 %conv, label %[[DEFAULT_LABEL2:.*]] [
; MEMOP_OPT: i64 1, label %[[CASE_1_LABEL2:.*]]
; MEMOP_OPT: i64 0, label %[[CASE_1_LABEL2:.*]]
; MEMOP_OPT: ], !prof [[SWITCH_BW:![0-9]+]]
; MEMOP_OPT: [[CASE_1_LABEL2]]:
; MEMOP_OPT: call void @llvm.memcpy.p0i8.p0i8.i64(i8* %dst2, i8* %src2, i64 1, i1 false)
; MEMOP_OPT: call void @llvm.memcpy.p0i8.p0i8.i64(i8* %dst2, i8* %src2, i64 0, i1 false)
; MEMOP_OPT: br label %[[MERGE_LABEL2:.*]]
; MEMOP_OPT: [[DEFAULT_LABEL2]]:
; MEMOP_OPT: call void @llvm.memcpy.p0i8.p0i8.i64(i8* %dst2, i8* %src2, i64 %conv, i1 false), !prof [[NEWVP]]
@ -104,10 +104,10 @@ for.body3:
br label %for.inc
; MEMOP_OPT: switch i64 %conv, label %[[DEFAULT_LABEL:.*]] [
; MEMOP_OPT: i64 1, label %[[CASE_1_LABEL:.*]]
; MEMOP_OPT: i64 0, label %[[CASE_1_LABEL:.*]]
; MEMOP_OPT: ], !prof [[SWITCH_BW:![0-9]+]]
; MEMOP_OPT: [[CASE_1_LABEL]]:
; MEMOP_OPT: %[[RV:.*]] = call i32 @memcmp(i8* %dst, i8* %src, i64 1)
; MEMOP_OPT: %[[RV:.*]] = call i32 @memcmp(i8* %dst, i8* %src, i64 0)
; MEMOP_OPT: br label %[[MERGE_LABEL:.*]]
; MEMOP_OPT: [[DEFAULT_LABEL]]:
; MEMOP_OPT: %[[RVD:.*]] = call i32 @memcmp(i8* %dst, i8* %src, i64 %conv), !prof [[NEWVP:![0-9]+]]
@ -115,10 +115,10 @@ for.body3:
; MEMOP_OPT: [[MERGE_LABEL]]:
; MEMOP_OPT: %[[PHI:.*]] = phi i32 [ %[[RVD]], %[[DEFAULT_LABEL]] ], [ %[[RV]], %[[CASE_1_LABEL]] ]
; MEMOP_OPT: switch i64 %conv, label %[[DEFAULT_LABEL2:.*]] [
; MEMOP_OPT: i64 1, label %[[CASE_1_LABEL2:.*]]
; MEMOP_OPT: i64 0, label %[[CASE_1_LABEL2:.*]]
; MEMOP_OPT: ], !prof [[SWITCH_BW:![0-9]+]]
; MEMOP_OPT: [[CASE_1_LABEL2]]:
; MEMOP_OPT: %[[RV2:.*]] = call i32 @bcmp(i8* %dst2, i8* %src2, i64 1)
; MEMOP_OPT: %[[RV2:.*]] = call i32 @bcmp(i8* %dst2, i8* %src2, i64 0)
; MEMOP_OPT: br label %[[MERGE_LABEL2:.*]]
; MEMOP_OPT: [[DEFAULT_LABEL2]]:
; MEMOP_OPT: %[[RVD2:.*]] = call i32 @bcmp(i8* %dst2, i8* %src2, i64 %conv), !prof [[NEWVP]]
@ -182,8 +182,8 @@ for.end6:
!27 = !{!"function_entry_count", i64 1}
!28 = !{!"branch_weights", i32 20, i32 1}
!29 = !{!"branch_weights", i32 556, i32 20}
!30 = !{!"VP", i32 1, i64 556, i64 1, i64 99, i64 2, i64 88, i64 3, i64 77, i64 9, i64 72, i64 4, i64 66, i64 5, i64 55, i64 6, i64 44, i64 7, i64 33, i64 8, i64 22}
!31 = !{!"VP", i32 1, i64 556, i64 1, i64 99, i64 2, i64 88, i64 3, i64 77, i64 9, i64 72, i64 4, i64 66, i64 5, i64 55, i64 6, i64 44, i64 7, i64 33, i64 8, i64 22}
!30 = !{!"VP", i32 1, i64 556, i64 0, i64 99, i64 2, i64 88, i64 3, i64 77, i64 9, i64 72, i64 4, i64 66, i64 5, i64 55, i64 6, i64 44, i64 7, i64 33, i64 8, i64 22}
!31 = !{!"VP", i32 1, i64 556, i64 0, i64 99, i64 2, i64 88, i64 3, i64 77, i64 9, i64 72, i64 4, i64 66, i64 5, i64 55, i64 6, i64 44, i64 7, i64 33, i64 8, i64 22}
declare void @llvm.lifetime.start(i64, i8* nocapture)

View File

@ -0,0 +1,30 @@
# RUN: llvm-profdata merge --text -j 4 %s %s %s %s -o %t 2>&1 | FileCheck %s
# RUN: llvm-profdata merge --binary -j 4 %s %s %s %s -o %t 2>&1 | FileCheck %s
# IR level Instrumentation Flag
# CHECK: Invalid profile
:ir
foo
# Func Hash:
35277121310
# Num Counters:
3
# Counter Values:
20
556
1
# Num Value Kinds:
1
# ValueKind = IPVK_MemOPSize:
1
# NumValueSites:
1
9
0:99
0:88
3:77
9:72
4:66
5:55
6:44
7:33
8:22

View File

@ -54,6 +54,14 @@ static void warn(Twine Message, std::string Whence = "",
WithColor::note() << Hint << "\n";
}
static void warn(Error E, StringRef Whence = "") {
if (E.isA<InstrProfError>()) {
handleAllErrors(std::move(E), [&](const InstrProfError &IPE) {
warn(IPE.message(), std::string(Whence), std::string(""));
});
}
}
static void exitWithError(Twine Message, std::string Whence = "",
std::string Hint = "") {
WithColor::error();
@ -304,9 +312,10 @@ static void writeInstrProfile(StringRef OutputFilename,
if (OutputFormat == PF_Text) {
if (Error E = Writer.writeText(Output))
exitWithError(std::move(E));
warn(std::move(E));
} else {
Writer.write(Output);
if (Error E = Writer.write(Output))
warn(std::move(E));
}
}