Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[lldb][test] Fix inline_sites_live.cpp Shell when run on Linux remotely from Windows #115722

Merged
merged 1 commit into from
Nov 13, 2024

Conversation

dzhidzhoev
Copy link
Member

@dzhidzhoev dzhidzhoev commented Nov 11, 2024

This test fails on https://lab.llvm.org/staging/#/builders/197/builds/76/steps/18/logs/FAIL__lldb-shell__inline_sites_live_cpp because of a little difference in the lldb output.

# .---command stderr------------
# | C:\buildbot\as-builder-10\lldb-x-aarch64\llvm-project\lldb\test\Shell\SymbolFile\NativePDB\inline_sites_live.cpp:25:11: error: CHECK: expected string not found in input
# | // CHECK: * thread #1, stop reason = breakpoint 1
# |           ^
# | <stdin>:1:1: note: scanning from here
# | (lldb) platform select remote-linux
# | ^
# | <stdin>:28:27: note: possible intended match here
# | * thread #1, name = 'inline_sites_li', stop reason = breakpoint 1.3
# |                           ^
# | 

@llvmbot
Copy link
Member

llvmbot commented Nov 11, 2024

@llvm/pr-subscribers-lldb

Author: Vladislav Dzhidzhoev (dzhidzhoev)

Changes

Test fails on https://lab.llvm.org/staging/#/builders/197/builds/76 because of a little difference in format of output.


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

1 Files Affected:

  • (modified) lldb/test/Shell/SymbolFile/NativePDB/inline_sites_live.cpp (+2-2)
diff --git a/lldb/test/Shell/SymbolFile/NativePDB/inline_sites_live.cpp b/lldb/test/Shell/SymbolFile/NativePDB/inline_sites_live.cpp
index df6353e28303a3..549bc881b19bb7 100644
--- a/lldb/test/Shell/SymbolFile/NativePDB/inline_sites_live.cpp
+++ b/lldb/test/Shell/SymbolFile/NativePDB/inline_sites_live.cpp
@@ -22,11 +22,11 @@ int main(int argc, char** argv) {
   foo(argc);
 }
 
-// CHECK:      * thread #1, stop reason = breakpoint 1
+// CHECK:      * thread #1, {{.*}}stop reason = breakpoint 1
 // CHECK-NEXT:    frame #0: {{.*}}`main [inlined] bar(param=2)
 // CHECK:      (lldb) expression param
 // CHECK-NEXT: (int) $0 = 2
-// CHECK:      * thread #1, stop reason = breakpoint 2
+// CHECK:      * thread #1, {{.*}}stop reason = breakpoint 2
 // CHECK-NEXT:    frame #0: {{.*}}`main [inlined] foo(param=1)
 // CHECK:      (lldb) expression param
 // CHECK-NEXT: (int) $1 = 1

@labath
Copy link
Collaborator

labath commented Nov 12, 2024

How can you run a PDB test on a linux machine?

@DavidSpickett
Copy link
Collaborator

If I understand correctly this is building the test on Linux and running it on Windows.

And it seems that the output is in fact better this way? Can't complain about that.

Copy link
Collaborator

@DavidSpickett DavidSpickett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought maybe the test passed accidentally with this change but the backtrace must still match so LGTM.

@DavidSpickett DavidSpickett changed the title [lldb][test] Fix inline_sites_live.cpp Shell test on Linux remote [lldb][test] Fix inline_sites_live.cpp Shell when run on Windows remotely from Linux Nov 12, 2024
@labath
Copy link
Collaborator

labath commented Nov 13, 2024

That might make more sense, but looking at the bot, it really does seem to run on windows, with a remote linux target (I guess it doesn't really run the binary there, it must fall back to host when it find the binary is not compatible). And if I had to guess, I'd say that all of this platform dancing causes it to somehow switch the the lldb-server windows implementation, which sends thread names (unlike the in-process one?).

Overall, I guess the change is fine, though the combination is not particularly useful to test.

@dzhidzhoev dzhidzhoev merged commit 0afdac4 into llvm:main Nov 13, 2024
9 checks passed
@dzhidzhoev
Copy link
Member Author

If I understand correctly this is building the test on Linux and running it on Windows.

And it seems that the output is in fact better this way? Can't complain about that.

No, it's the opposite, we're running on Windows host and Linux target.

And if I had to guess, I'd say that all of this platform dancing causes it to somehow switch the the lldb-server windows implementation, which sends thread names (unlike the in-process one?).

It makes sense. I also thought it was somehow related to remote vs local execution.

Overall, I guess the change is fine, though the combination is not particularly useful to test.

Thank you! Considering that there's "REQUIRES: system-windows" on top of the test, maybe I'll turn it off for target-linux at all?

@dzhidzhoev dzhidzhoev changed the title [lldb][test] Fix inline_sites_live.cpp Shell when run on Windows remotely from Linux [lldb][test] Fix inline_sites_live.cpp Shell when run on Linux remotely from Windows Nov 13, 2024
@labath
Copy link
Collaborator

labath commented Nov 14, 2024

Considering that there's "REQUIRES: system-windows" on top of the test, maybe I'll turn it off for target-linux at all?

This problem is not really linux specific any os!=windows will have the same problem. I think REQUIRES: target-windows would be better.

@dzhidzhoev
Copy link
Member Author

Considering that there's "REQUIRES: system-windows" on top of the test, maybe I'll turn it off for target-linux at all?

This problem is not really linux specific any os!=windows will have the same problem. I think REQUIRES: target-windows would be better.

Thanks! I'll change it.

dzhidzhoev added a commit to dzhidzhoev/llvm-project that referenced this pull request Nov 14, 2024
This is a follow-up for the conversation here
llvm#115722.

This test is designed for Windows target/PDB format, so it shouldn't be
built and run for DWARF/etc.
dzhidzhoev added a commit that referenced this pull request Nov 15, 2024
…116196)

This is a follow-up for the conversation here
#115722.

This test is designed for Windows target/PDB format, so it shouldn't be
built and run for DWARF/etc.
akshayrdeodhar pushed a commit to akshayrdeodhar/llvm-project that referenced this pull request Nov 18, 2024
…tely from Linux (llvm#115722)

This test fails on
https://lab.llvm.org/staging/#/builders/197/builds/76/steps/18/logs/FAIL__lldb-shell__inline_sites_live_cpp
because of a little difference in the lldb output.

```
# .---command stderr------------
# | C:\buildbot\as-builder-10\lldb-x-aarch64\llvm-project\lldb\test\Shell\SymbolFile\NativePDB\inline_sites_live.cpp:25:11: error: CHECK: expected string not found in input
# | // CHECK: * thread llvm#1, stop reason = breakpoint 1
# |           ^
# | <stdin>:1:1: note: scanning from here
# | (lldb) platform select remote-linux
# | ^
# | <stdin>:28:27: note: possible intended match here
# | * thread llvm#1, name = 'inline_sites_li', stop reason = breakpoint 1.3
# |                           ^
# | 

```
akshayrdeodhar pushed a commit to akshayrdeodhar/llvm-project that referenced this pull request Nov 18, 2024
…lvm#116196)

This is a follow-up for the conversation here
llvm#115722.

This test is designed for Windows target/PDB format, so it shouldn't be
built and run for DWARF/etc.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants