Skip to content

Use #[repr(C)] on debuginfo test structs - #162364

Merged
rust-bors[bot] merged 1 commit into
rust-lang:mainfrom
Walnut356:debuginfo_repr
Sep 6, 2026
Merged

Use #[repr(C)] on debuginfo test structs#162364
rust-bors[bot] merged 1 commit into
rust-lang:mainfrom
Walnut356:debuginfo_repr

Conversation

@Walnut356

@Walnut356 Walnut356 commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

This fixes a couple of issues.

  • LLDB reading PDB debug info re-orders fields and displays them in offset order instead of source-order like DWARF. While this has been fixed upstream, it'll be quite a while before it's reflected in CI runners and such
  • For all targets, the structs were often written (and named) under the assumption that they would have specific layouts. Nothing was actually enforcing those assumed layouts, so very often a type would be named e.g. HasInternalPadding but rust would reorder it such that no internal padding existed.

struct-with-destructor.rs is still iffy. I'll test it when I get home and can update my main PC's LLDB.

On my remote PC (lldb 22.1.2) it didn't fix the test, and it looked like the variable was straight up being read incorrectly:

// expected:
(struct_with_destructor::NestedOuter) nested = {a:{a:{x:7890, y:9870}}}
// got:
(struct_with_destructor::NestedOuter) nested = {a:{a:{y:1378684509906, x:9870}}

On my laptop when manually inspecting it (CodeLLDB which uses 22.1.8 under the hood) everything looked fine, so it might have been a bug that LLDB patched.

Part of the MSVC test fixes for #161657

r? @Kobzol, @jieyouxu


test-jobs: aarch64-apple-1
test-jobs: aarch64-msvc-1
test-jobs: x86_64-msvc-1
test-jobs: x86_64-mingw-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 6, 2026
@rustbot

rustbot commented Sep 6, 2026

Copy link
Copy Markdown
Collaborator

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

@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.

Yeah thanks, this definitely makes sense. I guess I'm more surprised that not more tests are failing due to assumed layouts.
r? me
You can r=me once the try jobs come back green.

View changes since this review

@rustbot rustbot assigned jieyouxu and unassigned Kobzol Sep 6, 2026
@jieyouxu

jieyouxu commented Sep 6, 2026

Copy link
Copy Markdown
Member

@bors delegate+
@bors try

@rust-bors

rust-bors Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

✌️ @Walnut356, you can now approve this pull request!

If @jieyouxu told you to "r=me" after making some further change, then please make that change and post @bors r=jieyouxu.

View changes since this delegation.

@rust-bors

This comment has been minimized.

rust-bors Bot pushed a commit that referenced this pull request Sep 6, 2026
Use `#[repr(C)]` on debuginfo test structs
@rust-bors

rust-bors Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

☀️ Try build successful (CI)
Build commit: e712a43 (e712a43793a5be62fafc25ae9f8da2fad1ca9a31)
Base parent: 46a266b (46a266b9fb0fb75e05fa4c9132f7355fcb62d4a4)

@jieyouxu

jieyouxu commented Sep 6, 2026

Copy link
Copy Markdown
Member

@bors r+

@rust-bors

rust-bors Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

📌 Commit 81fcec3 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 6, 2026
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Sep 6, 2026
Use `#[repr(C)]` on debuginfo test structs

This fixes a couple of issues.

* LLDB reading PDB debug info re-orders fields and displays them in offset order instead of source-order like DWARF. While this has been [fixed upstream](llvm/llvm-project#218731), it'll be quite a while before it's reflected in CI runners and such
* For all targets, the structs were often written (and named) under the assumption that they would have specific layouts. Nothing was actually enforcing those assumed layouts, so very often a type would be named e.g. `HasInternalPadding` but rust would reorder it such that no internal padding existed.

`struct-with-destructor.rs` is still iffy. I'll test it when I get home and can update my main PC's LLDB.

On my remote PC (lldb 22.1.2) it didn't fix the test, and it looked like the variable was straight up being read incorrectly:

 ```
// expected:
 (struct_with_destructor::NestedOuter) nested = {a:{a:{x:7890, y:9870}}}
// got:
 (struct_with_destructor::NestedOuter) nested = {a:{a:{y:1378684509906, x:9870}}
 ```
On my laptop when manually inspecting it (CodeLLDB which uses 22.1.8 under the hood) everything looked fine, so it might have been a bug that LLDB patched.

Part of the MSVC test fixes for rust-lang#161657

r? @Kobzol, @jieyouxu

---

test-jobs: aarch64-apple-1
test-jobs: aarch64-msvc-1
test-jobs: x86_64-msvc-1
test-jobs: x86_64-mingw-1
rust-bors Bot pushed a commit that referenced this pull request Sep 6, 2026
…uwer

Rollup of 5 pull requests

Successful merges:

 - #159541 (Use query for Variant InhabitedPredicate)
 - #162364 (Use `#[repr(C)]` on debuginfo test structs)
 - #162215 (remove stale/duplicate tests)
 - #162335 (Add several new LLDB feature flags)
 - #162341 (add regression test for packus_epi16 issue)
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Sep 6, 2026
Use `#[repr(C)]` on debuginfo test structs

This fixes a couple of issues.

* LLDB reading PDB debug info re-orders fields and displays them in offset order instead of source-order like DWARF. While this has been [fixed upstream](llvm/llvm-project#218731), it'll be quite a while before it's reflected in CI runners and such
* For all targets, the structs were often written (and named) under the assumption that they would have specific layouts. Nothing was actually enforcing those assumed layouts, so very often a type would be named e.g. `HasInternalPadding` but rust would reorder it such that no internal padding existed.

`struct-with-destructor.rs` is still iffy. I'll test it when I get home and can update my main PC's LLDB.

On my remote PC (lldb 22.1.2) it didn't fix the test, and it looked like the variable was straight up being read incorrectly:

 ```
// expected:
 (struct_with_destructor::NestedOuter) nested = {a:{a:{x:7890, y:9870}}}
// got:
 (struct_with_destructor::NestedOuter) nested = {a:{a:{y:1378684509906, x:9870}}
 ```
On my laptop when manually inspecting it (CodeLLDB which uses 22.1.8 under the hood) everything looked fine, so it might have been a bug that LLDB patched.

Part of the MSVC test fixes for rust-lang#161657

r? @Kobzol, @jieyouxu

---

test-jobs: aarch64-apple-1
test-jobs: aarch64-msvc-1
test-jobs: x86_64-msvc-1
test-jobs: x86_64-mingw-1
rust-bors Bot pushed a commit that referenced this pull request Sep 6, 2026
…uwer

Rollup of 7 pull requests

Successful merges:

 - #162358 (Force u8/i8 numeric formatting on LLDB)
 - #162359 (Use `lldb.eTypeOptionHideChildren` for msvc tuples)
 - #162364 (Use `#[repr(C)]` on debuginfo test structs)
 - #162215 (remove stale/duplicate tests)
 - #162335 (Add several new LLDB feature flags)
 - #162341 (add regression test for packus_epi16 issue)
 - #162374 (Make `VaArgSafe` dyn-incompatible)
@JonathanBrouwer

Copy link
Copy Markdown
Member

@bors try jobs=x86_64-mingw-1,x86_64-msvc-1

@rust-bors

rust-bors Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

⌛ Trying commit 81fcec3 with merge 115f425

To cancel the try build, run the command @bors try cancel.

Workflow: https://github.com/rust-lang/rust/actions/runs/34051488529

rust-bors Bot pushed a commit that referenced this pull request Sep 6, 2026
Use `#[repr(C)]` on debuginfo test structs


try-job: x86_64-mingw-1
try-job: x86_64-msvc-1
@JonathanBrouwer

Copy link
Copy Markdown
Member

@bors try cancel

@rust-bors

rust-bors Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Try build cancelled. Cancelled workflows:

Hint: if you want to run another try build, you do not need to manually cancel the previous one. Just run @bors try and bors will cancel the previous build automatically.

rust-bors Bot pushed a commit that referenced this pull request Sep 6, 2026
…uwer

Rollup of 5 pull requests

Successful merges:

 - #162358 (Force u8/i8 numeric formatting on LLDB)
 - #162359 (Use `lldb.eTypeOptionHideChildren` for msvc tuples)
 - #162364 (Use `#[repr(C)]` on debuginfo test structs)
 - #162215 (remove stale/duplicate tests)
 - #162335 (Add several new LLDB feature flags)
@rust-bors
rust-bors Bot merged commit 2a91880 into rust-lang:main Sep 6, 2026
13 of 14 checks passed
@rustbot rustbot added this to the 1.100.0 milestone Sep 6, 2026
rust-bors Bot pushed a commit that referenced this pull request Sep 6, 2026
Rollup merge of #162364 - Walnut356:debuginfo_repr, r=jieyouxu

Use `#[repr(C)]` on debuginfo test structs

This fixes a couple of issues.

* LLDB reading PDB debug info re-orders fields and displays them in offset order instead of source-order like DWARF. While this has been [fixed upstream](llvm/llvm-project#218731), it'll be quite a while before it's reflected in CI runners and such
* For all targets, the structs were often written (and named) under the assumption that they would have specific layouts. Nothing was actually enforcing those assumed layouts, so very often a type would be named e.g. `HasInternalPadding` but rust would reorder it such that no internal padding existed.

`struct-with-destructor.rs` is still iffy. I'll test it when I get home and can update my main PC's LLDB.

On my remote PC (lldb 22.1.2) it didn't fix the test, and it looked like the variable was straight up being read incorrectly:

 ```
// expected:
 (struct_with_destructor::NestedOuter) nested = {a:{a:{x:7890, y:9870}}}
// got:
 (struct_with_destructor::NestedOuter) nested = {a:{a:{y:1378684509906, x:9870}}
 ```
On my laptop when manually inspecting it (CodeLLDB which uses 22.1.8 under the hood) everything looked fine, so it might have been a bug that LLDB patched.

Part of the MSVC test fixes for #161657

r? @Kobzol, @jieyouxu

---

test-jobs: aarch64-apple-1
test-jobs: aarch64-msvc-1
test-jobs: x86_64-msvc-1
test-jobs: x86_64-mingw-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.

5 participants