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

Create more accurate debuginfo for vtables. #89597

Merged
merged 1 commit into from
Oct 11, 2021

Conversation

michaelwoerister
Copy link
Member

Before this PR all vtables would have the same name ("vtable") in debuginfo. Now they get an unambiguous name that identifies the implementing type and the trait that is being implemented.

This is only one of several possible improvements:

  • This PR describes vtables as arrays of *const u8 pointers. It would nice to describe them as structs where function pointer is represented by a field with a name indicative of the method it maps to. However, this requires coming up with a naming scheme that avoids clashes between methods with the same name (which is possible if the vtable contains multiple traits).
  • The PR does not update the debuginfo we generate for the vtable-pointer field in a fat dyn pointer. Right now there does not seem to be an easy way of getting ahold of a vtable-layout without also knowing the concrete self-type of a trait object.

r? @wesleywiser

@rust-highfive
Copy link
Collaborator

Some changes occured to rustc_codegen_gcc

cc @antoyo

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 6, 2021
Copy link
Contributor

@antoyo antoyo left a comment

Choose a reason for hiding this comment

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

The GCC part looks good to me.

@rust-log-analyzer

This comment has been minimized.

@michaelwoerister
Copy link
Member Author

Thanks for taking a look, @antoyo!

Comment on lines 2612 to 2618
// FIXME: We describe the vtable as an array of *const u8 pointers. The length of the array is
// correct - but we could create a more accurate description, e.g. by describing it
// as a struct where each field has a name that corresponds to the name of the method
// it points to.
// However, this is not entirely straightforward because there might be multiple
// methods with the same name if the vtable is for multiple traits. So for now we keep
// things simple instead of adding some ad-hoc disambiguation scheme.
Copy link
Member

Choose a reason for hiding this comment

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

Should these perhaps be void/() pointers instead of u8 pointers? That would probably be less misleading in the debugger than showing whatever the first byte of the function happens to be.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done. However, I noticed that we print () as tuple$<> on MSVC, which looks a bit weird. Should we change that to void? I'm not sure how special void is as a type in C/C++ and therefore could cause trouble.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm that is weird. I'll put a bug in for that and maybe fix that next week.

Before this commit all vtables would have the same name "vtable" in
debuginfo. Now they get a name that identifies the implementing type
and the trait that is being implemented.
@wesleywiser
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Oct 9, 2021

📌 Commit 61c5a6d has been approved by wesleywiser

@bors bors 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 Oct 9, 2021
@camelid camelid added A-debuginfo Area: Debugging information in compiled programs (DWARF, PDB, etc.) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 11, 2021
@bors
Copy link
Contributor

bors commented Oct 11, 2021

⌛ Testing commit 61c5a6d with merge 9a75781...

@bors
Copy link
Contributor

bors commented Oct 11, 2021

☀️ Test successful - checks-actions
Approved by: wesleywiser
Pushing 9a75781 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Oct 11, 2021
@bors bors merged commit 9a75781 into rust-lang:master Oct 11, 2021
@rustbot rustbot added this to the 1.57.0 milestone Oct 11, 2021
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (9a75781): comparison url.

Summary: This benchmark run did not return any relevant changes.

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

wip-sync pushed a commit to NetBSD/pkgsrc-wip that referenced this pull request Dec 6, 2021
Pkgsrc changes:
 * Adapt a couple of patches

Upstream changes:

Version 1.57.0 (2021-12-02)
==========================

Language
--------

- [Macro attributes may follow `#[derive]` and will see the original
  (pre-`cfg`) input.][87220]
- [Accept curly-brace macros in expressions, like `m!{ .. }.method()`
  and `m!{ .. }?`.][88690]
- [Allow panicking in constant evaluation.][89508]

Compiler
--------

- [Create more accurate debuginfo for vtables.][89597]
- [Add `armv6k-nintendo-3ds` at Tier 3\*.][88529]
- [Add `armv7-unknown-linux-uclibceabihf` at Tier 3\*.][88952]
- [Add `m68k-unknown-linux-gnu` at Tier 3\*.][88321]
- [Add SOLID targets at Tier 3\*:][86191] `aarch64-kmc-solid_asp3`,
  `armv7a-kmc-solid_asp3-eabi`, `armv7a-kmc-solid_asp3-eabihf`

\* Refer to Rust's [platform support page][platform-support-doc] for more
   information on Rust's tiered platform support.

Libraries
---------

