[PATCH] D95480: [NFC] Fix bug with parsing hybrid sample trace line

[Lei Wang via Phabricator via llvm-commits]

wlei added inline comments.


================
Comment at: llvm/tools/llvm-profgen/PerfReader.cpp:413
+      // skip it
       TraceIt.advance();
+      return false;
----------------
wenlei wrote:
> line 413 and line 416 can be hoisted to line 409?
This is tricky, because the underlying `TraceStream` read line by line .. you see in line 408 `StringRef FrameStr` which use StringRef, if we hoist `TraceIt.advance();`, the internal string instance will be freed. That means we must ensure to use the StringRef before call  `advance`. have the comments in  `TraceStream`.


================
Comment at: llvm/tools/llvm-profgen/PerfReader.cpp:414
       TraceIt.advance();
-      break;
+      return false;
     }
----------------
wenlei wrote:
> This change means we no longer handle a comment line mixed in stack sample? Although I'm not sure if we ever need a comment line.. 
> 
> 
yeah, I think so. For the `perf` generated perfscript, I saw the only case is a log line at the head of the file, shouldn't have comments in other place. Here the comments are most for the regression test, but as you see we have to make sure it's not mixed in the sample.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95480/new/

https://reviews.llvm.org/D95480