Skip to content

Support referencing stack slots in the DWARF debug info#6960

Merged
alexcrichton merged 5 commits into
bytecodealliance:mainfrom
SingleAccretion:DI-Vars
Sep 5, 2023
Merged

Support referencing stack slots in the DWARF debug info#6960
alexcrichton merged 5 commits into
bytecodealliance:mainfrom
SingleAccretion:DI-Vars

Conversation

@SingleAccretion
Copy link
Copy Markdown
Contributor

@SingleAccretion SingleAccretion commented Sep 4, 2023 •
edited
Loading

Implements the plan essentially outlined in #2856 (comment):

  1. Emit or use SysV-style unwind info if debug info is requested.
  2. Reference that unwind info in the form of .debug_frame using DW_OP_call_frame_cfa.
  3. Plumb the right offsets through.

Fixes #3884 (at least to some degree - however, that issue does not contain anything actionable), and also the example I was looking at in C#, though only for -opt-level 0 codegen.

@github-actions github-actions Bot added cranelift Issues related to the Cranelift code generator cranelift:area:machinst Issues related to instruction selection and the new MachInst backend. cranelift:area:aarch64 Issues related to AArch64 backend. cranelift:area:x64 Issues related to x64 codegen labels Sep 4, 2023
@SingleAccretion
Add support for specifying stack locations in debug info
d9a3e7f 
Always emit SysV-style CFI unwind info if we need debug info,
and reference it in the debug info using DW_OP_call_frame_cfa.
@SingleAccretion SingleAccretion marked this pull request as ready for review September 4, 2023 18:30
@SingleAccretion SingleAccretion requested review from a team as code owners September 4, 2023 18:30
@SingleAccretion SingleAccretion requested review from abrown and pchickey and removed request for a team September 4, 2023 18:30
@pchickey pchickey requested review from alexcrichton and removed request for pchickey September 4, 2023 20:49
alexcrichton
alexcrichton reviewed Sep 5, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@alexcrichton alexcrichton left a comment

Choose a reason for hiding this comment

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

Looks all reasonable to me, thanks again for working on this! I'm no expert myself in debuginfo but I don't think we necessarily have a resident expert to turn to, so happy to review at a somewhat higher level and lean on you for the DWARF correctness and all that.

One final comment I'd have in addition to those below is that our current debuginfo tests are pretty primitive, so if you're interested or so inspired I think it'd be great to soup them up. For example I'd love to have "just" a directory of C and/or Rust files which are compiled automatically in CI and the source itself has various special comments for commands/breakpoints/etc.

Comment thread cranelift/codegen/src/machinst/vcode.rs
Comment thread crates/cranelift/src/debug/write_debuginfo.rs
Comment thread tests/all/debug/testsuite/spilled_frame_base.wasm
@alexcrichton
Copy link
Copy Markdown
Member

alexcrichton commented Sep 5, 2023

Oh interesting! So there's no way for the dwarf sections to actually refer to .eh_frame? It's required at the binary level that the information is duplicated?

@SingleAccretion
Copy link
Copy Markdown
Contributor Author

SingleAccretion commented Sep 5, 2023

So there's no way for the dwarf sections to actually refer to .eh_frame? It's required at the binary level that the information is duplicated?

I am no great Unix expert myself, so do not know for certain. I suspect that it is indeed possible for the debugger to use .eh_frame alone, though the two sections are a little different in format details as well as usage (.eh_frame is like .pdata on Windows in that it is "mandatory").

FWIW, there are much larger (pun intended) size problems with -g-produced object files at the moment. For one, all the location lists for variables with are very fragmented, it would help a lot if they could always refer to the one and only stack location, at least for unoptimized code.

alexcrichton
alexcrichton approved these changes Sep 5, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@alexcrichton alexcrichton left a comment

Choose a reason for hiding this comment

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

Ok and no worries, I wanted to confirm for my own curiosity mostly. I think it's reasonable to basically incrementally improve things and any work in this space is very much appreciated!

@bjorn3
Copy link
Copy Markdown
Contributor

bjorn3 commented Sep 5, 2023

Rustc doesn't ever emit .debug_frame. The debugger has to use .eh_frame for this information for rust code.

@alexcrichton alexcrichton added this pull request to the merge queue Sep 5, 2023
Merged via the queue into bytecodealliance:main with commit 7b16ecc Sep 5, 2023
eduardomourar pushed a commit to eduardomourar/wasmtime that referenced this pull request Sep 6, 2023
@SingleAccretion @eduardomourar
Support referencing stack slots in the DWARF debug info (bytecodealli…
d4277e8 
…ance#6960)

* Add a test

* Disable test

* Add support for specifying stack locations in debug info

Always emit SysV-style CFI unwind info if we need debug info,
and reference it in the debug info using DW_OP_call_frame_cfa.

* Add toolchain comment to the test

* Add a comment and assert
@cfallin cfallin mentioned this pull request Oct 18, 2023
Open
@SingleAccretion SingleAccretion deleted the DI-Vars branch October 15, 2024 21:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alexcrichton alexcrichton alexcrichton approved these changes

@abrown abrown Awaiting requested review from abrown abrown is a code owner automatically assigned from bytecodealliance/wasmtime-compiler-reviewers

Assignees

No one assigned

Labels

cranelift:area:aarch64 Issues related to AArch64 backend. cranelift:area:machinst Issues related to instruction selection and the new MachInst backend. cranelift:area:x64 Issues related to x64 codegen cranelift Issues related to the Cranelift code generator

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Debugging module with lldb shows local variables as not available

3 participants

@SingleAccretion @alexcrichton @bjorn3