- [Avoid allocations and copying in `Vec::leak`][89337]
- [Add `#[repr(i8)]` to `Ordering`][89507]
- [Optimize `File::read_to_end` and `read_to_string`][89582]
- [Update to Unicode 14.0][89614]
- [Many more functions are marked `#[must_use]`][89692], producing a warning
  when ignoring their return value. This helps catch mistakes such as expecting
  a function to mutate a value in place rather than return a new value.

Stabilised APIs
---------------

- [`[T; N]::as_mut_slice`][`array::as_mut_slice`]
- [`[T; N]::as_slice`][`array::as_slice`]
- [`collections::TryReserveError`]
- [`HashMap::try_reserve`]
- [`HashSet::try_reserve`]
- [`String::try_reserve`]
- [`String::try_reserve_exact`]
- [`Vec::try_reserve`]
- [`Vec::try_reserve_exact`]
- [`VecDeque::try_reserve`]
- [`VecDeque::try_reserve_exact`]
- [`Iterator::map_while`]
- [`iter::MapWhile`]
- [`proc_macro::is_available`]
- [`Command::get_program`]
- [`Command::get_args`]
- [`Command::get_envs`]
- [`Command::get_current_dir`]
- [`CommandArgs`]
- [`CommandEnvs`]

These APIs are now usable in const contexts:

- [`hint::unreachable_unchecked`]

Cargo
-----

- [Stabilize custom profiles][cargo/9943]

Compatibility notes
-------------------

Internal changes
----------------
These changes provide no direct user facing benefits, but represent significant
improvements to the internals and overall performance of rustc
and related tools.

- [Added an experimental backend for codegen with `libgccjit`.][87260]

[86191]: rust-lang/rust#86191
[87220]: rust-lang/rust#87220
[87260]: rust-lang/rust#87260
[88243]: rust-lang/rust#88243
[88321]: rust-lang/rust#88321
[88529]: rust-lang/rust#88529
[88690]: rust-lang/rust#88690
[88952]: rust-lang/rust#88952
[89337]: rust-lang/rust#89337
[89507]: rust-lang/rust#89507
[89508]: rust-lang/rust#89508
[89582]: rust-lang/rust#89582
[89597]: rust-lang/rust#89597
[89614]: rust-lang/rust#89614
[89692]: rust-lang/rust#89692
[cargo/9943]: rust-lang/cargo#9943
[`array::as_mut_slice`]: https://doc.rust-lang.org/std/primitive.array.html#method.as_mut_slice
[`array::as_slice`]: https://doc.rust-lang.org/std/primitive.array.html#method.as_slice
[`collections::TryReserveError`]: https://doc.rust-lang.org/std/collections/struct.TryReserveError.html
[`HashMap::try_reserve`]: https://doc.rust-lang.org/std/collections/hash_map/struct.HashMap.html#method.try_reserve
[`HashSet::try_reserve`]: https://doc.rust-lang.org/std/collections/hash_set/struct.HashSet.html#method.try_reserve
[`String::try_reserve`]: https://doc.rust-lang.org/alloc/string/struct.String.html#method.try_reserve
[`String::try_reserve_exact`]: https://doc.rust-lang.org/alloc/string/struct.String.html#method.try_reserve_exact
[`Vec::try_reserve`]: https://doc.rust-lang.org/std/vec/struct.Vec.html#method.try_reserve
[`Vec::try_reserve_exact`]: https://doc.rust-lang.org/std/vec/struct.Vec.html#method.try_reserve_exact
[`VecDeque::try_reserve`]: https://doc.rust-lang.org/std/collections/struct.VecDeque.html#method.try_reserve
[`VecDeque::try_reserve_exact`]: https://doc.rust-lang.org/std/collections/struct.VecDeque.html#method.try_reserve_exact
[`Iterator::map_while`]: https://doc.rust-lang.org/std/iter/trait.Iterator.html#method.map_while
[`iter::MapWhile`]: https://doc.rust-lang.org/std/iter/struct.MapWhile.html
[`proc_macro::is_available`]: https://doc.rust-lang.org/proc_macro/fn.is_available.html
[`Command::get_program`]: https://doc.rust-lang.org/std/process/struct.Command.html#method.get_program
[`Command::get_args`]: https://doc.rust-lang.org/std/process/struct.Command.html#method.get_args
[`Command::get_envs`]: https://doc.rust-lang.org/std/process/struct.Command.html#method.get_envs
[`Command::get_current_dir`]: https://doc.rust-lang.org/std/process/struct.Command.html#method.get_current_dir
[`CommandArgs`]: https://doc.rust-lang.org/std/process/struct.CommandArgs.html
[`CommandEnvs`]: https://doc.rust-lang.org/std/process/struct.CommandEnvs.html
bjorn3 pushed a commit to bjorn3/rust that referenced this pull request Dec 31, 2021
…info, r=wesleywiser

