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 @@ -290,6 +290,7 @@ truncated, malformed, unknown_function, + invalid_merge, hash_mismatch, count_mismatch, counter_overflow, diff --git a/llvm/include/llvm/ProfileData/InstrProfWriter.h b/llvm/include/llvm/ProfileData/InstrProfWriter.h --- a/llvm/include/llvm/ProfileData/InstrProfWriter.h +++ b/llvm/include/llvm/ProfileData/InstrProfWriter.h @@ -64,11 +64,13 @@ function_ref 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 @@ void addRecord(StringRef Name, uint64_t Hash, InstrProfRecord &&I, uint64_t Weight, function_ref Warn); bool shouldEncodeData(const ProfilingData &PD); - void writeImpl(ProfOStream &OS); + + Error writeImpl(ProfOStream &OS); }; } // end namespace llvm 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 @@ -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 @@ return "Truncated profile data"; case instrprof_error::malformed: return "Malformed instrumentation profile data"; + case instrprof_error::invalid_merge: + return "Merge created invalid profile. 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: diff --git a/llvm/lib/ProfileData/InstrProfWriter.cpp b/llvm/lib/ProfileData/InstrProfWriter.cpp --- a/llvm/lib/ProfileData/InstrProfWriter.cpp +++ b/llvm/lib/ProfileData/InstrProfWriter.cpp @@ -285,7 +285,7 @@ TheSummary->setEntry(I, Res[I]); } -void InstrProfWriter::writeImpl(ProfOStream &OS) { +Error InstrProfWriter::writeImpl(ProfOStream &OS) { using namespace IndexedInstrProf; OnDiskChainedHashTableGenerator Generator; @@ -376,12 +376,19 @@ (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 InstrProfWriter::writeBuffer() { @@ -389,7 +396,8 @@ 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 @@ #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 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(instrprof_error::invalid_merge); + WasZero = true; + } + } + } + + return Error::success(); +} + void InstrProfWriter::writeRecordInText(StringRef Name, uint64_t Hash, const InstrProfRecord &Func, InstrProfSymtab &Symtab, @@ -473,5 +502,11 @@ 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(); } diff --git a/llvm/lib/Transforms/Instrumentation/PGOMemOPSizeOpt.cpp b/llvm/lib/Transforms/Instrumentation/PGOMemOPSizeOpt.cpp --- a/llvm/lib/Transforms/Instrumentation/PGOMemOPSizeOpt.cpp +++ b/llvm/lib/Transforms/Instrumentation/PGOMemOPSizeOpt.cpp @@ -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 @@ SmallVector CaseCounts; uint64_t MaxCount = 0; unsigned Version = 0; + uint64_t LastC = 1; // Default case is in the front -- save the slot here. CaseCounts.push_back(0); for (auto &VD : VDs) { @@ -354,6 +356,14 @@ if (!isProfitable(C, RemainCount)) break; + if (!V && !LastC) { + LLVM_DEBUG(dbgs() << "Invalid Profile Data: " + "Two consecutive zeros in MemOp value counts.\n"); + return false; + } + + LastC = V; + SizeIds.push_back(V); CaseCounts.push_back(C); if (C > MaxCount) diff --git a/llvm/test/Transforms/PGOProfile/Inputs/consecutive-zeros.proftext b/llvm/test/Transforms/PGOProfile/Inputs/consecutive-zeros.proftext new file mode 100644 --- /dev/null +++ b/llvm/test/Transforms/PGOProfile/Inputs/consecutive-zeros.proftext @@ -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 diff --git a/llvm/test/Transforms/PGOProfile/consecutive-zeros.ll b/llvm/test/Transforms/PGOProfile/consecutive-zeros.ll new file mode 100644 --- /dev/null +++ b/llvm/test/Transforms/PGOProfile/consecutive-zeros.ll @@ -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) diff --git a/llvm/test/tools/llvm-profdata/invalid-profile-gen-zeros.proftext b/llvm/test/tools/llvm-profdata/invalid-profile-gen-zeros.proftext new file mode 100644 --- /dev/null +++ b/llvm/test/tools/llvm-profdata/invalid-profile-gen-zeros.proftext @@ -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: Merge created invalid profile. Please file a bug at: +: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 diff --git a/llvm/tools/llvm-profdata/llvm-profdata.cpp b/llvm/tools/llvm-profdata/llvm-profdata.cpp --- a/llvm/tools/llvm-profdata/llvm-profdata.cpp +++ b/llvm/tools/llvm-profdata/llvm-profdata.cpp @@ -54,6 +54,14 @@ WithColor::note() << Hint << "\n"; } +static void warn(Error E, StringRef Whence = "") { + if (E.isA()) { + 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 @@ 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)); } }