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

Add debug symbol transformation for AArch64 #4393

Closed
jeffcharles opened this issue Jul 6, 2022 · 6 comments
Closed

Add debug symbol transformation for AArch64 #4393

jeffcharles opened this issue Jul 6, 2022 · 6 comments
Labels
cranelift:area:aarch64 Issues related to AArch64 backend. cranelift:area:debug cranelift Issues related to the Cranelift code generator enhancement

Comments

@jeffcharles
Copy link
Contributor

Feature

Debug symbol transformation for AArch64. That is, when running wasmtime compile -g <path_to_wasm>, wasmtime does not exit with an error message and debug symbols are included in the resulting .cwasm file.

Benefit

It enables developers writing code targeting a Wasm environment to be able to step debug their code in a Wasm environment if they are using Apple Silicon.

Implementation

https://github.com/jeffcharles/wasmtime/pull/1/files. This uses a similar approach to how the symbols are mapped for x86_64. I can open that PR on this repo instead. The contribution guidelines mentioned I should open an issue here first to discuss.

The PR above seems to work for a number of test cases that I've tried.

Alternatives

I'm not sure what the alternatives are.

@akirilov-arm akirilov-arm added enhancement cranelift Issues related to the Cranelift code generator cranelift:area:aarch64 Issues related to AArch64 backend. cranelift:area:debug labels Jul 11, 2022
@akirilov-arm
Copy link
Contributor

Have you seen issue #2856? It documents some of the issues with full debugging support on non-x86 platforms (in fact, it subsumes the previous AArch64-specific issue, #1523) and is probably a better place to discuss any details that are not AArch64-specific.

@jeffcharles
Copy link
Contributor Author

I had not seen that issue. I can take a look at those details.

Even without changes to the debug crate though, given my change, I'm currently able to get a reasonable debugging experience on AArch64 as opposed to an error message with the status quo.

@jameysharp
Copy link
Contributor

I see the PR just makes aarch64 do exactly what x64 and s390x already do. I don't know much yet about Cranelift's target backends, but it seems plausible to me that inst::unwind::systemv::map_reg would be the right choice for aarch64 as well. @akirilov-arm, is there any reason not to merge this?

Also, maybe we should edit the contributor guidelines to say that if it's easier to explain your issue by just opening a pull request showing what you want changed, then you don't need to open an issue first? Anyway I'd say go ahead and open a PR here with your proposed changes.

@akirilov-arm
Copy link
Contributor

@akirilov-arm, is there any reason not to merge this?

I don't know, usually I don't look at PRs that have not been opened in this repository. From a cursory glance it looks OK, but I am not really familiar with that part of the code.

@jeffcharles
Copy link
Contributor Author

I've opened a PR at #4468.

@jameysharp
Copy link
Contributor

Merged, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cranelift:area:aarch64 Issues related to AArch64 backend. cranelift:area:debug cranelift Issues related to the Cranelift code generator enhancement
Projects
None yet
Development

No branches or pull requests

3 participants