Skip to content

Fix msvc-specific differences in debuginfo tests - #162412

Open
Walnut356 wants to merge 2 commits into
rust-lang:mainfrom
Walnut356:msvc_revision
Open

Fix msvc-specific differences in debuginfo tests#162412
Walnut356 wants to merge 2 commits into
rust-lang:mainfrom
Walnut356:msvc_revision

Conversation

@Walnut356

@Walnut356 Walnut356 commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

As part of #161657

Fixes ~every test that fails due to an msvc-specific problem. I tried to modify the lldb-check commands in-place where possible. If I couldn't, I used revisions.

There is 1 very tiny visualizer bug fix that I lumped in here that affects summaries of empty arrays on msvc (] -> [])

basic-types-globals.rs- msvc stores globals under the statics category, and LLDB-with-PDB-debug-info requires that those be access by a fully qualified name (which shouldn't pose any issue for dwarf debug info, which IIRC can access via the qualified or unqualified name). I enforced the crate name through compiler flags, so even if this file moves around or is renamed, it shouldn't break.

The above also applies to no_mangle-info.rs except, for some reason, non-msvc targets are only able to access the top level static through the unqualified path. I'm not 100% sure if this is an LLDB bug or not, but it's not a huge deal for us since we can just use revisions.

The only test that changed behaviorally is thread-names.rs, which now uses a script command to print all threads, and then compiletest can pick the 2 it cares about out of that list. On windows, the spawned thread isn't thread 2, since there's typically a bunch of ntdll.dll threads. Even for non-windows, relying on exact thread numbers seems pretty fragile so this should help all around.

After this patch, the remainder of the 22 test failures on windows-msvc are the <variable has been optimized out> issue and the variable shadowing issue. The variable shadowing is a bug on their end, but we can work around it. I need to look into the <variable has been optimized out> issue a bit more to see what's going on there.

r? @Kobzol , @jieyouxu


try-job: test-aarch64-msvc-1
try-job: test-x86_64-msvc-1
try-job: test-aarch64-gnu-debug
try-job: test-x86_64-mingw-1
try-job: test-aarch64-apple-1

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Sep 7, 2026
@rustbot

rustbot commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

Kobzol is not on the review rotation at the moment.
They may take a while to respond.

@Kobzol

Kobzol commented Sep 7, 2026

Copy link
Copy Markdown
Member

Do you perhaps want to move those tests to the new format first, to avoid having to modify these diffs by hand?

@Walnut356

Walnut356 commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

In the short term, I'm aiming to get windows-gnu and windows-msvc into a state where all the tests pass so we can actually run them in CI (and so that running them locally is useful). Once those are running in CI, we can transition the tests over on all targets at once. That way we don't have to disable them on windows-msvc in the interim, nor do we have blessed files that aren't being tested.

I forgot to mention it in the main comment, but this patch fixes the last failing test on windows-gnu (at least on my machine lol) so we can start testing that in CI whenever, we'd just need to do like the other patch and acquire LLDB on the windows runners. LLVM distributes via winget, and you can explicitly download a specific version, so it should be pretty much the same change as #161574

@jieyouxu jieyouxu self-assigned this Sep 8, 2026
@jieyouxu

jieyouxu commented Sep 9, 2026

Copy link
Copy Markdown
Member

@bors try

@rust-bors

This comment has been minimized.

rust-bors Bot pushed a commit that referenced this pull request Sep 9, 2026
Fix msvc-specific differences in debuginfo tests

try-job: test-aarch64-msvc-1
try-job: test-x86_64-msvc-1
try-job: test-aarch64-gnu-debug
try-job: test-x86_64-mingw-1
try-job: test-aarch64-apple-1

@jieyouxu jieyouxu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Changes look good. I'll try this locally.

View changes since this review

Comment on lines +23 to +24
// We use the python API to do this because `thread list` refuses to print anything inside the test
// harness for some reason.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Interesting 🤔

@jieyouxu

jieyouxu commented Sep 9, 2026

Copy link
Copy Markdown
Member

I tested this locally, yeah essentially remaining ones are shadowing / optimized out. dummy_span.rs just... has some weird span, and issue-22656.rs indeed has some ZST issues. Otherwise this seems good, nice.

Thanks
@bors r+ rollup=iffy note="debuginfo tests + visualizer fixes"

@rust-bors

rust-bors Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

📌 Commit ca69880 has been approved by jieyouxu

It is now in the queue for this repository.

@rust-bors rust-bors Bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 9, 2026
@rust-bors

rust-bors Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

☀️ Try build successful (CI)
Build commit: 41603dd (41603dd6c0292bce95ec154bf60c79061a082b0b)
Base parent: eca445e (eca445e5ae4a6679cc27d3a09106ce245e13a5a6)

jhpratt added a commit to jhpratt/rust that referenced this pull request Sep 9, 2026
Fix msvc-specific differences in debuginfo tests

As part of rust-lang#161657

Fixes ~every test that fails due to an msvc-specific problem. I tried to modify the `lldb-check` commands in-place where possible. If I couldn't, I used revisions.

There is 1 very tiny visualizer bug fix that I lumped in here that affects summaries of empty arrays on msvc (`]` -> `[]`)

`basic-types-globals.rs`- msvc stores globals under the `statics` category, and LLDB-with-PDB-debug-info requires that those be access by a fully qualified name (which shouldn't pose any issue for dwarf debug info, which IIRC can access via the qualified or unqualified name). I enforced the crate name through compiler flags, so even if this file moves around or is renamed, it shouldn't break.

The above also applies to `no_mangle-info.rs` except, for some reason, non-msvc targets are *only* able to access the top level static through the unqualified path. I'm not 100% sure if this is an LLDB bug or not, but it's not a huge deal for us since we can just use revisions.

The only test that changed behaviorally is `thread-names.rs`, which now uses a `script` command to print all threads, and then `compiletest` can pick the 2 it cares about out of that list. On windows, the spawned thread isn't thread 2, since there's typically a bunch of `ntdll.dll` threads. Even for non-windows, relying on exact thread numbers seems pretty fragile so this should help all around.

After this patch, the remainder of the 22 test failures on `windows-msvc` are the `<variable has been optimized out>` issue and the variable shadowing issue. The variable shadowing is a bug on their end, but we can work around it. I need to look into the `<variable has been optimized out>` issue a bit more to see what's going on there.

r? @Kobzol , @jieyouxu

---

try-job: test-aarch64-msvc-1
try-job: test-x86_64-msvc-1
try-job: test-aarch64-gnu-debug
try-job: test-x86_64-mingw-1
try-job: test-aarch64-apple-1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants