Stream: git-wasmtime

Topic: wasmtime / issue #2856 Debug support for non-x86 architec...


view this post on Zulip Wasmtime GitHub notifications bot (May 04 2022 at 22:32):

cfallin labeled issue #2856:

Feature

Support debugging JITted WebAssembly code on non-x86 platforms.

Benefit

Currently, the debug crate only supports x86. All other platforms should be supported as well.

Implementation

There are a number of places that currently prevent the debug crate from supporting non-x86 platforms:

    match header.e_machine.get(e) {
        EM_X86_64 => (),
        machine => {
            bail!("Unsupported ELF target machine: {:x}", machine);
        }
    }

(This should just go away, I think.)

writer.write_op_breg(X86_64::RBP.0)?;
writer.write_sleb128(ss_offset as i64 + X86_64_STACK_OFFSET)?;

(This is only used for old-style back-ends and can probably go away soon.)

writer.write_op_breg(X86_64::RSP.0)?;

(This should probably use the register mapper that unwinder code also uses.)

view this post on Zulip Wasmtime GitHub notifications bot (May 04 2022 at 22:32):

cfallin labeled issue #2856:

Feature

Support debugging JITted WebAssembly code on non-x86 platforms.

Benefit

Currently, the debug crate only supports x86. All other platforms should be supported as well.

Implementation

There are a number of places that currently prevent the debug crate from supporting non-x86 platforms:

    match header.e_machine.get(e) {
        EM_X86_64 => (),
        machine => {
            bail!("Unsupported ELF target machine: {:x}", machine);
        }
    }

(This should just go away, I think.)

writer.write_op_breg(X86_64::RBP.0)?;
writer.write_sleb128(ss_offset as i64 + X86_64_STACK_OFFSET)?;

(This is only used for old-style back-ends and can probably go away soon.)

writer.write_op_breg(X86_64::RSP.0)?;

(This should probably use the register mapper that unwinder code also uses.)

view this post on Zulip Wasmtime GitHub notifications bot (May 04 2022 at 22:33):

cfallin labeled issue #2856:

Feature

Support debugging JITted WebAssembly code on non-x86 platforms.

Benefit

Currently, the debug crate only supports x86. All other platforms should be supported as well.

Implementation

There are a number of places that currently prevent the debug crate from supporting non-x86 platforms:

    match header.e_machine.get(e) {
        EM_X86_64 => (),
        machine => {
            bail!("Unsupported ELF target machine: {:x}", machine);
        }
    }

(This should just go away, I think.)

writer.write_op_breg(X86_64::RBP.0)?;
writer.write_sleb128(ss_offset as i64 + X86_64_STACK_OFFSET)?;

(This is only used for old-style back-ends and can probably go away soon.)

writer.write_op_breg(X86_64::RSP.0)?;

(This should probably use the register mapper that unwinder code also uses.)

view this post on Zulip Wasmtime GitHub notifications bot (Jul 20 2022 at 21:51):

jeffcharles commented on issue #2856:

I took a brief look at this for the purposes of avoiding unexpected behaviour on AArch64. So I focused more on the references to x86 registers and not at all on the questions around endianess.

The only two locations that appear to reference X86 registers in transform/expression.rs are:

in both cases, the code is only run when a LabelValueLoc is matched against an SPOffset variant. The only code that I could find that creates a LabelValueLoc::SPOffset is in a setup function for test cases https://github.com/bytecodealliance/wasmtime/blob/9c43749dfe0b378c40b9932694d248c3546abac3/crates/cranelift/src/debug/transform/expression.rs#L1159-L1200

Based on that, I don't think the two pieces of code that reference the x86 stack pointer register will ever execute outside of tests. Perhaps the tests and the setup for them can be rewritten such that they no longer use the SPOffset variant so the two pieces of code for handling the SPOffset variant containing the reference to the x86 stack pointer register can be deleted.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 22 2022 at 00:32):

jameysharp commented on issue #2856:

It looks to me like LabelValueLoc::SPOffset is supposed to get used here, so maybe deleting it isn't the right plan, but it's apparently hard to use correctly: https://github.com/bytecodealliance/wasmtime/blob/ca36ce57c2e7903fe2410aae95d564bb7d792cb5/cranelift/codegen/src/machinst/vcode.rs#L1095-L1108

If this is a common thing across different platforms using DWARF, should TargetIsa provide the DWARF index of the stack-pointer register? I know very little about how DWARF works.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 22 2022 at 10:19):

uweigand commented on issue #2856:

The way this normally works in DWARF is that the location of local variables and spillslots is specified via the DW_OP_fbreg operation as constant offsets from a "frame base". Then, separately, there is a description of how to compute that frame base value from the current register context (e.g. stack or frame pointer), given via a DW_AT_frame_base function attribute.

These days, it is often easiest to specify that frame base in terms of DWARF CFI unwind information (note that DWARF debug info and DWARF unwind info are separate entities - but if you have both, it makes sense to avoid duplication). This works by defining DW_AT_frame_base in terms of the DW_OP_call_frame_cfa operation as a constant offset from the current Canonical Frame Address (CFA) defined by unwind info. Since cranelift already provides this CFI unwind data, I think this would be the best option for us.

The compiler is free to choose where exactly to place the "frame base", so we have some options here. We could define the frame base at either the top or the bottom of the fixed frame area - that makes variable locations trivial to define, but then we'd need to provide the information from the compiler to the debug crate what the (per-function) offset from the CFA to that frame base is.

Or else, we could define the frame base to be always identical to the CFA, which would make the implementation in the debug crate trivial and avoid this new interface, but would make the definition of variable locations a little bit more complex (but that's all in the compiler backend which knows everything about the frame layout anyway).

In either case, to describe variable locations we would not use an SP offset, but rather a frame base offset, so we should eliminate LabelValueLoc::SPOffset in favor of some new LabelValueLoc::FrameBaseOffset or maybe LabelValueLoc::CFAOffset.

@cfallin any thoughts on this?

view this post on Zulip Wasmtime GitHub notifications bot (Jul 22 2022 at 22:00):

cfallin commented on issue #2856:

Yes, I agree that making everything relative to FP would be substantially simpler here: it would let us translate spillslot addresses without regard to emission state (nominal-SP offset) in the code linked above.

In general we really need someone to do an overhaul of our DWARF translation code and this is one concrete example -- unfortunately just no one has the time at the moment :-/


Last updated: Oct 23 2024 at 20:03 UTC