Skip to content

Commit 62360e3

Browse files
authored
Use a runtime setting for debug checks (#11604)
* Update test expectations * Use a runtime setting for debug checks This commit updates Wasmtime's trampoline compilation to insert debug checks when a runtime setting is configured instead of the compile-time `debug_assertions` setting. Using a compile-time variable can be confusing because it means that different builds of Wasmtime can produce different output when the builds only differ in optimization settings. An example of this is that `cargo test --test disas --release` is broken before this PR. By using a runtime setting this won't be enabled as often but we'll still enable it for all `wast` testing, for example. * Fix a test with Winch
1 parent a631d20 commit 62360e3

Some content is hidden

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

48 files changed

+186
-197
lines changed

crates/cranelift/src/builder.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ use wasmtime_environ::{CacheStore, CompilerBuilder, Setting, Tunables};
1818
struct Builder {
1919
tunables: Option<Tunables>,
2020
inner: IsaBuilder<CodegenResult<OwnedTargetIsa>>,
21+
emit_debug_checks: bool,
2122
linkopts: LinkOptions,
2223
cache_store: Option<Arc<dyn CacheStore>>,
2324
clif_dir: Option<path::PathBuf>,
@@ -45,6 +46,7 @@ pub fn builder(triple: Option<Triple>) -> Result<Box<dyn CompilerBuilder>> {
4546
cache_store: None,
4647
clif_dir: None,
4748
wmemcheck: false,
49+
emit_debug_checks: false,
4850
}))
4951
}
5052

