Stream: general

Topic: wasmtime debugging: lldb c++ (this) compatibility / work...


view this post on Zulip Steve Williams (Oct 08 2021 at 18:16):

rust side: there's a resolve_vmctx_memory_ptr that seems to do what I want, its just not exported.

as that's not messing about with string tables, I might even be able to get that running.

copy set export template, figure out how to add the integer parameter & done, maybe.

at least to get something working.

view this post on Zulip Steve Williams (Oct 08 2021 at 18:17):

https://github.com/bytecodealliance/wasmtime/blob/main/crates/runtime/src/debug_builtins.rs

Standalone JIT-style runtime for WebAssembly, using Cranelift - wasmtime/debug_builtins.rs at main · bytecodealliance/wasmtime

view this post on Zulip Steve Williams (Oct 08 2021 at 20:38):

oops, wrong thread, sorry.

view this post on Zulip bjorn3 (Oct 08 2021 at 21:13):

If you chose edit for the first commit in the wrong topic, you can change the topic title to match the topic you actually wanted to post it in. Zulip will then ask you if you want to move all later messages too.

view this post on Zulip bjorn3 (Oct 08 2021 at 21:14):

I would do this for you if not for the fact that the mobile app doesn't support this.

view this post on Zulip Steve Williams (Oct 09 2021 at 09:47):

issue summary: lldb expects c++ "this" to be a standard system memory pointer. when its not as is the case with current wasmtime wrapped pointer delvered via jit debug connection, we run into difficulties. we are prevented from being able to access 'this' as we would other variables because the evaluate fails because lldb is hardwired to expect "this" to be a standard pointer.

(continued from detail in lldbg post - its not an lldbg issue, its core)

https://bytecodealliance.zulipchat.com/#narrow/stream/206238-general/topic/debugging.20wasm.20with.20lldbg

view this post on Zulip Steve Williams (Oct 09 2021 at 13:55):

I've got some kind of regression or config error on windows. switching to linux where things appear more sane right now.

running wasmtime trunk, lldb 12.0.1

(lldb) target create "wasmtime"
Current executable set to 'wasmtime' (x86_64).
(lldb) settings set -- target.run-args "-g" "./test.wasm"
(lldb) b test.cpp:7
Breakpoint 1: no locations (pending).
WARNING: Unable to resolve breakpoint to any actual locations.
(lldb) r
Process 21261 launched: '/home/x/dev/tools/wasmtime' (x86_64)
1 location added to breakpoint 1
Process 21261 stopped

view this post on Zulip Steve Williams (Oct 09 2021 at 14:06):

resolve_vmctx_memory_ptr seems to want address of sandbox pointer, not its direct value, so that's probably why it's faulting.

view this post on Zulip Steve Williams (Oct 09 2021 at 14:22):

tried :

#[no_mangle]
pub unsafe extern "C" fn resolve_vmctx_memory(ptr: usize) -> *const u8 {
let handle = InstanceHandle::from_vmctx(VMCTX_AND_MEMORY.0);
assert!(
VMCTX_AND_MEMORY.1 < handle.module().memory_plans.len(),
"memory index for debugger is out of bounds"
);
let index = MemoryIndex::new(VMCTX_AND_MEMORY.1);
let mem = handle.instance().get_memory(index);
mem.base.add(ptr)
}

but not found so there's something else I need to tweak somewhere to add a new reserved function.

view this post on Zulip Steve Williams (Oct 09 2021 at 14:32):

ahh, ensure_exported section :)

view this post on Zulip Steve Williams (Oct 09 2021 at 14:48):

Looks promising:

(lldb) target create "wasmtime"
Current executable set to 'wasmtime' (x86_64).
(lldb) settings set -- target.run-args "-g" "./test.wasm"
(lldb) b test.cpp:7
Breakpoint 1: no locations (pending).
WARNING: Unable to resolve breakpoint to any actual locations.
(lldb) r
Process 29127 launched: '/home/x/dev/tools/wasmtime' (x86_64)
1 location added to breakpoint 1
Process 29127 stopped

