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

Coredump: add local/stack value typing #199

Merged
merged 1 commit into from
Feb 16, 2023

Conversation

xtuc
Copy link
Contributor

@xtuc xtuc commented Feb 8, 2023

This change adds typing to local and stack values. Technically the type
isn't strictly necessary as DWARF should indicate how to interpret the values.

However, using types allows us to represent the lack of value, because
it has been optimized out and more complex values like reference types.

Closes #198

Coredump.md Outdated Show resolved Hide resolved
This change adds typing to local and stack values. Technically the type
isn't strictly necessary as DWARF should indicate how to interpret the values.

However, using types allows us to represent the lack of value, because
it has been optimized out and more complex values like reference types.

Closes WebAssembly#198

This change also updates links after switching to a new repo.
@xtuc xtuc force-pushed the sven/type-local-stack-values branch from 4a8587e to 6fc92e1 Compare February 9, 2023 11:58
Copy link
Collaborator

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

Seems reasonable to me!

As some unrelated comments to this PR itself:

  • What is codeoffset relative to? The start of the wasm file or the start of the code section?
  • Right now there's a reserved:u32 at the end of a frame but at least elsewhere in the binary encoding for wasm this is typically done with a prefix byte rather than a postfix byte. That way the current frame would be 0x00 codeoffset:u32 ... and that 0x00 could be reinterpreted as flags in the future for more fields or as a discriminant for an alternative way to specify a frame.

@xtuc
Copy link
Contributor Author

xtuc commented Feb 9, 2023

What is codeoffset relative to?

the start of the code section, as specified by https://yurydelendik.github.io/webassembly-dwarf/#pc. I sometimes found it to be unreliable, I'm using the funcidx as codeoffset for my experiments.

Right now there's a reserved:u32 at the end of a frame but at least elsewhere in the binary encoding for wasm this is typically done with a prefix byte rather than a postfix byte

Yeah that's a good point. It's inconsistent and I'll fix it. Thanks

@alexcrichton
Copy link
Collaborator

FWIW I do think that the actual offset within the function is helpful since, as you've linked, it can be used to translate to a filename/line number in a backtrace. If you're unable to get codeoffset working, though, perhaps the funcidx could always be encoded and the offset within-the-function (either specified as relative to the function's start or relative to the code section start) could be an optional field to only get filled in if recoverable?

@xtuc
Copy link
Contributor Author

xtuc commented Feb 9, 2023

@alexcrichton yes, that's actually a good idea; storing both the funcidx and the code offset relative to the function's start, which can be 0 if unknown.

It has the added benefit of allowing to degrade to a backtrace of func indices if you don't have the source module with debug infos, which is not possible with just code offsets.

@dschuff
Copy link
Member

dschuff commented Feb 9, 2023

code offsets are pretty important for DWARF because DWARF usually needs to refer to instructions rather than functions per se (I actually can't think of any cases off the top of my head where DWARF needs to refer a function abstractly rather than a particular piece of a function, but I could very well be wrong about that).
e.g. in backtraces, each location usually points at the exact call instruction rather than at the frame of each function generally.

If the intention is to have information general to the whole function or frame (and the exact path taken from one frame to the next isn't relevant) IMO a funcidx makes more sense.

@dschuff
Copy link
Member

dschuff commented Feb 9, 2023

oops, I raced with @xtuc
I guess the advantage of a code offset is that it's consistent with DWARF (which wants to pretend there's a unified address space and everything is just a pointer), and the advantage of a func + func offset is that you can degrade to having the funcoffset be 0 (in the former case you could also degrade to having a codeoffset pointing to the top of the function, but maybe the latter is easier).

@alexcrichton
Copy link
Collaborator

Oh sorry, so to clarify, I don't mind if it's funcidx, code-section offset, or function offset plus function index. I found it odd that it's specified as codeoffset but is being used as a funcidx right now. I don't personally have a horse in this race so don't have a preference.

@xtuc
Copy link
Contributor Author

xtuc commented Feb 9, 2023

got it @alexcrichton.

I like the idea of having both funcidx and codeoffset (relative to the start of the function's code), for the reasons mentioned above.

@xtuc
Copy link
Contributor Author

xtuc commented Feb 16, 2023

@dschuff please merge the change. I'm planning to follow up on funcidx and/or codeoffset in a future change.

@dschuff dschuff merged commit fc2df94 into WebAssembly:main Feb 16, 2023
@xtuc xtuc deleted the sven/type-local-stack-values branch February 17, 2023 12:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Coredump: ability to mark local/stack values as optimized out
3 participants