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

Debug support for non-x86 architectures #2856

Open
uweigand opened this issue Apr 21, 2021 · 5 comments
Open

Debug support for non-x86 architectures #2856

uweigand opened this issue Apr 21, 2021 · 5 comments
Labels
cranelift:area:debug cranelift Issues related to the Cranelift code generator enhancement

Comments

@uweigand
Copy link
Member

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:

  • Explicit architecture check in lib.rs:
    match header.e_machine.get(e) {
        EM_X86_64 => (),
        machine => {
            bail!("Unsupported ELF target machine: {:x}", machine);
        }
    }

(This should just go away, I think.)

  • Explicit X86 assumptions in transform/expression.rs:
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.)

  • Various little-endian assumptions accessing ELF files and WebAssembly memory
    (See debug: Support big-endian architectures #2854 for details.)

  • Additional endian issues (not solved by the PR above) in creating DWARF expressions
    Current code in transform/expression.rs simply copies portions of the incoming WebAssembly DWARF expressions directly into the native DWARF output. This is not correct in case the native architecture is big-endian. Fortunately, the byte code for many DWARF expressions is not endian-sensitive, so I can actually debug simple applications even so. But to be fully correct, those portions of DWARF bytecode that are endian-sensitive will need to be handled here somehow.

@uweigand
Copy link
Member Author

Hi @cfallin, this is the topic we talked about recently. I just wanted to open this issue to document all the places I've found where there is currently X86-specific code in the debug crate.

@jeffcharles
Copy link
Contributor

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

fn create_mock_value_ranges() -> (ValueLabelsRanges, (ValueLabel, ValueLabel, ValueLabel)) {
use cranelift_codegen::ir::LabelValueLoc;
use cranelift_codegen::ValueLocRange;
use cranelift_entity::EntityRef;
use std::collections::HashMap;
let mut value_ranges = HashMap::new();
let value_0 = ValueLabel::new(0);
let value_1 = ValueLabel::new(1);
let value_2 = ValueLabel::new(2);
value_ranges.insert(
value_0,
vec![ValueLocRange {
loc: LabelValueLoc::SPOffset(0),
start: 0,
end: 25,
}],
);
value_ranges.insert(
value_1,
vec![ValueLocRange {
loc: LabelValueLoc::SPOffset(0),
start: 5,
end: 30,
}],
);
value_ranges.insert(
value_2,
vec![
ValueLocRange {
loc: LabelValueLoc::SPOffset(0),
start: 0,
end: 10,
},
ValueLocRange {
loc: LabelValueLoc::SPOffset(0),
start: 20,
end: 30,
},
],
);
(value_ranges, (value_0, value_1, value_2))
}

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.

@jameysharp
Copy link
Contributor

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:

let loc = if let Some(preg) = alloc.as_reg() {
LabelValueLoc::Reg(Reg::from(preg))
} else {
// We can't translate spillslot locations at the
// moment because ValueLabelLoc requires an
// instantaneous SP offset, and this can *change*
// within the range we have here because of callsites
// adjusting SP temporarily. To avoid the complexity
// of accurately plumbing through nominal-SP
// adjustment sites, we just omit debug info for
// values that are spilled. Not ideal, but debug info
// is best-effort.
continue;
};

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.

@uweigand
Copy link
Member Author

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?

@cfallin
Copy link
Member

cfallin commented Jul 22, 2022

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 :-/

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

No branches or pull requests

4 participants