view this post on Zulip Steve Williams (Oct 09 2021 at 14:51):

Is there an actual need for the resolve in here to be a pointer to a sandbox pointer rather than the pointer itself ?
https://github.com/bytecodealliance/wasmtime/blob/main/crates/runtime/src/debug_builtins.rs
if we can mod the one that's there, there's no need to add another method. usize rather than u32 though ideally so we have 64 bit covered too.

Standalone JIT-style runtime for WebAssembly, using Cranelift - wasmtime/debug_builtins.rs at main · bytecodealliance/wasmtime

view this post on Zulip Steve Williams (Oct 09 2021 at 14:53):

if can perform above from lldbg seems I've got a solution. lldb doesn't 'like' the invalid this pointer but it seems to let us pass.

view this post on Zulip Steve Williams (Oct 09 2021 at 15:00):

& we shouldn't be sending that as const as debuggers will sometimes mess with it & that's valid.

view this post on Zulip Steve Williams (Oct 09 2021 at 15:01):

done, I think but won't know for sure until I have it fixed up in the GUI & there's a more elegant solution in there with a valid this pointer to keep lldb happier.

view this post on Zulip Steve Williams (Oct 10 2021 at 15:00):

updated : https://github.com/adv-sw/lldbg

includes 'this' workaround with all detail hidden in gui so it behaves as expected.
requires minor wasmtime tweak to add resolve_vmctx_memory function - ready to push that.

A lightweight native GUI for LLDB. Contribute to adv-sw/lldbg development by creating an account on GitHub.

view this post on Zulip Steve Williams (Oct 11 2021 at 09:10):

resolves "this" pointer cannot be evaluated by lldb issue.

https://github.com/bytecodealliance/wasmtime/commit/92a10d1ace70fe75c3e2c646ea613168fc417851
https://github.com/bytecodealliance/wasmtime/issues/3419

plz review/merge.

…ndbox pointers - required because sandbox 'this' pointer cannot be resolved by lldb any other way as lldb expects "this" and "self" to be standard pointers, not sandbox...
Unzip test case. On Linux : Run make_wasm.sh to build Optional lldbg debugging front end - tested linux & windows : https://github.com/adv-sw/lldbg Modify dbg_wasm.sh to run under lldb direct i...

view this post on Zulip Steve Williams (Oct 11 2021 at 17:20):

more specifically, could I have a reviewer for this, please.

no huge rush but then it's done. mod is trivial, doesn't affect anything else.

view this post on Zulip Steve Williams (Oct 11 2021 at 17:22):

security: only invoked under debugger which legitimately can access/modify anything in the sandbox.

view this post on Zulip bjorn3 (Oct 11 2021 at 17:24):

cc @Yury Delendik

view this post on Zulip Steve Williams (Oct 11 2021 at 17:25):

could factor resolve_ptr into resolve if you'd like.

view this post on Zulip Steve Williams (Oct 11 2021 at 17:27):

to answer my own q: resolve_ptr is required for * operators injected by wasmtime. so yes, we do need both unless that code is to be modified (unnecessary imo - lets keep this simple for now)

view this post on Zulip Steve Williams (Oct 11 2021 at 17:41):

cfallon : Would you mind creating a PR for the review and merging of your commit?

ummm sure ... how ?

:)

The usual flow is that one will create this PR with the text "Fixes #ISSUE-NUMBER" in the commit message, which links it back to this issue and closes it when merged)

view this post on Zulip Steve Williams (Oct 11 2021 at 17:42):

PR = pull request. thought I did. that's what this is, isn't it ?

https://github.com/bytecodealliance/wasmtime/commit/92a10d1ace70fe75c3e2c646ea613168fc417851

git collab noob also :)

…ndbox pointers - required because sandbox 'this' pointer cannot be resolved by lldb any other way as lldb expects "this" and "self" to be standard pointers, not sandbox...

view this post on Zulip bjorn3 (Oct 11 2021 at 17:43):

On your fork there is a button "contribute" that you can click on.

