[llvm-profgen] Ignore broken LBR samples

[hoy]

Summary

Perf script can sometimes give disordered LBR samples like below.

          b022500
          32de0044
          3386e1d1
      7f118e05720c
      7f118df2d81f
 0x2a0b9622/0x2a0b9610/P/-/-/1  0x2a0b79ff/0x2a0b9618/P/-/-/2  0x2a0b7a4a/0x2a0b79e8/P/-/-/1  0x2a0b7a33/0x2a0b7a46/P/-/-/1  0x2a0b7a42/0x2a0b7a23/P/-/-/1  0x2a0b7a21/0x2a0b7a37/P/-/-/2  0x2a0b79e6/0x2a0b7a07/P/-/-/1  0x2a0b79d4/0x2a0b79dc/P/-/-/2  0x2a0b7a03/0x2a0b79aa/P/-/-/1  0x2a0b79a8/0x2a0b7a00/P/-/-/234  0x2a0b9613/0x2a0b7930/P/-/-/1  0x2a0b9622/0x2a0b9610/P/-/-/1  0x2a0b79ff/0x2a0b9618/P/-/-/2  0x2a0b7a4a/0x2aWarning:
Processed 10263226 events and lost 1 chunks!

Note that the last LBR record 0x2a0b7a4a/0x2aWarning: . Currently llvm-profgen does not detect that and as a result an uninitialized branch target value will be used. The uninitialized value can cause creepy instruction ranges created which which in turn will result in a completely wrong profile. An example is like

.... @ _ZN5folly13loadUnalignedIsEET_PKv]:18446744073709551615:18446744073709551615
1: 18446744073709551615
!CFGChecksum: 4294967295
!Attributes: 0

[wlei]

llvm/tools/llvm-profgen/PerfReader.cpp

640–641

Please help to remove this line since it's already done in recordRangeCount.

llvm/tools/llvm-profgen/PerfReader.h

376

Nit: invalid --> Invalid , same for other assert message.

[hoy]

llvm/tools/llvm-profgen/PerfReader.cpp

640–641

Good catch. Assert removed.

llvm/tools/llvm-profgen/PerfReader.h

376

Fixed.

[hoy]

Addressing Lei's comments.

[hoy]

Updating D109637: [llvm-profgen] Ignore broken LBR samples

[wlei]

LGTM, thanks!

[wenlei]

llvm/tools/llvm-profgen/PerfReader.cpp

428

Print a warning so we know when this happens? Ideally the warning should also include the invalid Line TraceIt.getCurrentLine(), so we know what is the cause.

[wenlei]

Thanks for the fix. It's unfortunate that perf script does not support redirecting its output all to a file, and it always uses stderr for both output and warning.

[hoy]

llvm/tools/llvm-profgen/PerfReader.cpp

428

Sounds good. Warning added in place to be more specific about LBR.

[hoy]

Addressing Wenlei's comment.

[wenlei]

llvm/tools/llvm-profgen/PerfReader.cpp

430

Can we have the content of the line printed? I think that helps users know what is going on immediately. We can have both line number and content, but I think content is more important.
nit: "line" -> "line: " (add a white space between "line" and its content)

[hoy]

llvm/tools/llvm-profgen/PerfReader.cpp

430

Sounds good. Full line printed.

[hoy]

Printing full line content in warning.

[wenlei]

lgtm, thanks.

llvm/tools/llvm-profgen/PerfReader.h

 // which is represented as the start and end offset pair.
 using RangeSample = std::map<std::pair<uint64_t, uint64_t>, uint64_t>;
 // Wrapper for sample counters including range counter and branch counter
 struct SampleCounter {
   RangeSample RangeCounter;
   BranchSample BranchCounter;
 
   void recordRangeCount(uint64_t Start, uint64_t End, uint64_t Repeat) {
+    assert(Start <= End && "Invalid instruction range");
     RangeCounter[{Start, End}] += Repeat;
   }
   void recordBranchCount(uint64_t Source, uint64_t Target, uint64_t Repeat) {
     BranchCounter[{Source, Target}] += Repeat;
   }
 };
 
 // Sample counter with context to support context-sensitive profile