Create more accurate debuginfo for vtables.

Before this PR all vtables would have the same name (`"vtable"`) in debuginfo. Now they get an unambiguous name that identifies the implementing type and the trait that is being implemented.

This is only one of several possible improvements:
- This PR describes vtables as arrays of `*const u8` pointers. It would nice to describe them as structs where function pointer is represented by a field with a name indicative of the method it maps to. However, this requires coming up with a naming scheme that avoids clashes between methods with the same name (which is possible if the vtable contains multiple traits).
- The PR does not update the debuginfo we generate for the vtable-pointer field in a fat `dyn` pointer. Right now there does not seem to be an easy way of getting ahold of a vtable-layout without also knowing the concrete self-type of a trait object.

r? `@wesleywiser`
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Jan 22, 2022
Pkgsrc changes:
 * Adjust line numbers in a number of patches
 * remove the --disable-dist-src option, so that we produce
   the rust-src rust component, which we upload to LOCALSRC
   to allow the rust-src package to build, which is needed
   for rust-analyzer.
 * Cargo checksum for vendor/cc no longer needs patching;
   checksum for vendor/libc updated

Upstream changes:

Version 1.57.0 (2021-12-02)
==========================

Language
--------

- [Macro attributes may follow `#[derive]` and will see the original
  (pre-`cfg`) input.][87220]
- [Accept curly-brace macros in expressions, like `m!{ .. }.method()`
  and `m!{ .. }?`.][88690]
- [Allow panicking in constant evaluation.][89508]

Compiler
--------

- [Create more accurate debuginfo for vtables.][89597]
- [Add `armv6k-nintendo-3ds` at Tier 3\*.][88529]
- [Add `armv7-unknown-linux-uclibceabihf` at Tier 3\*.][88952]
- [Add `m68k-unknown-linux-gnu` at Tier 3\*.][88321]
- [Add SOLID targets at Tier 3\*:][86191] `aarch64-kmc-solid_asp3`,
  `armv7a-kmc-solid_asp3-eabi`, `armv7a-kmc-solid_asp3-eabihf`

\* Refer to Rust's [platform support page][platform-support-doc] for more
   information on Rust's tiered platform support.

Libraries
---------

- [Avoid allocations and copying in `Vec::leak`][89337]
- [Add `#[repr(i8)]` to `Ordering`][89507]
- [Optimize `File::read_to_end` and `read_to_string`][89582]
- [Update to Unicode 14.0][89614]
- [Many more functions are marked `#[must_use]`][89692], producing a warning
  when ignoring their return value. This helps catch mistakes such as expecting
  a function to mutate a value in place rather than return a new value.

Stabilised APIs
---------------

- [`[T; N]::as_mut_slice`][`array::as_mut_slice`]
- [`[T; N]::as_slice`][`array::as_slice`]
- [`collections::TryReserveError`]
- [`HashMap::try_reserve`]
- [`HashSet::try_reserve`]
- [`String::try_reserve`]
- [`String::try_reserve_exact`]
- [`Vec::try_reserve`]
- [`Vec::try_reserve_exact`]
- [`VecDeque::try_reserve`]
- [`VecDeque::try_reserve_exact`]
- [`Iterator::map_while`]
- [`iter::MapWhile`]
- [`proc_macro::is_available`]
- [`Command::get_program`]
- [`Command::get_args`]
- [`Command::get_envs`]
- [`Command::get_current_dir`]
- [`CommandArgs`]
- [`CommandEnvs`]

These APIs are now usable in const contexts:

- [`hint::unreachable_unchecked`]

Cargo
-----

- [Stabilize custom profiles][cargo/9943]

Compatibility notes
-------------------

Internal changes
----------------
These changes provide no direct user facing benefits, but represent significant
improvements to the internals and overall performance of rustc
and related tools.

- [Added an experimental backend for codegen with `libgccjit`.][87260]

