Skip to content

Commit e6691d0

Browse files
explodingcameraexplodingcamera
committed
fix: remove unsafe rewrites
Signed-off-by: Henry <mail@henrygressmann.de>
1 parent 4d829a8 commit e6691d0

5 files changed

Lines changed: 52 additions & 136 deletions

File tree

‎crates/parser/src/lib.rs‎

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,6 @@ pub struct ParserOptions {
5151
pub optimize_rewrite: bool,
5252
/// Whether to remove `Nop` and `MergeBarrier` instructions after rewriting.
5353
pub optimize_remove_nop: bool,
54-
/// Whether to invert conditional branches over an unconditional jump.
55-
pub optimize_branch_inversion: bool,
5654
}
5755

5856
impl Default for ParserOptions {
@@ -61,7 +59,6 @@ impl Default for ParserOptions {
6159
optimize_local_memory_allocation: true,
6260
optimize_rewrite: true,
6361
optimize_remove_nop: true,
64-
optimize_branch_inversion: false,
6562
}
6663
}
6764
}
@@ -100,16 +97,6 @@ impl ParserOptions {
10097
self.optimize_remove_nop
10198
}
10299

103-
/// Enable or disable the optimization that inverts conditional branches over an unconditional jump.
104-
pub const fn with_branch_inversion_optimization(mut self, enabled: bool) -> Self {
105-
self.optimize_branch_inversion = enabled;
106-
self
107-
}
108-
109-
/// Returns whether conditional branch inversion optimization is enabled.
110-
pub const fn optimize_branch_inversion(&self) -> bool {
111-
self.optimize_branch_inversion
112-
}
113100
}
114101

115102
/// A WebAssembly parser