@@ -81,6 +83,9 @@ impl CompilerBuilder for Builder {
8183
"wasmtime_inlining_sum_size_threshold" => {
8284
self.tunables.as_mut().unwrap().inlining_sum_size_threshold = value.parse()?;
8385
}
86+
"wasmtime_debug_checks" => {
87+
self.emit_debug_checks = true;
88+
}
8489
_ => {
8590
self.inner.set(name, value)?;
8691
}
@@ -106,6 +111,7 @@ impl CompilerBuilder for Builder {
106111
.clone(),
107112
isa,
108113
self.cache_store.clone(),
114+
self.emit_debug_checks,
109115
self.linkopts.clone(),
110116
self.clif_dir.clone(),
111117
self.wmemcheck,

crates/cranelift/src/compiler.rs

Lines changed: 48 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ pub struct Compiler {
8787
tunables: Tunables,
8888
contexts: Mutex<Vec<CompilerContext>>,
8989
isa: OwnedTargetIsa,
90+
emit_debug_checks: bool,
9091
linkopts: LinkOptions,
9192
cache_store: Option<Arc<dyn CacheStore>>,
9293
clif_dir: Option<path::PathBuf>,
@@ -127,6 +128,7 @@ impl Compiler {
127128
tunables: Tunables,
128129
isa: OwnedTargetIsa,
129130
cache_store: Option<Arc<dyn CacheStore>>,
131+
emit_debug_checks: bool,
130132
linkopts: LinkOptions,
131133
clif_dir: Option<path::PathBuf>,
132134
wmemcheck: bool,
@@ -136,6 +138,7 @@ impl Compiler {
136138
contexts: Default::default(),
137139
tunables,
138140
isa,
141+
emit_debug_checks,
139142
linkopts,
140143
cache_store,
141144
clif_dir,
@@ -392,7 +395,7 @@ impl wasmtime_environ::Compiler for Compiler {
392395
//
393396
// Assert that we were really given a core Wasm vmctx, since that's
394397
// what we are assuming with our offsets below.
395-
debug_assert_vmctx_kind(isa, &mut builder, vmctx, wasmtime_environ::VMCONTEXT_MAGIC);
398+
self.debug_assert_vmctx_kind(&mut builder, vmctx, wasmtime_environ::VMCONTEXT_MAGIC);
396399
let offsets = VMOffsets::new(isa.pointer_bytes(), &translation.module);
397400
let vm_store_context_offset = offsets.ptr.vmctx_store_context();
398401
save_last_wasm_entry_fp(
@@ -456,8 +459,7 @@ impl wasmtime_environ::Compiler for Compiler {
456459
//
457460
// Assert that the caller vmctx really is a core Wasm vmctx, since
458461
// that's what we are assuming with our offsets below.
459-
debug_assert_vmctx_kind(
460-
isa,
462+
self.debug_assert_vmctx_kind(
461463
&mut builder,
462464
caller_vmctx,
463465
wasmtime_environ::VMCONTEXT_MAGIC,
@@ -707,7 +709,7 @@ impl wasmtime_environ::Compiler for Compiler {
707709
// Debug-assert that this is the right kind of vmctx, and then
708710
// additionally perform the "routine of the exit trampoline" of saving
709711
// fp/pc/etc.
710-
debug_assert_vmctx_kind(isa, &mut builder, vmctx, wasmtime_environ::VMCONTEXT_MAGIC);
712+
self.debug_assert_vmctx_kind(&mut builder, vmctx, wasmtime_environ::VMCONTEXT_MAGIC);
711713
let vm_store_context = builder.ins().load(
712714
pointer_type,
713715
MemFlags::trusted(),
@@ -1034,7 +1036,7 @@ impl Compiler {
10341036
values_vec_capacity: Value,
10351037
) {
10361038
debug_assert_eq!(types.len(), values.len());
1037-
debug_assert_enough_capacity_for_length(builder, types.len(), values_vec_capacity);
1039+
self.debug_assert_enough_capacity_for_length(builder, types.len(), values_vec_capacity);
10381040

10391041
// Note that loads and stores are unconditionally done in the
10401042
// little-endian format rather than the host's native-endianness,
@@ -1074,7 +1076,7 @@ impl Compiler {
10741076
let isa = &*self.isa;
10751077
let value_size = mem::size_of::<u128>();
10761078

1077-
debug_assert_enough_capacity_for_length(builder, types.len(), values_vec_capacity);
1079+
self.debug_assert_enough_capacity_for_length(builder, types.len(), values_vec_capacity);
10781080

10791081
// Note that this is little-endian like `store_values_to_array` above,
10801082
// see notes there for more information.
@@ -1197,6 +1199,46 @@ impl Compiler {
11971199
pub fn tunables(&self) -> &Tunables {
11981200
&self.tunables
11991201
}
1202+
1203+
fn debug_assert_enough_capacity_for_length(
1204+
&self,
1205+
builder: &mut FunctionBuilder,
1206+
length: usize,
1207+
capacity: ir::Value,
1208+
) {
1209+
if !self.emit_debug_checks {
1210+
return;
1211+
}
1212+
let enough_capacity = builder.ins().icmp_imm(
1213+
ir::condcodes::IntCC::UnsignedGreaterThanOrEqual,
1214+
capacity,
1215+
ir::immediates::Imm64::new(length.try_into().unwrap()),
1216+
);
1217+
builder.ins().trapz(enough_capacity, TRAP_INTERNAL_ASSERT);
1218+
}
1219+
1220+
fn debug_assert_vmctx_kind(
1221+
&self,
1222+
builder: &mut FunctionBuilder,
1223+
vmctx: ir::Value,
1224+
expected_vmctx_magic: u32,
1225+
) {
1226+
if !self.emit_debug_checks {
1227+
return;
1228+
}
1229+
let magic = builder.ins().load(
1230+
ir::types::I32,
1231+
MemFlags::trusted().with_endianness(self.isa.endianness()),
1232+
vmctx,
1233+
0,
1234+
);
1235+
let is_expected_vmctx = builder.ins().icmp_imm(
1236+
ir::condcodes::IntCC::Equal,
1237+
magic,
1238+
i64::from(expected_vmctx_magic),
1239+
);
1240+
builder.ins().trapz(is_expected_vmctx, TRAP_INTERNAL_ASSERT);
1241+
}
12001242
}
12011243

12021244
struct FunctionCompiler<'a> {
@@ -1381,43 +1423,6 @@ fn declare_and_call(
13811423
builder.ins().call(callee, &args)
13821424
}
13831425

1384-
fn debug_assert_enough_capacity_for_length(
1385-
builder: &mut FunctionBuilder,
1386-
length: usize,
1387-
capacity: ir::Value,
1388-
) {
1389-
if cfg!(debug_assertions) {
1390-
let enough_capacity = builder.ins().icmp_imm(
1391-
ir::condcodes::IntCC::UnsignedGreaterThanOrEqual,
1392-
capacity,
1393-
ir::immediates::Imm64::new(length.try_into().unwrap()),
1394-
);
1395-
builder.ins().trapz(enough_capacity, TRAP_INTERNAL_ASSERT);
1396-
}
1397-
}
1398-
1399-
fn debug_assert_vmctx_kind(
1400-
isa: &dyn TargetIsa,
1401-
builder: &mut FunctionBuilder,
1402-
vmctx: ir::Value,
1403-
expected_vmctx_magic: u32,
1404-
) {
1405-
if cfg!(debug_assertions) {
1406-
let magic = builder.ins().load(
1407-
ir::types::I32,
1408-
MemFlags::trusted().with_endianness(isa.endianness()),
1409-
vmctx,
1410-
0,
1411-
);
1412-
let is_expected_vmctx = builder.ins().icmp_imm(
1413-
ir::condcodes::IntCC::Equal,
1414-
magic,
1415-
i64::from(expected_vmctx_magic),
1416-
);
1417-
builder.ins().trapz(is_expected_vmctx, TRAP_INTERNAL_ASSERT);
1418-
}
1419-
}
1420-
14211426
fn save_last_wasm_entry_fp(
14221427
builder: &mut FunctionBuilder,
14231428
pointer_type: ir::Type,

crates/cranelift/src/compiler/component.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1060,7 +1060,7 @@ impl<'a> TrampolineCompiler<'a> {
10601060
vmctx,
10611061
i32::try_from(self.offsets.resource_destructor(index)).unwrap(),
10621062
);
1063-
if cfg!(debug_assertions) {
1063+
if self.compiler.emit_debug_checks {
10641064
self.builder
10651065
.ins()
10661066
.trapz(dtor_func_ref, TRAP_INTERNAL_ASSERT);
@@ -1304,8 +1304,7 @@ impl ComponentCompiler for Compiler {
13041304
// always hold.
13051305
let vmctx = c.builder.block_params(c.block0)[0];
13061306
let pointer_type = self.isa.pointer_type();
1307-
super::debug_assert_vmctx_kind(
1308-
&*self.isa,
1307+
self.debug_assert_vmctx_kind(
13091308
&mut c.builder,
13101309
vmctx,
13111310
wasmtime_environ::component::VMCOMPONENT_MAGIC,

crates/misc/component-async-tests/tests/scenario/util.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ pub fn config() -> Config {
3131
config.signals_based_traps(false);
3232
} else {
3333
config.cranelift_debug_verifier(true);
34+
config.cranelift_wasmtime_debug_checks(true);
3435
}
3536
config.wasm_component_model(true);
3637
config.wasm_component_model_async(true);

crates/wasmtime/src/config.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1269,6 +1269,15 @@ impl Config {
12691269
self
12701270
}
12711271

1272+
/// Configures whether extra debug checks are inserted into
1273+
/// Wasmtime-generated code by Cranelift.
1274+
///
1275+
/// The default value for this is `false`
1276+
#[cfg(any(feature = "cranelift", feature = "winch"))]
1277+
pub fn cranelift_wasmtime_debug_checks(&mut self, enable: bool) -> &mut Self {
1278+
unsafe { self.cranelift_flag_set("wasmtime_debug_checks", &enable.to_string()) }
1279+
}
1280+
12721281
/// Configures the Cranelift code generator optimization level.
12731282
///
12741283
/// When the Cranelift code generator is used you can configure the

tests/disas/aarch64-entry-trampoline.wat

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,20 +7,12 @@
77
;; wasm[0]::array_to_wasm_trampoline[0]:
88
;; stp x29, x30, [sp, #-0x10]!
99
;; mov x29, sp
10-
;; ldr w10, [x0]
11-
;; mov w9, #0x6f63
12-
;; movk w9, #0x6572, lsl #16
13-
;; cmp w10, w9
14-
;; cset x13, eq
15-
;; uxtb w11, w13
16-
;; cbz x11, #0x58
17-
;; 34: ldr x12, [x0, #8]
18-
;; mov x13, x29
19-
;; str x13, [x12, #0x38]
10+
;; ldr x5, [x0, #8]
11+
;; mov x6, x29
12+
;; str x6, [x5, #0x38]
2013
;; mov x2, x0
2114
;; mov x3, x1
2215
;; bl #0
23-
;; 4c: mov w0, #1
16+
;; 30: mov w0, #1
2417
;; ldp x29, x30, [sp], #0x10
2518
;; ret
26-
;; 58: .byte 0x1f, 0xc1, 0x00, 0x00

tests/disas/epoch-interruption-x86.wat

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,12 @@
2828
;; jae 0x64
2929
;; jmp 0x46
3030
;; 57: movq %r15, %rdi
31-
;; callq 0xf2
31+
;; callq 0xe1
3232
;; jmp 0x46
3333
;; 64: movq 8(%r13), %rax
3434
;; cmpq %rax, %r11
3535
;; jb 0x46
3636
;; 71: movq %r15, %rdi
37-
;; callq 0xf2
37+
;; callq 0xe1
3838
;; jmp 0x46
3939
;; 7e: ud2

tests/disas/exceptions.wat

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,14 +33,14 @@
3333
;; movq %rdi, %r12
3434
;; movq %rcx, %r13
3535
;; movq %rdx, %r15
36-
;; callq 0x3c9
36+
;; callq 0x360
3737
;; movq %rax, %r14
3838
;; movl $0x4000000, %esi
3939
;; movl $3, %edx
4040
;; movl $0x30, %ecx
4141
;; movl $8, %r8d
4242
;; movq %r12, %rdi
43-
;; callq 0x355
43+
;; callq 0x2fd
4444
;; movq 8(%r12), %r8
4545
;; movq 0x18(%r8), %r8
4646
;; movl %eax, %r9d
@@ -54,7 +54,7 @@
5454
;; movq %rax, %rsi
5555
;; movq %r12, %rdi
5656
;; movq %r12, (%rsp)
57-
;; callq 0x405
57+
;; callq 0x38c
5858
;; ud2
5959
;; ud2
6060
;;

tests/disas/gc/struct-new-stack-map.wat

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@
3232
;; movl $0x28, %ecx
3333
;; movl $8, %r8d
3434
;; movq %rdi, %r13
35-
;; callq 0x161
35+
;; callq 0x14b
3636
;; movq 8(%r13), %r8
3737
;; ╰─╼ stack_map: frame_size=64, frame_offsets=[0]
3838
;; movq 0x18(%r8), %r8

tests/disas/pulley-entry-trampoline.wat

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,10 @@
66

77
;; wasm[0]::array_to_wasm_trampoline[0]:
88
;; push_frame
9-
;; xload32le_o32 x6, x0, 0
10-
;; br_if_xneq32_i32 x6, 1701998435, 0x25 // target = 0x30
11-
;; 15: xload64le_o32 x7, x0, 8
12-
;; xmov_fp x8
13-
;; xstore64le_o32 x7, 56, x8
14-
;; call -0x27 // target = 0x0
9+
;; xload64le_o32 x5, x0, 8
10+
;; xmov_fp x6
11+
;; xstore64le_o32 x5, 56, x6
12+
;; call -0x16 // target = 0x0
1513
;; xone x0
1614
;; pop_frame
1715
;; ret
18-
;; 30: trap

0 commit comments

Comments
 (0)