image.png

view this post on Zulip bjorn3 (Oct 11 2021 at 17:44):

Normally you would create a branch and push the changes there though instead of push to the main branch.

view this post on Zulip bjorn3 (Oct 11 2021 at 17:44):

This allows you to have multiple PR's at the same time.

view this post on Zulip bjorn3 (Oct 11 2021 at 17:45):

Also I think you will want to remove the merge commit by rebasing on top of the bytecodealliance/wasmtime main branch.

view this post on Zulip Steve Williams (Oct 11 2021 at 17:45):

@bjorn3 - right ... thought that's what I did, though via pull requests, create pull request.
didn't label anything with issue # happy to do so, going forwards.

view this post on Zulip Steve Williams (Oct 11 2021 at 17:50):

unsure what else you want other than "fixes (indirectly) #3419 by providing debugger with ability to resolve sandbox pointers"

view this post on Zulip Chris Fallin (Oct 11 2021 at 17:56):

@Steve Williams that is just a git commit, not a PR; a PR is a separate entity on GitHub that creates a discussion thread, a space for code review, and asks the maintainers to merge your commit into the mainline. You can click on "Pull Requests" in the main bytecodealliance/wasmtime repo and then the "New Pull Request" button (green, upper right), and select your repo fork and branch with the commit(s) you want to contribute

view this post on Zulip Steve Williams (Oct 11 2021 at 18:04):

Thanks, Chris. Will see if I can present patch as requested.

view this post on Zulip Steve Williams (Oct 11 2021 at 18:08):

Like this ?

https://github.com/bytecodealliance/wasmtime/pull/3443

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

view this post on Zulip Steve Williams (Oct 11 2021 at 18:30):

The direct fix is lldbg which provides high level this access - appears to be working nicely with minimal testing. Equivalent could be presented as macro for lldb direct, but haven't done that as don't currently need it. Quite liking the new front end. Could be a bytecode alliance project if desired, though as its largely generic lldb front end, perhaps better over there. Dunno, don't care. Just pleased we have it.

Looking forwards to being able to present my globals :)

https://github.com/adv-sw/lldbg

Direct fix requires indirect fix in PR 3443 & also lldb patch on windows to fix their JIT interface.

Other platforms should work out of the box.

A lightweight native GUI for LLDB. Contribute to adv-sw/lldbg development by creating an account on GitHub.

view this post on Zulip Chris Fallin (Oct 11 2021 at 18:35):

Got it, will review -- thanks!

view this post on Zulip Steve Williams (Oct 11 2021 at 18:39):

LLDB/Windows JIT patch fix here : https://reviews.llvm.org/D110066

Other platforms should interoperate with above out of the box.

view this post on Zulip Steve Williams (Oct 11 2021 at 18:41):

Dan - do you have commit over there ? Can it go in plz. LLDB is currently broken on Windows, that's not clever wrt wide spread adoption which is surely part of the goal here. Test can wait, it works. -my 2c.

view this post on Zulip Steve Williams (Oct 11 2021 at 18:45):

Test is lldb or lldbg :) Without patch - JIT debugging fails. With patch, JIT debugging connects.

view this post on Zulip Steve Williams (Oct 11 2021 at 18:49):

That comes out sounding like a demand. Sorry. I humbly request LLVM D110066 is accepted as is following confirmation it functions as stated. Can maintain out of tree, but I think its better for all that it goes in. Eliminate the wonky.

view this post on Zulip Steve Williams (Oct 11 2021 at 18:55):

or anyone else with commit - not trying to put anyone on the spot with my nonsense :)

view this post on Zulip Steve Williams (Oct 11 2021 at 19:00):

also, sooner its in, sooner its into a standard release regular folks have access to.

view this post on Zulip Steve Williams (Oct 11 2021 at 20:01):

Thanks Chris - think that's done as requested now. Assistance appreciated.

LLDB aside, perhaps the wrong place, certainly the wrong thread :)


Last updated: Dec 23 2024 at 12:05 UTC