‎crates/parser/src/macros.rs‎

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -222,25 +222,16 @@ pub(crate) mod optimize {
222222
get = $get:ident,
223223
tee = $tee:ident,
224224
set = $set:ident,
225-
copy = $copy:ident,
226225
binop_local_local_tee = $lltee:ident,
227226
binop_local_local_set = $llset:ident,
228227
binop_local_const_tee = $lctee:ident,
229228
binop_local_const_set = $lcset:ident
230229
$(, load_local_tee = $loadtee:ident, load_local_set = $loadset:ident)?
231230
) => {
232-
fn $name(instrs: &mut [Instruction], read: usize, instr: Instruction) -> Option<(Instruction, u16)> {
231+
fn $name(instr: Instruction) -> Option<(Instruction, u16)> {
233232
Some(match instr {
234233
Instruction::$get(local) => (Instruction::Nop, local),
235-
Instruction::$tee(local) => {
236-
let replacement = if let Some([(prev_idx, Instruction::$get(src))]) = previous_non_nop::<1>(instrs, read) {
237-
instrs[prev_idx] = Instruction::Nop;
238-
if src == local { Instruction::Nop } else { Instruction::$copy(src, local) }
239-
} else {
240-
Instruction::$set(local)
241-
};
242-
(replacement, local)
243-
}
234+
Instruction::$tee(local) => (Instruction::$set(local), local),
244235
Instruction::$lltee(op, a, b, local) => (Instruction::$llset(op, a, b, local), local),
245236
Instruction::$lctee(op, src, c, local) => (Instruction::$lcset(op, src, c, local), local),
246237
$(Instruction::$loadtee(memarg, addr, local) => (Instruction::$loadset(memarg, addr, local), local.into()),)?
@@ -261,10 +252,10 @@ pub(crate) mod optimize {
261252
) => {{
262253
if let Some([(lhs_idx, lhs_src), (rhs_idx, rhs_src), (op_idx, raw_op)]) =
263254
previous_non_nop::<3>($instrs, $read)
264-
&& let Some((lhs_instr, lhs)) = $source($instrs, lhs_idx, lhs_src)
255+
&& let Some((lhs_instr, lhs)) = $source(lhs_src)
265256
&& let Some(op) = $op(raw_op)
266257
{
267-
if let Some((rhs_instr, rhs)) = $source($instrs, rhs_idx, rhs_src) {
258+
if let Some((rhs_instr, rhs)) = $source(rhs_src) {
268259
$instrs[lhs_idx] = lhs_instr;
269260
$instrs[rhs_idx] = rhs_instr;
270261
$instrs[op_idx] = Instruction::Nop;

‎crates/parser/src/optimize.rs‎

Lines changed: 0 additions & 98 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ pub(crate) fn optimize_instructions(
2222
self_func_addr,
2323
imported_memory_count,
2424
track_local_memory_usage,
25-
options.optimize_branch_inversion(),
2625
)
2726
} else {
2827
track_local_memory_usage
@@ -40,7 +39,6 @@ fn rewrite(
4039
self_func_addr: u32,
4140
imported_memory_count: u32,
4241
track_local_memory_usage: bool,
43-
optimize_branch_inversion: bool,
4442
) -> bool {
4543
use Instruction::*;
4644
let mut uses_local_memory = false;
@@ -416,9 +414,6 @@ fn rewrite(
416414
(0, CmpOp::Ne) => JumpIfNonZero64(target),
417415
(imm, op) => JumpCmpStackConst64 { target_ip: target, imm, op },
418416
});
419-
if optimize_branch_inversion {
420-
invert_conditional_over_jump(instrs, i);
421-
}
422417
canonicalize_jump_like(instrs, i);
423418
if let JumpIfZero(current) = &mut instrs[i] {
424419
*current = target;
@@ -478,9 +473,6 @@ fn rewrite(
478473
(0, CmpOp::Ne) => JumpIfNonZero64(target),
479474
(imm, op) => JumpCmpStackConst64 { target_ip: target, imm, op },
480475
});
481-
if optimize_branch_inversion {
482-
invert_conditional_over_jump(instrs, i);
483-
}
484476
canonicalize_jump_like(instrs, i);
485477
if let JumpIfNonZero(current) = &mut instrs[i] {
486478
*current = target;
@@ -489,9 +481,6 @@ fn rewrite(
489481
JumpIfZero32(ip) => {
490482
let target = resolve_jump_target(instrs, ip);
491483
rewrite!(instrs, i, [LocalGet32(local)] => JumpIfLocalZero32 { target_ip: target, local });
492-
if optimize_branch_inversion {
493-
invert_conditional_over_jump(instrs, i);
494-
}
495484
canonicalize_jump_like(instrs, i);
496485
if let JumpIfZero32(current) = &mut instrs[i] {
497486
*current = target;
@@ -500,9 +489,6 @@ fn rewrite(
500489
JumpIfNonZero32(ip) => {
501490
let target = resolve_jump_target(instrs, ip);
502491
rewrite!(instrs, i, [LocalGet32(local)] => JumpIfLocalNonZero32 { target_ip: target, local });
503-
if optimize_branch_inversion {
504-
invert_conditional_over_jump(instrs, i);
505-
}
506492
canonicalize_jump_like(instrs, i);
507493
if let JumpIfNonZero32(current) = &mut instrs[i] {
508494
*current = target;
@@ -511,9 +497,6 @@ fn rewrite(
511497
JumpIfZero64(ip) => {
512498
let target = resolve_jump_target(instrs, ip);
513499
rewrite!(instrs, i, [LocalGet64(local)] => JumpIfLocalZero64 { target_ip: target, local });
514-
if optimize_branch_inversion {
515-
invert_conditional_over_jump(instrs, i);
516-
}
517500
canonicalize_jump_like(instrs, i);
518501
if let JumpIfZero64(current) = &mut instrs[i] {
519502
*current = target;
@@ -522,9 +505,6 @@ fn rewrite(
522505
JumpIfNonZero64(ip) => {
523506
let target = resolve_jump_target(instrs, ip);
524507
rewrite!(instrs, i, [LocalGet64(local)] => JumpIfLocalNonZero64 { target_ip: target, local });
525-
if optimize_branch_inversion {
526-
invert_conditional_over_jump(instrs, i);
527-
}
528508
canonicalize_jump_like(instrs, i);
529509
if let JumpIfNonZero64(current) = &mut instrs[i] {
530510
*current = target;
@@ -536,9 +516,6 @@ fn rewrite(
536516
CmpOp::Ne => instrs[i] = JumpIfNonZero32(target_ip),
537517
_ => {}
538518
}
539-
if optimize_branch_inversion {
540-
invert_conditional_over_jump(instrs, i);
541-
}
542519
canonicalize_jump_like(instrs, i);
543520
}
544521
JumpCmpStackConst64 { target_ip, imm: 0, op } => {
@@ -547,9 +524,6 @@ fn rewrite(
547524
CmpOp::Ne => instrs[i] = JumpIfNonZero64(target_ip),
548525
_ => {}
549526
}
550-
if optimize_branch_inversion {
551-
invert_conditional_over_jump(instrs, i);
552-
}
553527
canonicalize_jump_like(instrs, i);
554528
}
555529
JumpCmpLocalConst32 { target_ip, local, imm: 0, op } => {
@@ -558,9 +532,6 @@ fn rewrite(
558532
CmpOp::Ne => instrs[i] = JumpIfLocalNonZero32 { target_ip, local },
559533
_ => {}
560534
}
561-
if optimize_branch_inversion {
562-
invert_conditional_over_jump(instrs, i);
563-
}
564535
canonicalize_jump_like(instrs, i);
565536
}
566537
JumpCmpLocalConst64 { target_ip, local, imm: 0, op } => {
@@ -569,9 +540,6 @@ fn rewrite(
569540
CmpOp::Ne => instrs[i] = JumpIfLocalNonZero64 { target_ip, local },
570541
_ => {}
571542
}
572-
if optimize_branch_inversion {
573-
invert_conditional_over_jump(instrs, i);
574-
}
575543
canonicalize_jump_like(instrs, i);
576544
}
577545
JumpCmpStackConst32 { .. }
@@ -584,9 +552,6 @@ fn rewrite(
584552
| JumpIfLocalNonZero32 { .. }
585553
| JumpIfLocalZero64 { .. }
586554
| JumpIfLocalNonZero64 { .. } => {
587-
if optimize_branch_inversion {
588-
invert_conditional_over_jump(instrs, i);
589-
}
590555
canonicalize_jump_like(instrs, i);
591556
}
592557
_ => {}
@@ -710,7 +675,6 @@ define_local_source_resolver!(
710675
get = LocalGet32,
711676
tee = LocalTee32,
712677
set = LocalSet32,
713-
copy = LocalCopy32,
714678
binop_local_local_tee = BinOpLocalLocalTee32,
715679
binop_local_local_set = BinOpLocalLocalSet32,
716680
binop_local_const_tee = BinOpLocalConstTee32,
@@ -724,7 +688,6 @@ define_local_source_resolver!(
724688
get = LocalGet64,
725689
tee = LocalTee64,
726690
set = LocalSet64,
727-
copy = LocalCopy64,
728691
binop_local_local_tee = BinOpLocalLocalTee64,
729692
binop_local_local_set = BinOpLocalLocalSet64,
730693
binop_local_const_tee = BinOpLocalConstTee64,
@@ -736,7 +699,6 @@ define_local_source_resolver!(
736699
get = LocalGet128,
737700
tee = LocalTee128,
738701
set = LocalSet128,
739-
copy = LocalCopy128,
740702
binop_local_local_tee = BinOpLocalLocalTee128,
741703
binop_local_local_set = BinOpLocalLocalSet128,
742704
binop_local_const_tee = BinOpLocalConstTee128,
@@ -881,30 +843,6 @@ fn set_jump_target(instr: &mut Instruction, target: u32) {
881843
}
882844
}
883845

884-
fn invert_jump(instr: Instruction, target: u32) -> Option<Instruction> {
885-
Some(match instr {
886-
Instruction::JumpCmpStackConst32 { imm, op, .. } => {
887-
Instruction::JumpCmpStackConst32 { target_ip: target, imm, op: inverse_cmp_op(op) }
888-
}
889-
Instruction::JumpCmpStackConst64 { imm, op, .. } => {
890-
Instruction::JumpCmpStackConst64 { target_ip: target, imm, op: inverse_cmp_op(op) }
891-
}
892-
Instruction::JumpCmpLocalConst32 { local, imm, op, .. } => {
893-
Instruction::JumpCmpLocalConst32 { target_ip: target, local, imm, op: inverse_cmp_op(op) }
894-
}
895-
Instruction::JumpCmpLocalConst64 { local, imm, op, .. } => {
896-
Instruction::JumpCmpLocalConst64 { target_ip: target, local, imm, op: inverse_cmp_op(op) }
897-
}
898-
Instruction::JumpCmpLocalLocal32 { left, right, op, .. } => {
899-
Instruction::JumpCmpLocalLocal32 { target_ip: target, left, right, op: inverse_cmp_op(op) }
900-
}
901-
Instruction::JumpCmpLocalLocal64 { left, right, op, .. } => {
902-
Instruction::JumpCmpLocalLocal64 { target_ip: target, left, right, op: inverse_cmp_op(op) }
903-
}
904-
_ => return None,
905-
})
906-
}
907-
908846
fn canonicalize_jump_like(instrs: &mut [Instruction], idx: usize) {
909847
let Some(target) = jump_target(instrs[idx]) else {
910848
return;
@@ -918,42 +856,6 @@ fn canonicalize_jump_like(instrs: &mut [Instruction], idx: usize) {
918856
}
919857
}
920858

921-
fn invert_conditional_over_jump(instrs: &mut [Instruction], idx: usize) {
922-
let Some(target) = jump_target(instrs[idx]) else {
923-
return;
924-
};
925-
if matches!(instrs[idx], Instruction::Jump(_)) {
926-
return;
927-
}
928-
929-
let target_idx = next_non_nop(instrs, target as usize);
930-
if target_idx >= instrs.len() || target_idx <= idx + 1 {
931-
return;
932-
}
933-
934-
let Some(jump_idx) = ((idx + 1)..target_idx)
935-
.rev()
936-
.find(|&candidate| !matches!(instrs[candidate], Instruction::Nop | Instruction::MergeBarrier))
937-
else {
938-
return;
939-
};
940-
941-
let Instruction::Jump(exit_target) = instrs[jump_idx] else {
942-
return;
943-
};
944-
if next_non_nop(instrs, jump_idx + 1) != target_idx {
945-
return;
946-
}
947-
948-
let exit_target = resolve_jump_target(instrs, exit_target);
949-
let Some(inverted) = invert_jump(instrs[idx], exit_target) else {
950-
return;
951-
};
952-
953-
instrs[idx] = inverted;
954-
instrs[jump_idx] = Instruction::Nop;
955-
}
956-
957859
fn remove_nop(instructions: &mut Vec<Instruction>, function_data: &mut WasmFunctionData) {
958860
let old_len = instructions.len();
959861
if old_len == 0 {

‎crates/parser/src/visit.rs‎

Lines changed: 46 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -218,14 +218,52 @@ impl<'a, R: WasmModuleResources> wasmparser::VisitOperator<'a> for FunctionBuild
218218
fn visit_local_tee(&mut self, idx: u32) -> Self::Output {
219219
let resolved_idx = self.local_addr_map[idx as usize];
220220
if let Some(Some(t)) = self.validator.get_operand_type(0) {
221-
self.instructions.push(match t {
222-
wasmparser::ValType::I32 => Instruction::LocalTee32(resolved_idx),
223-
wasmparser::ValType::F32 => Instruction::LocalTee32(resolved_idx),
224-
wasmparser::ValType::I64 => Instruction::LocalTee64(resolved_idx),
225-
wasmparser::ValType::F64 => Instruction::LocalTee64(resolved_idx),
226-
wasmparser::ValType::V128 => Instruction::LocalTee128(resolved_idx),
227-
wasmparser::ValType::Ref(_) => Instruction::LocalTee32(resolved_idx),
228-
})
221+
let last = self.instructions.last();
222+
let src = match t {
223+
wasmparser::ValType::I32 | wasmparser::ValType::F32 => {
224+
if let Some(Instruction::LocalGet32(src)) = last { Some(*src) } else { None }
225+
}
226+
wasmparser::ValType::I64 | wasmparser::ValType::F64 => {
227+
if let Some(Instruction::LocalGet64(src)) = last { Some(*src) } else { None }
228+
}
229+
wasmparser::ValType::V128 => {
230+
if let Some(Instruction::LocalGet128(src)) = last { Some(*src) } else { None }
231+
}
232+
wasmparser::ValType::Ref(_) => {
233+
if let Some(Instruction::LocalGet32(src)) = last { Some(*src) } else { None }
234+
}
235+
};
236+
237+
if let Some(src) = src {
238+
self.instructions.pop();
239+
match t {
240+
wasmparser::ValType::I32 | wasmparser::ValType::F32 => {
241+
self.instructions.push(Instruction::LocalCopy32(src, resolved_idx));
242+
self.instructions.push(Instruction::LocalGet32(resolved_idx));
243+
}
244+
wasmparser::ValType::I64 | wasmparser::ValType::F64 => {
245+
self.instructions.push(Instruction::LocalCopy64(src, resolved_idx));
246+
self.instructions.push(Instruction::LocalGet64(resolved_idx));
247+
}
248+
wasmparser::ValType::V128 => {
249+
self.instructions.push(Instruction::LocalCopy128(src, resolved_idx));
250+
self.instructions.push(Instruction::LocalGet128(resolved_idx));
251+
}
252+
wasmparser::ValType::Ref(_) => {
253+
self.instructions.push(Instruction::LocalCopy32(src, resolved_idx));
254+
self.instructions.push(Instruction::LocalGet32(resolved_idx));
255+
}
256+
}
257+
} else {
258+
self.instructions.push(match t {
259+
wasmparser::ValType::I32 => Instruction::LocalTee32(resolved_idx),
260+
wasmparser::ValType::F32 => Instruction::LocalTee32(resolved_idx),
261+
wasmparser::ValType::I64 => Instruction::LocalTee64(resolved_idx),
262+
wasmparser::ValType::F64 => Instruction::LocalTee64(resolved_idx),
263+
wasmparser::ValType::V128 => Instruction::LocalTee128(resolved_idx),
264+
wasmparser::ValType::Ref(_) => Instruction::LocalTee32(resolved_idx),
265+
})
266+
}
229267
}
230268
}
231269

‎crates/tinywasm/src/interpreter/values.rs‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -191,10 +191,8 @@ macro_rules! impl_internalvalue {
191191
#[inline(always)]
192192
fn local_set(stack: &mut ValueStack, frame: &CallFrame, index: LocalAddr, value: Self) {
193193
let $to_stack_v = value;
194-
stack.$stack.set(
195-
frame.locals_base.$stack_base as usize + index as usize,
196-
$to_stack,
197-
);
194+
let abs_index = frame.locals_base.$stack_base as usize + index as usize;
195+
stack.$stack.set(abs_index, $to_stack);
198196
}
199197

200198
#[inline(always)]

0 commit comments

Comments
 (0)