llvm/tools/llvm-profgen/PerfReader.cpp

while (Index < Records.size()) {
     auto &Token = Records[Index++];
     if (Token.size() == 0)
       continue;
 
     SmallVector<StringRef, 8> Addresses;
     Token.split(Addresses, "/");
     uint64_t Src;
     uint64_t Dst;
-    Addresses[0].substr(2).getAsInteger(16, Src);
-    Addresses[1].substr(2).getAsInteger(16, Dst);
+
+    // Stop at broken LBR records.
+    if (Addresses[0].substr(2).getAsInteger(16, Src) ||
+        Addresses[1].substr(2).getAsInteger(16, Dst))
+      break;
 
     bool SrcIsInternal = Binary->addressIsCode(Src);
     bool DstIsInternal = Binary->addressIsCode(Dst);
     bool IsExternal = !SrcIsInternal && !DstIsInternal;
     bool IsIncoming = !SrcIsInternal && DstIsInternal;
     bool IsOutgoing = SrcIsInternal && !DstIsInternal;
     bool IsArtificial = false;
 
     // Ignore branches outside the current binary.
for (const LBREntry &LBR : Sample->LBRStack) {
     uint64_t TargetOffset = Binary->virtualAddrToOffset(LBR.Target);
 
     if (!LBR.IsArtificial) {
       Counter.recordBranchCount(SourceOffset, TargetOffset, Repeat);
     }
 
     // If this not the first LBR, update the range count between TO of current
     // LBR and FROM of next LBR.
     uint64_t StartOffset = TargetOffset;
-    if (EndOffeset != 0) {
-      assert(StartOffset <= EndOffeset &&
-             "Bogus range should be filtered ealier!");
+    if (EndOffeset != 0)
       Counter.recordRangeCount(StartOffset, EndOffeset, Repeat);
-    }
     EndOffeset = SourceOffset;
   }
 }
 
 void LBRPerfReader::parseSample(TraceStream &TraceIt, uint64_t Count) {
   std::shared_ptr<PerfSample> Sample = std::make_shared<PerfSample>();
   // Parsing LBR stack and populate into PerfSample.LBRStack
   if (extractLBRStack(TraceIt, Sample->LBRStack)) {

llvm/tools/llvm-profgen/ProfileGenerator.cpp

void ProfileGenerator::findDisjointRanges(RangeSample &DisjointRanges,
   [B, B]:   300
   [B+1, C]: 200.
   */
   std::map<uint64_t, BoundaryPoint> Boundaries;
 
   for (auto Item : Ranges) {
     uint64_t Begin = Item.first.first;
     uint64_t End = Item.first.second;
+    assert(Begin <= End && "invalid instruction range");
     uint64_t Count = Item.second;
     if (Boundaries.find(Begin) == Boundaries.end())
       Boundaries[Begin] = BoundaryPoint();
     Boundaries[Begin].addBeginCount(Count);
 
     if (Boundaries.find(End) == Boundaries.end())
       Boundaries[End] = BoundaryPoint();
     Boundaries[End].addEndCount(Count);
   }
 
   uint64_t BeginAddress = UINT64_MAX;
-  int Count = 0;
+  uint64_t Count = 0;
   for (auto Item : Boundaries) {
     uint64_t Address = Item.first;
     BoundaryPoint &Point = Item.second;
     if (Point.BeginCount) {
       if (BeginAddress != UINT64_MAX)
         DisjointRanges[{BeginAddress, Address - 1}] = Count;
       Count += Point.BeginCount;
       BeginAddress = Address;
     }
     if (Point.EndCount) {
       assert((BeginAddress != UINT64_MAX) &&
              "First boundary point cannot be 'end' point");
       DisjointRanges[{BeginAddress, Address}] = Count;
+      assert(Count >= Point.EndCount && "mismatched live ranges");
       Count -= Point.EndCount;
       BeginAddress = Address + 1;
     }
   }
 }
 
 FunctionSamples &
 CSProfileGenerator::getFunctionProfileForContext(SampleContextFrames Context,