[MC] Fix accidentally eating the newline when handling a comment char at the end of the line.

[aemerson]

If we have a target where both # and ## are valid comment strings,
a line ending in # would trigger the lexer to eat 2 characters
and therefore lex the next line as a comment. Oops. This was introduced
in 4946db1

rdar://162635338

[aemerson]

• [MC] Fix accidentally eating the newline when handling a comment char at the end of the line. #165129 👈 (View in Graphite)
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[llvmbot]

@llvm/pr-subscribers-llvm-mc

Author: Amara Emerson (aemerson)

Changes

If we have a target where both # and ## are valid comment strings,
a line ending in # would trigger the lexer to eat 2 characters
and therefore lex the next line as a comment. Oops. This was introduced
in 4946db1

---

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

2 Files Affected:

• (modified) llvm/lib/MC/MCParser/AsmLexer.cpp (+8-1)
• (added) llvm/test/MC/AsmParser/comments-x86-darwin-eol-dropped.s (+10)

diff --git a/llvm/lib/MC/MCParser/AsmLexer.cpp b/llvm/lib/MC/MCParser/AsmLexer.cpp
index 968ccf776440b..8062ce88a154b 100644
--- a/llvm/lib/MC/MCParser/AsmLexer.cpp
+++ b/llvm/lib/MC/MCParser/AsmLexer.cpp
@@ -835,7 +835,14 @@ AsmToken AsmLexer::LexToken() {
   }
 
   if (isAtStartOfComment(TokStart)) {
-    CurPtr += MAI.getCommentString().size() - 1;
+    StringRef CommentString = MAI.getCommentString();
+    // For multi-char comment strings, advance CurPtr only if we matched the full
+    // string. This stops us from accidentally eating the newline if the current
+    // line ends in a single comment char.
+    if (CommentString.size() > 1 &&
+        StringRef(TokStart, CommentString.size()) == CommentString) {
+      CurPtr += CommentString.size() - 1;
+    }
     return LexLineComment();
   }
 
diff --git a/llvm/test/MC/AsmParser/comments-x86-darwin-eol-dropped.s b/llvm/test/MC/AsmParser/comments-x86-darwin-eol-dropped.s
new file mode 100644
index 0000000000000..662e5987db6e2
--- /dev/null
+++ b/llvm/test/MC/AsmParser/comments-x86-darwin-eol-dropped.s
@@ -0,0 +1,10 @@
+// RUN: llvm-mc -triple i386-apple-darwin %s 2>&1 | FileCheck %s
+.p2align 3
+// CHECK: .p2align 3
+test:
+// CHECK-LABEL: test:
+// CHECK: pushl %ebp
+// CHECK: movl %esp, %ebp
+# Check that the following line's comment # doesn't drop the movl after
+   pushl %ebp #
+   movl %esp, %ebp

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[aemerson]

Merge activity

• Oct 26, 5:49 AM UTC: A user started a stack merge that includes this pull request via Graphite.
• Oct 26, 5:51 AM UTC: @aemerson merged this pull request with Graphite.

[llvm-ci]

LLVM Buildbot has detected a new failure on builder ppc64le-flang-rhel-clang running on ppc64le-flang-rhel-test while building llvm at step 7 "test-build-unified-tree-check-flang-rt".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/157/builds/41925

Here is the relevant piece of the build log for the reference

Step 7 (test-build-unified-tree-check-flang-rt) failure: 1200 seconds without output running [b'ninja', b'check-flang-rt'], attempting to kill
...
21.605 [2/8/33] Building CXX object flang-rt/unittests/Runtime/CMakeFiles/RuntimeTests.dir/TemporaryStack.cpp.o
21.680 [2/7/34] Building CXX object flang-rt/unittests/Runtime/CMakeFiles/RuntimeTests.dir/NumericalFormatTest.cpp.o
22.556 [2/6/35] Linking CXX executable flang-rt/unittests/Evaluate/reshape.test
23.345 [2/5/36] Building CXX object flang-rt/unittests/Runtime/CMakeFiles/RuntimeTests.dir/Format.cpp.o
23.949 [2/4/37] Building CXX object flang-rt/unittests/Runtime/CMakeFiles/RuntimeTests.dir/Transformational.cpp.o
24.668 [2/3/38] Building CXX object flang-rt/unittests/Runtime/CMakeFiles/RuntimeTests.dir/CommandTest.cpp.o
32.435 [2/2/39] Building CXX object flang-rt/unittests/Runtime/CMakeFiles/RuntimeTests.dir/CharacterTest.cpp.o
49.628 [2/1/40] Building CXX object flang-rt/unittests/Runtime/CMakeFiles/RuntimeTests.dir/Reduction.cpp.o
53.064 [1/1/41] Linking CXX executable flang-rt/unittests/Runtime/RuntimeTests
53.064 [0/1/41] Running the Flang-RT regression tests
command timed out: 1200 seconds without output running [b'ninja', b'check-flang-rt'], attempting to kill
process killed by signal 9
program finished with exit code -1
elapsedTime=1262.145622