[llvm-profgen] Trim tail CR+LF for LBR record line

[HaohaiWen]

On Windows, perfscript generated by sep contains CR+LF at the end of
LBR records line. This '\r' will be treated as a LBR record when running
llvm-profgen on Linux and then generate warning.

[llvmbot]

@llvm/pr-subscribers-pgo

Author: Haohai Wen (HaohaiWen)

Changes

On Windows, perfscript generated by sep contains CR+LF at the end of
LBR records line. This '\r' will be treated as an LBR record and then
generate warning.

---

Full diff: https://github.com/llvm/llvm-project/pull/93210.diff

1 Files Affected:

• (modified) llvm/tools/llvm-profgen/PerfReader.cpp (+1-1)

diff --git a/llvm/tools/llvm-profgen/PerfReader.cpp b/llvm/tools/llvm-profgen/PerfReader.cpp
index e9442027aed3f..e63c6d61b3bfc 100644
--- a/llvm/tools/llvm-profgen/PerfReader.cpp
+++ b/llvm/tools/llvm-profgen/PerfReader.cpp
@@ -552,7 +552,7 @@ bool PerfScriptReader::extractLBRStack(TraceStream &TraceIt,
   //                           ... 0x4005c8/0x4005dc/P/-/-/0
   // It's in FIFO order and seperated by whitespace.
   SmallVector<StringRef, 32> Records;
-  TraceIt.getCurrentLine().split(Records, " ", -1, false);
+  TraceIt.getCurrentLine().rtrim().split(Records, " ", -1, false);
   auto WarnInvalidLBR = [](TraceStream &TraceIt) {
     WithColor::warning() << "Invalid address in LBR record at line "
                          << TraceIt.getLineNumber() << ": "

[WenleiHe]

LGTM, might be helpful to have a test case protecting against it since most of us don't have windows environment.

[HaohaiWen]

LGTM, might be helpful to have a test case protecting against it since most of us don't have windows environment.

I'm not going to add a test since rtrim() will not do anything bad.
The warning message only prints the whole line:
warning: Invalid address in LBR record at line xxx

The reason is if we run llvm-profgen on Linux with perfscript generated by sep on Windows, StringRef treats '\n' as EOL and '\r' as a normal char so that profgen try to parse '\r' as LBR record.
That would not be problem when running llvm-profgen on Windows.
See

llvm-project/llvm/include/llvm/ADT/StringRef.h

Line 84 in 06aadbe

/*implicit*/ constexpr StringRef(const char *Str)

[WenleiHe]

LGTM, might be helpful to have a test case protecting against it since most of us don't have windows environment.

I'm not going to add a test since rtrim() will not do anything bad. The warning message only prints the whole line: warning: Invalid address in LBR record at line xxx

The reason is if we run llvm-profgen on Linux with perfscript generated by sep on Windows, StringRef treats '\n' as EOL and '\r' as a normal char so that profgen try to parse '\r' as LBR record. That would not be problem when running llvm-profgen on Windows. See

llvm-project/llvm/include/llvm/ADT/StringRef.h

Line 84 in 06aadbe

/*implicit*/ constexpr StringRef(const char *Str)

It's fine if we don't have test case for this small change. By protecting, what I mean is making sure if someone removes the rtrim later on (likely unintentionally with refactoring etc.), something will fail.