Skip to content

Commit d635cec

Browse files
committed
Debug: implement breakpoints and single-stepping.
This is a PR that puts together a bunch of earlier pieces (patchable calls in bytecodealliance#12061 and bytecodealliance#12101, private copies of code in bytecodealliance#12051, and all the prior debug event and instrumentation infrastructure) to implement breakpoints in the guest debugger. These are implemented in the way we have planned in bytecodealliance#11964: each sequence point (location prior to a Wasm opcode) is now a patchable call instruction, patched out (replaced with NOPs) by default. When patched in, the breakpoint callsite calls a trampoline with the `patchable` ABI which then invokes the `breakpoint` hostcall. That hostcall emits the debug event and nothing else. A few of the interesting bits in this PR include: - Implementations of "unpublish" (switch permissions back to read/write from read/execute) for mmap'd code memory on all our platforms. - Infrastructure in the frame-tables (debug info) metadata producer and parser to record "breakpoint patches". - A tweak to the NOP metadata packaged with the `MachBuffer` to allow multiple NOP sizes. This lets us use one 5-byte NOP on x86-64, for example (did you know x86-64 had these?!) rather than five 1-byte NOPs. This PR also implements single-stepping with a global-per-`Store` flag, because at this point why not; it's a small additional bit of logic to do *all* patches in all modules registered in the `Store` when that flag is enabled. A few realizations for future work: - The need for an introspection API available to a debugger to see the modules within a component is starting to become clear; either that, or the "module and PC" location identifier for a breakpoint switches to a "module or component" sum type. Right now, the tests for this feature use only core modules. Extending to components should not actually be hard at all, we just need to build the API for it. - The interaction between inlining and `patchable_call` is interesting: what happens if we inline a `patchable_call` at a `try_call` callsite? Right now, we do *not* update the `patchable_call` to a `try_call`, because there is no `patchable_try_call`; this is fine in the Wasmtime embedding in practice because we never (today!) throw exceptions from a breakpoint handler. This does suggest to me that maybe we should make patchability a property of any callsite, and allow try-calls to be patchable too (with the same restriction about no return values as the only restriction); but happy to discuss that one further.
1 parent 54826c0 commit d635cec

File tree

86 files changed

+1396
-200
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

86 files changed

+1396
-200
lines changed

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ capstone = { workspace = true, optional = true }
7979
termcolor = { workspace = true, optional = true }
8080
gimli = { workspace = true, optional = true }
8181
pulley-interpreter = { workspace = true, optional = true }
82+
smallvec = { workspace = true }
8283

8384
async-trait = { workspace = true }
8485
bytes = { workspace = true }

cranelift/codegen/src/inline.rs

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,14 @@ pub(crate) fn do_inlining(
227227
// Can't inline indirect calls; need to have some earlier
228228
// pass rewrite them into direct calls first, when possible.
229229
}
230+
ir::InstructionData::Call {
231+
opcode: ir::Opcode::PatchableCall,
232+
..
233+
} => {
234+
// Can't inline patchable calls; they need to
235+
// remain patchable and inlining the whole body is
236+
// decidedly *not* patchable!
237+
}
230238
_ => {
231239
debug_assert!(
232240
!cursor.func.dfg.insts[inst].opcode().is_call(),
@@ -497,7 +505,19 @@ fn inline_one(
497505
call_opcode == ir::Opcode::TryCall,
498506
call_exception_table.is_some()
499507
);
500-
if call_opcode == ir::Opcode::TryCall {
508+
// Note that we do not fix up patchable calls
509+
// inlined at a try-call to a try-call, because
510+
// the "patchable ABI" does not support catching
511+
// exceptions. This does mean that we cannot have
512+
// an exception-throw propagate out of a
513+
// breakpoint when we use patchable calls to set
514+
// up breakpoints, but we don't expect that to
515+
// occur.
516+
//
517+
// FIXME: consider making patchability an aspect
518+
// of any call; then we can remove this special
519+
// case.
520+
if call_opcode == ir::Opcode::TryCall && opcode != ir::Opcode::PatchableCall {
501521
allocs
502522
.calls_needing_exception_table_fixup
503523
.push(inlined_inst);

cranelift/codegen/src/isa/aarch64/inst/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1111,8 +1111,8 @@ impl MachInst for Inst {
11111111
Inst::Nop4
11121112
}
11131113

1114-
fn gen_nop_unit() -> SmallVec<[u8; 8]> {
1115-
smallvec![0x1f, 0x20, 0x03, 0xd5]
1114+
fn gen_nop_units() -> Vec<Vec<u8>> {
1115+
vec![vec![0x1f, 0x20, 0x03, 0xd5]]
11161116
}
11171117

11181118
fn rc_for_type(ty: Type) -> CodegenResult<(&'static [RegClass], &'static [Type])> {

cranelift/codegen/src/isa/pulley_shared/inst/mod.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ use crate::isa::pulley_shared::abi::PulleyMachineDeps;
1010
use crate::{CodegenError, CodegenResult, settings};
1111
use crate::{machinst::*, trace};
1212
use alloc::string::{String, ToString};
13+
use alloc::vec;
14+
use alloc::vec::Vec;
1315
use regalloc2::RegClass;
1416
use smallvec::SmallVec;
1517

@@ -535,14 +537,14 @@ where
535537
todo!()
536538
}
537539

538-
fn gen_nop_unit() -> SmallVec<[u8; 8]> {
539-
let mut bytes = smallvec::smallvec![];
540+
fn gen_nop_units() -> Vec<Vec<u8>> {
541+
let mut bytes = vec![];
540542
let nop = pulley_interpreter::op::Nop {};
541543
nop.encode(&mut bytes);
542544
// NOP needs to be a 1-byte opcode so it can be used to
543545
// overwrite a callsite of any length.
544546
assert_eq!(bytes.len(), 1);
545-
bytes
547+
vec![bytes]
546548
}
547549

548550
fn rc_for_type(ty: Type) -> CodegenResult<(&'static [RegClass], &'static [Type])> {

cranelift/codegen/src/isa/riscv64/inst/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -811,8 +811,8 @@ impl MachInst for Inst {
811811
Inst::Nop4
812812
}
813813

814-
fn gen_nop_unit() -> SmallVec<[u8; 8]> {
815-
smallvec![0x13, 0x00, 0x00, 0x00]
814+
fn gen_nop_units() -> Vec<Vec<u8>> {
815+
vec![vec![0x13, 0x00, 0x00, 0x00]]
816816
}
817817

818818
fn rc_for_type(ty: Type) -> CodegenResult<(&'static [RegClass], &'static [Type])> {

cranelift/codegen/src/isa/s390x/inst/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1164,8 +1164,8 @@ impl MachInst for Inst {
11641164
}
11651165
}
11661166

1167-
fn gen_nop_unit() -> SmallVec<[u8; 8]> {
1168-
smallvec::smallvec![0x07, 0x07]
1167+
fn gen_nop_units() -> Vec<Vec<u8>> {
1168+
vec![vec![0x07, 0x07]]
11691169
}
11701170

11711171
fn rc_for_type(ty: Type) -> CodegenResult<(&'static [RegClass], &'static [Type])> {

cranelift/codegen/src/isa/x64/inst/mod.rs

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ use crate::isa::{CallConv, FunctionAlignment};
1111
use crate::{CodegenError, CodegenResult, settings};
1212
use crate::{machinst::*, trace};
1313
use alloc::boxed::Box;
14+
use alloc::vec;
15+
use alloc::vec::Vec;
1416
use core::slice;
1517
use cranelift_assembler_x64 as asm;
1618
use smallvec::{SmallVec, smallvec};
@@ -1308,6 +1310,7 @@ impl MachInst for Inst {
13081310
match self {
13091311
Inst::CallKnown { .. }
13101312
| Inst::CallUnknown { .. }
1313+
| Inst::PatchableCallKnown { .. }
13111314
| Inst::ElfTlsGetAddr { .. }
13121315
| Inst::MachOTlsGetAddr { .. } => CallType::Regular,
13131316

@@ -1391,8 +1394,13 @@ impl MachInst for Inst {
13911394
Inst::nop(std::cmp::min(preferred_size, 9) as u8)
13921395
}
13931396

1394-
fn gen_nop_unit() -> SmallVec<[u8; 8]> {
1395-
smallvec![0x90]
1397+
fn gen_nop_units() -> Vec<Vec<u8>> {
1398+
vec![
1399+
// Standard 1-byte NOP.
1400+
vec![0x90],
1401+
// 5-byte NOP useful for patching out patchable calls.
1402+
vec![0x0f, 0x1f, 0x44, 0x00, 0x00],
1403+
]
13961404
}
13971405

13981406
fn rc_for_type(ty: Type) -> CodegenResult<(&'static [RegClass], &'static [Type])> {

cranelift/codegen/src/machinst/buffer.rs

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -355,7 +355,7 @@ impl MachBufferFinalized<Stencil> {
355355
unwind_info: self.unwind_info,
356356
alignment: self.alignment,
357357
frame_layout: self.frame_layout,
358-
nop: self.nop,
358+
nop_units: self.nop_units,
359359
}
360360
}
361361
}
@@ -406,7 +406,10 @@ pub struct MachBufferFinalized<T: CompilePhase> {
406406
/// This allows a consumer of a `MachBufferFinalized` to disable
407407
/// patchable call sites (which are enabled by default) without
408408
/// specific knowledge of the target ISA.
409-
pub nop: SmallVec<[u8; 8]>,
409+
///
410+
/// Each entry is one form of nop, and these are required to be
411+
/// sorted in ascending-size order.
412+
pub nop_units: Vec<Vec<u8>>,
410413
}
411414

412415
const UNKNOWN_LABEL_OFFSET: CodeOffset = 0xffff_ffff;
@@ -1586,7 +1589,7 @@ impl<I: VCodeInst> MachBuffer<I> {
15861589
unwind_info: self.unwind_info,
15871590
alignment,
15881591
frame_layout: self.frame_layout,
1589-
nop: I::gen_nop_unit(),
1592+
nop_units: I::gen_nop_units(),
15901593
}
15911594
}
15921595

@@ -1841,6 +1844,12 @@ impl<T: CompilePhase> MachBufferFinalized<T> {
18411844
&self.data[..]
18421845
}
18431846

1847+
/// Get a mutable slice of the code bytes, allowing patching
1848+
/// post-passes.
1849+
pub fn data_mut(&mut self) -> &mut [u8] {
1850+
&mut self.data[..]
1851+
}
1852+
18441853
/// Get the list of external relocations for this code.
18451854
pub fn relocs(&self) -> &[FinalizedMachReloc] {
18461855
&self.relocs[..]
@@ -1900,10 +1909,7 @@ impl<T: CompilePhase> MachBufferFinalized<T> {
19001909
/// region is guaranteed to be an integer multiple of that NOP
19011910
/// unit size.)
19021911
pub fn patchable_call_sites(&self) -> impl Iterator<Item = &MachPatchableCallSite> + '_ {
1903-
self.patchable_call_sites.iter().map(|call_site| {
1904-
debug_assert!(call_site.len as usize % self.nop.len() == 0);
1905-
call_site
1906-
})
1912+
self.patchable_call_sites.iter()
19071913
}
19081914
}
19091915

cranelift/codegen/src/machinst/mod.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -177,9 +177,9 @@ pub trait MachInst: Clone + Debug {
177177
/// the instruction must have a nonzero size if preferred_size is nonzero.
178178
fn gen_nop(preferred_size: usize) -> Self;
179179

180-
/// The smallest possible NOP, as a unit that can be used to patch
181-
/// out code.
182-
fn gen_nop_unit() -> SmallVec<[u8; 8]>;
180+
/// The various kinds of NOP, with size, sorted in ascending-size
181+
/// order.
182+
fn gen_nop_units() -> Vec<Vec<u8>>;
183183

184184
/// Align a basic block offset (from start of function). By default, no
185185
/// alignment occurs.

0 commit comments

Comments
 (0)