From 4335a8cfbb8823219128f6547146d938e12ccba3 Mon Sep 17 00:00:00 2001
From: Zhou Fang <33002388+zhou-w-fang@users.noreply.github.com>
Date: Sun, 17 Nov 2024 21:08:15 -0800
Subject: [PATCH 1/6] feat: design skip_to()

---
 crates/interpreter/src/parser.rs | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/crates/interpreter/src/parser.rs b/crates/interpreter/src/parser.rs
index 730f11d1..1dbf0f80 100644
--- a/crates/interpreter/src/parser.rs
+++ b/crates/interpreter/src/parser.rs
@@ -64,6 +64,10 @@ impl<'m, M: Mode> Parser<'m, M> {
         Ok(result)
     }
 
+    pub fn parse_bytes_reversely(&mut self, delta: i32) -> MResult<&'m [u8], M> {
+        unimplemented!()
+    }
+
     pub fn split_at(&mut self, len: usize) -> MResult<Parser<'m, M>, M> {
         Ok(Self::internal_new(self.parse_bytes(len)?))
     }
@@ -564,6 +568,15 @@ impl<'m, M: Mode> Parser<'m, M> {
         }
         Ok(())
     }
+
+    pub fn skip_to(&mut self, delta: i32) -> MResult<(), M> {
+        if delta >= 0 {
+            self.parse_bytes(delta as usize)?;
+        } else {
+            self.parse_bytes_reversely(delta)?;
+        }
+        Ok(())
+    }
 }
 
 pub trait ParseElem<'m, M: Mode> {

From b1e636b1995ea2003c4886ac59d6d0ed6e0c3288 Mon Sep 17 00:00:00 2001
From: Zhou Fang <33002388+zhou-w-fang@users.noreply.github.com>
Date: Sat, 30 Nov 2024 17:22:51 -0800
Subject: [PATCH 2/6] feat!: apply delta_ip and delta_stp in exec.rs

---
 crates/interpreter/src/parser.rs | 13 -------------
 1 file changed, 13 deletions(-)

diff --git a/crates/interpreter/src/parser.rs b/crates/interpreter/src/parser.rs
index 1dbf0f80..730f11d1 100644
--- a/crates/interpreter/src/parser.rs
+++ b/crates/interpreter/src/parser.rs
@@ -64,10 +64,6 @@ impl<'m, M: Mode> Parser<'m, M> {
         Ok(result)
     }
 
-    pub fn parse_bytes_reversely(&mut self, delta: i32) -> MResult<&'m [u8], M> {
-        unimplemented!()
-    }
-
     pub fn split_at(&mut self, len: usize) -> MResult<Parser<'m, M>, M> {
         Ok(Self::internal_new(self.parse_bytes(len)?))
     }
@@ -568,15 +564,6 @@ impl<'m, M: Mode> Parser<'m, M> {
         }
         Ok(())
     }
-
-    pub fn skip_to(&mut self, delta: i32) -> MResult<(), M> {
-        if delta >= 0 {
-            self.parse_bytes(delta as usize)?;
-        } else {
-            self.parse_bytes_reversely(delta)?;
-        }
-        Ok(())
-    }
 }
 
 pub trait ParseElem<'m, M: Mode> {

From 03a5bfa677da74ab302b14810574d68f96f994d6 Mon Sep 17 00:00:00 2001
From: Zhou Fang <33002388+zhou-w-fang@users.noreply.github.com>
Date: Tue, 10 Dec 2024 21:11:13 -0800
Subject: [PATCH 3/6] refactor: apply->restore side table for loop

---
 crates/interpreter/src/exec.rs | 22 +++++-----------------
 1 file changed, 5 insertions(+), 17 deletions(-)