[86191]: rust-lang/rust#86191
[87220]: rust-lang/rust#87220
[87260]: rust-lang/rust#87260
[88243]: rust-lang/rust#88243
[88321]: rust-lang/rust#88321
[88529]: rust-lang/rust#88529
[88690]: rust-lang/rust#88690
[88952]: rust-lang/rust#88952
[89337]: rust-lang/rust#89337
[89507]: rust-lang/rust#89507
[89508]: rust-lang/rust#89508
[89582]: rust-lang/rust#89582
[89597]: rust-lang/rust#89597
[89614]: rust-lang/rust#89614
[89692]: rust-lang/rust#89692
[cargo/9943]: rust-lang/cargo#9943
[`array::as_mut_slice`]: https://doc.rust-lang.org/std/primitive.array.html#method.as_mut_slice
[`array::as_slice`]: https://doc.rust-lang.org/std/primitive.array.html#method.as_slice
[`collections::TryReserveError`]: https://doc.rust-lang.org/std/collections/struct.TryReserveError.html
[`HashMap::try_reserve`]: https://doc.rust-lang.org/std/collections/hash_map/struct.HashMap.html#method.try_reserve
[`HashSet::try_reserve`]: https://doc.rust-lang.org/std/collections/hash_set/struct.HashSet.html#method.try_reserve
[`String::try_reserve`]: https://doc.rust-lang.org/alloc/string/struct.String.html#method.try_reserve
[`String::try_reserve_exact`]: https://doc.rust-lang.org/alloc/string/struct.String.html#method.try_reserve_exact
[`Vec::try_reserve`]: https://doc.rust-lang.org/std/vec/struct.Vec.html#method.try_reserve
[`Vec::try_reserve_exact`]: https://doc.rust-lang.org/std/vec/struct.Vec.html#method.try_reserve_exact
[`VecDeque::try_reserve`]: https://doc.rust-lang.org/std/collections/struct.VecDeque.html#method.try_reserve
[`VecDeque::try_reserve_exact`]: https://doc.rust-lang.org/std/collections/struct.VecDeque.html#method.try_reserve_exact
[`Iterator::map_while`]: https://doc.rust-lang.org/std/iter/trait.Iterator.html#method.map_while
[`iter::MapWhile`]: https://doc.rust-lang.org/std/iter/struct.MapWhile.html
[`proc_macro::is_available`]: https://doc.rust-lang.org/proc_macro/fn.is_available.html
[`Command::get_program`]: https://doc.rust-lang.org/std/process/struct.Command.html#method.get_program
[`Command::get_args`]: https://doc.rust-lang.org/std/process/struct.Command.html#method.get_args
[`Command::get_envs`]: https://doc.rust-lang.org/std/process/struct.Command.html#method.get_envs
[`Command::get_current_dir`]: https://doc.rust-lang.org/std/process/struct.Command.html#method.get_current_dir
[`CommandArgs`]: https://doc.rust-lang.org/std/process/struct.CommandArgs.html
[`CommandEnvs`]: https://doc.rust-lang.org/std/process/struct.CommandEnvs.html
michaelwoerister added a commit to michaelwoerister/rust that referenced this pull request Feb 3, 2022
…accidentally been

removed in rust-lang#89597.

Also describe vtables as structs with a field for each entry.
michaelwoerister added a commit to michaelwoerister/rust that referenced this pull request Feb 7, 2022
…accidentally been

removed in rust-lang#89597.

Also describe vtables as structs with a field for each entry.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 9, 2022
…debuginfo-regression, r=wesleywiser

debuginfo: Fix DW_AT_containing_type vtable debuginfo regression

This PR brings back the `DW_AT_containing_type` attribute for vtables after it has accidentally been removed in rust-lang#89597.

It also implements a more accurate description of vtables. Instead of describing them as an array of void pointers, the compiler will now emit a struct type description with a field for each entry of the vtable.

r? `@wesleywiser`

This PR should fix issue rust-lang#93164.
~~The PR is blocked on rust-lang#93154 because both of them modify the `codegen/debug-vtable.rs` test case.~~
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 9, 2022
…debuginfo-regression, r=wesleywiser

debuginfo: Fix DW_AT_containing_type vtable debuginfo regression

This PR brings back the `DW_AT_containing_type` attribute for vtables after it has accidentally been removed in rust-lang#89597.

It also implements a more accurate description of vtables. Instead of describing them as an array of void pointers, the compiler will now emit a struct type description with a field for each entry of the vtable.

r? ``@wesleywiser``

This PR should fix issue rust-lang#93164.
~~The PR is blocked on rust-lang#93154 because both of them modify the `codegen/debug-vtable.rs` test case.~~
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-debuginfo Area: Debugging information in compiled programs (DWARF, PDB, etc.) merged-by-bors This PR was explicitly merged by bors. 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.

9 participants