diff --git a/crates/interpreter/src/exec.rs b/crates/interpreter/src/exec.rs
index 84238bc6..349d8c2d 100644
--- a/crates/interpreter/src/exec.rs
+++ b/crates/interpreter/src/exec.rs
@@ -766,13 +766,7 @@ impl<'m> Thread<'m> {
             Unreachable => return Err(trap()),
             Nop => (),
             Block(b) => self.push_label(self.blocktype(inst, &b), LabelKind::Block),
-            Loop(b) => {
-                let side_table = self.frame().side_table;
-                self.push_label(
-                    self.blocktype(inst, &b),
-                    LabelKind::Loop(LoopState { parser_data: saved, side_table }),
-                )
-            }
+            Loop(b) => self.push_label(self.blocktype(inst, &b), LabelKind::Loop(saved)),
             If(b) => match self.pop_value().unwrap_i32() {
                 0 => {
                     self.take_jump(0);
@@ -1064,9 +1058,9 @@ impl<'m> Thread<'m> {
         self.values().drain(values_len - values_cnt .. values_len - arity);
         self.label().values_cnt += arity;
         match kind {
-            LabelKind::Loop(state) => unsafe {
-                self.frame().side_table = state.side_table;
-                self.parser.restore(state.parser_data)
+            LabelKind::Loop(parser_data) => unsafe {
+                self.take_jump(offset);
+                self.parser.restore(parser_data)
             },
             LabelKind::Block | LabelKind::If => self.take_jump(offset),
         }
@@ -1454,12 +1448,6 @@ struct Label<'m> {
     values_cnt: usize,
 }
 
-#[derive(Debug)]
-struct LoopState<'m> {
-    parser_data: &'m [u8],
-    side_table: &'m [SideTableEntry],
-}
-
 #[derive(Debug)]
 enum LabelKind<'m> {
     // TODO: If and Block can be merged and then we just have Option<NonNull<u8>> which is
@@ -1467,7 +1455,7 @@ enum LabelKind<'m> {
     Block,
     // TODO: Could be just NonNull<u8> since we can reuse the end of current parser since it
     // never changes.
-    Loop(LoopState<'m>),
+    Loop(&'m [u8]),
     If,
 }
 

From d1f028b0e632b35baf83a1cc6512622056efd4bb Mon Sep 17 00:00:00 2001
From: Zhou Fang <33002388+zhou-w-fang@users.noreply.github.com>
Date: Wed, 11 Dec 2024 14:32:13 -0800
Subject: [PATCH 4/6] fix!: delta_ip-=2 for loop

---
 crates/interpreter/src/exec.rs  | 30 ++++++++++++------------------
 crates/interpreter/src/valid.rs | 20 +++++++++++++-------
 2 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/crates/interpreter/src/exec.rs b/crates/interpreter/src/exec.rs
index 349d8c2d..6845da86 100644
--- a/crates/interpreter/src/exec.rs
+++ b/crates/interpreter/src/exec.rs
@@ -15,6 +15,7 @@
 // TODO: Some toctou could be used instead of panic.
 use alloc::vec;
 use alloc::vec::Vec;
+use core::fmt::Debug;
 
 use crate::error::*;
 use crate::module::*;
@@ -759,14 +760,13 @@ impl<'m> Thread<'m> {
 
     fn step(mut self, store: &mut Store<'m>) -> Result<ThreadResult<'m>, Error> {
         use Instr::*;
-        let saved = self.parser.save();
         let inst_id = self.frame().inst_id;
         let inst = &mut store.insts[inst_id];
         match self.parser.parse_instr().into_ok() {
             Unreachable => return Err(trap()),
             Nop => (),
             Block(b) => self.push_label(self.blocktype(inst, &b), LabelKind::Block),
-            Loop(b) => self.push_label(self.blocktype(inst, &b), LabelKind::Loop(saved)),
+            Loop(b) => self.push_label(self.blocktype(inst, &b), LabelKind::Loop),
             If(b) => match self.pop_value().unwrap_i32() {
                 0 => {
                     self.take_jump(0);
@@ -982,11 +982,11 @@ impl<'m> Thread<'m> {
         self.frames.last_mut().unwrap()
     }
 
-    fn labels(&mut self) -> &mut Vec<Label<'m>> {
+    fn labels(&mut self) -> &mut Vec<Label> {
         &mut self.frame().labels
     }
 
-    fn label(&mut self) -> &mut Label<'m> {
+    fn label(&mut self) -> &mut Label {
         self.labels().last_mut().unwrap()
     }
 
@@ -1036,10 +1036,10 @@ impl<'m> Thread<'m> {
         self.values().split_off(len)
     }
 
-    fn push_label(&mut self, type_: FuncType<'m>, kind: LabelKind<'m>) {
+    fn push_label(&mut self, type_: FuncType<'m>, kind: LabelKind) {
         let arity = match kind {
             LabelKind::Block | LabelKind::If => type_.results.len(),
-            LabelKind::Loop(_) => type_.params.len(),
+            LabelKind::Loop => type_.params.len(),
         };
         let label = Label { arity, kind, values_cnt: type_.params.len() };
         self.label().values_cnt -= label.values_cnt;
@@ -1058,11 +1058,7 @@ impl<'m> Thread<'m> {
         self.values().drain(values_len - values_cnt .. values_len - arity);
         self.label().values_cnt += arity;
         match kind {
-            LabelKind::Loop(parser_data) => unsafe {
-                self.take_jump(offset);
-                self.parser.restore(parser_data)
-            },
-            LabelKind::Block | LabelKind::If => self.take_jump(offset),
+            LabelKind::Block | LabelKind::If | LabelKind::Loop => self.take_jump(offset),
         }
         ThreadResult::Continue(self)
     }
@@ -1416,7 +1412,7 @@ struct Frame<'m> {
     arity: usize,
     ret: &'m [u8],
     locals: Vec<Val>,
-    labels: Vec<Label<'m>>,
+    labels: Vec<Label>,
     side_table: &'m [SideTableEntry],
 }
 
@@ -1442,20 +1438,18 @@ impl<'m> Frame<'m> {
 }
 
 #[derive(Debug)]
-struct Label<'m> {
+struct Label {
     arity: usize,
-    kind: LabelKind<'m>,
+    kind: LabelKind,
     values_cnt: usize,
 }
 
 #[derive(Debug)]
-enum LabelKind<'m> {
+enum LabelKind {
     // TODO: If and Block can be merged and then we just have Option<NonNull<u8>> which is
     // optimized.
     Block,
-    // TODO: Could be just NonNull<u8> since we can reuse the end of current parser since it
-    // never changes.
-    Loop(&'m [u8]),
+    Loop,
     If,
 }
 
diff --git a/crates/interpreter/src/valid.rs b/crates/interpreter/src/valid.rs
index 97e799be..9f3eb299 100644
--- a/crates/interpreter/src/valid.rs
+++ b/crates/interpreter/src/valid.rs
@@ -586,7 +586,9 @@ impl<'a, 'm> Expr<'a, 'm> {
             Block(b) => self.push_label(self.blocktype(&b)?, LabelKind::Block)?,
             Loop(b) => {
                 let type_ = self.blocktype(&b)?;
-                self.push_label(type_, LabelKind::Loop(self.branch_target(type_.params.len())))?
+                let mut target = self.branch_target(type_.params.len());
+                target.parser = offset_parser(target, -2);
+                self.push_label(type_, LabelKind::Loop(target))?
             }
             If(b) => {
                 self.pop_check(ValType::I32)?;
@@ -859,12 +861,7 @@ impl<'a, 'm> Expr<'a, 'm> {
         if let LabelKind::If(source) = label.kind {
             check(label.type_.params == label.type_.results)?;
             // SAFETY: This function is only called after parsing an End instruction.
-            target.parser = unsafe {
-                core::slice::from_raw_parts(
-                    target.parser.as_ptr().offset(-1),
-                    target.parser.len() + 1,
-                )
-            };
+            target.parser = offset_parser(target, -1);
             self.side_table.stitch(source, target)?;
         }
         let results = self.label().type_.results;
@@ -979,3 +976,12 @@ impl<'a, 'm> Expr<'a, 'm> {
         }
     }
 }
+
+fn offset_parser(branch: SideTableBranch, off: isize) -> &[u8] {
+    unsafe {
+        core::slice::from_raw_parts(
+            branch.parser.as_ptr().offset(off),
+            (branch.parser.len() as isize - off) as usize,
+        )
+    }
+}

From 2824362bec3fa5a8b37b49a83eccdfa979237e1c Mon Sep 17 00:00:00 2001
From: Zhou Fang <33002388+zhou-w-fang@users.noreply.github.com>
Date: Wed, 11 Dec 2024 14:44:23 -0800
Subject: [PATCH 5/6] chore: remove redundant import

---
 crates/interpreter/src/exec.rs | 1 -
 1 file changed, 1 deletion(-)

diff --git a/crates/interpreter/src/exec.rs b/crates/interpreter/src/exec.rs
index 6845da86..e312b1a9 100644
--- a/crates/interpreter/src/exec.rs
+++ b/crates/interpreter/src/exec.rs
@@ -15,7 +15,6 @@
 // TODO: Some toctou could be used instead of panic.
 use alloc::vec;
 use alloc::vec::Vec;
-use core::fmt::Debug;
 
 use crate::error::*;
 use crate::module::*;

From e57936a84f509d7000d27d96d0057d3eb5b20a66 Mon Sep 17 00:00:00 2001
From: Zhou Fang <33002388+zhou-w-fang@users.noreply.github.com>
Date: Thu, 12 Dec 2024 10:45:45 -0800
Subject: [PATCH 6/6] refactor: review

---
 crates/interpreter/src/exec.rs  | 20 +++++---------------
 crates/interpreter/src/lib.rs   |  1 +
 crates/interpreter/src/util.rs  | 21 +++++++++++++++++++++
 crates/interpreter/src/valid.rs | 15 ++++-----------
 4 files changed, 31 insertions(+), 26 deletions(-)
 create mode 100644 crates/interpreter/src/util.rs

diff --git a/crates/interpreter/src/exec.rs b/crates/interpreter/src/exec.rs
index e312b1a9..eaac31b3 100644
--- a/crates/interpreter/src/exec.rs
+++ b/crates/interpreter/src/exec.rs
@@ -21,6 +21,7 @@ use crate::module::*;
 use crate::side_table::SideTableEntry;
 use crate::syntax::*;
 use crate::toctou::*;
+use crate::util::*;
 use crate::*;
 
 pub const MEMORY_ALIGN: usize = 16;
@@ -1040,7 +1041,7 @@ impl<'m> Thread<'m> {
             LabelKind::Block | LabelKind::If => type_.results.len(),
             LabelKind::Loop => type_.params.len(),
         };
-        let label = Label { arity, kind, values_cnt: type_.params.len() };
+        let label = Label { arity, values_cnt: type_.params.len() };
         self.label().values_cnt -= label.values_cnt;
         self.labels().push(label);
     }
@@ -1052,13 +1053,11 @@ impl<'m> Thread<'m> {
         }
         let frame = self.frame();
         let values_cnt: usize = frame.labels[i ..].iter().map(|label| label.values_cnt).sum();
-        let Label { arity, kind, .. } = frame.labels.drain(i ..).next().unwrap();
+        let Label { arity, .. } = frame.labels.drain(i ..).next().unwrap();
         let values_len = self.values().len();
         self.values().drain(values_len - values_cnt .. values_len - arity);
         self.label().values_cnt += arity;
-        match kind {
-            LabelKind::Block | LabelKind::If | LabelKind::Loop => self.take_jump(offset),
-        }
+        self.take_jump(offset);
         ThreadResult::Continue(self)
     }
 
@@ -1420,7 +1419,7 @@ impl<'m> Frame<'m> {
         inst_id: usize, arity: usize, ret: &'m [u8], locals: Vec<Val>,
         side_table: &'m [SideTableEntry],
     ) -> Self {
-        let label = Label { arity, kind: LabelKind::Block, values_cnt: 0 };
+        let label = Label { arity, values_cnt: 0 };
         Frame { inst_id, arity, ret, locals, labels: vec![label], side_table }
     }
 
@@ -1439,7 +1438,6 @@ impl<'m> Frame<'m> {
 #[derive(Debug)]
 struct Label {
     arity: usize,
-    kind: LabelKind,
     values_cnt: usize,
 }
 
@@ -1625,11 +1623,3 @@ fn memory_too_small(x: usize, n: usize, mem: &Memory) {
     #[cfg(feature = "debug")]
     eprintln!("Memory too small: {x} + {n} > {}", mem.len());
 }
-
-// TODO(dev/fast-interp): Add debug asserts when `off` is positive and negative, and `toctou`
-// support.
-fn offset_front<T>(cur: &[T], off: isize) -> &[T] {
-    unsafe {
-        core::slice::from_raw_parts(cur.as_ptr().offset(off), (cur.len() as isize - off) as usize)
-    }
-}
diff --git a/crates/interpreter/src/lib.rs b/crates/interpreter/src/lib.rs
index 05ce2534..daa1bbe4 100644
--- a/crates/interpreter/src/lib.rs
+++ b/crates/interpreter/src/lib.rs
@@ -148,6 +148,7 @@ mod parser;
 mod side_table;
 mod syntax;
 mod toctou;
+mod util;
 mod valid;
 
 pub use error::{Error, TRAP_CODE, Unsupported};
diff --git a/crates/interpreter/src/util.rs b/crates/interpreter/src/util.rs
new file mode 100644
index 00000000..def1feb5
--- /dev/null
+++ b/crates/interpreter/src/util.rs
@@ -0,0 +1,21 @@
+// Copyright 2024 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+// TODO(dev/fast-interp): Add debug asserts when `off` is positive and negative, and `toctou`
+// support.
+pub fn offset_front<T>(cur: &[T], off: isize) -> &[T] {
+    unsafe {
+        core::slice::from_raw_parts(cur.as_ptr().offset(off), (cur.len() as isize - off) as usize)
+    }
+}
diff --git a/crates/interpreter/src/valid.rs b/crates/interpreter/src/valid.rs
index 9f3eb299..0df81d0b 100644
--- a/crates/interpreter/src/valid.rs
+++ b/crates/interpreter/src/valid.rs
@@ -21,6 +21,7 @@ use crate::error::*;
 use crate::side_table::*;
 use crate::syntax::*;
 use crate::toctou::*;
+use crate::util::*;
 use crate::*;
 
 /// Checks whether a WASM module in binary format is valid.
@@ -558,6 +559,7 @@ impl<'a, 'm> Expr<'a, 'm> {
 
     fn instr(&mut self) -> CheckResult {
         use Instr::*;
+        let saved = self.parser.save();
         let instr = self.parser.parse_instr()?;
         if matches!(instr, End) {
             return self.end_label();
@@ -587,7 +589,7 @@ impl<'a, 'm> Expr<'a, 'm> {
             Loop(b) => {
                 let type_ = self.blocktype(&b)?;
                 let mut target = self.branch_target(type_.params.len());
-                target.parser = offset_parser(target, -2);
+                target.parser = saved;
                 self.push_label(type_, LabelKind::Loop(target))?
             }
             If(b) => {
@@ -861,7 +863,7 @@ impl<'a, 'm> Expr<'a, 'm> {
         if let LabelKind::If(source) = label.kind {
             check(label.type_.params == label.type_.results)?;
             // SAFETY: This function is only called after parsing an End instruction.
-            target.parser = offset_parser(target, -1);
+            target.parser = offset_front(target.parser, -1);
             self.side_table.stitch(source, target)?;
         }
         let results = self.label().type_.results;
@@ -976,12 +978,3 @@ impl<'a, 'm> Expr<'a, 'm> {
         }
     }
 }
-
-fn offset_parser(branch: SideTableBranch, off: isize) -> &[u8] {
-    unsafe {
-        core::slice::from_raw_parts(
-            branch.parser.as_ptr().offset(off),
-            (branch.parser.len() as isize - off) as usize,
-        )
-    }
-}