From 7a3cb539e1f1b28a54f07ea2737df7777fb4428b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Hru=C5=A1ka?= Date: Sat, 10 Oct 2020 15:39:18 +0200 Subject: [PATCH] add new rdwr access type to guard against illegal writes that can be caught at compile time --- crsn/src/asm/data/mod.rs | 9 +- crsn/src/asm/data/rd.rs | 3 - crsn/src/asm/data/rdwr.rs | 59 ++++++++++++ crsn/src/asm/data/wr.rs | 11 ++- crsn/src/asm/parse/arg_parser.rs | 15 ++- crsn/src/builtin/defs.rs | 4 +- crsn/src/builtin/exec.rs | 8 +- crsn/src/builtin/parse.rs | 4 +- crsn/src/runtime/run_thread/state.rs | 11 ++- crsn_arith/src/parse.rs | 132 +++++++++++++-------------- 10 files changed, 168 insertions(+), 88 deletions(-) create mode 100644 crsn/src/asm/data/rdwr.rs diff --git a/crsn/src/asm/data/mod.rs b/crsn/src/asm/data/mod.rs index 179cd37..4bda70e 100644 --- a/crsn/src/asm/data/mod.rs +++ b/crsn/src/asm/data/mod.rs @@ -6,6 +6,7 @@ pub use rd::Rd; pub use rd::RdObj; pub use reg::Register; pub use wr::Wr; +pub use rdwr::RdWr; use crate::asm::data::literal::{as_signed, is_negative, Value}; @@ -15,8 +16,8 @@ pub mod literal; pub mod reg; mod rd; - mod wr; +mod rdwr; /// Data source disposition #[derive(Debug, Clone, Copy, Eq, PartialEq)] @@ -134,6 +135,12 @@ pub enum WrData { Discard, } +impl WrData { + pub fn is_readable(self) -> bool { + self != Self::Discard + } +} + impl Display for WrData { fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { match self { diff --git a/crsn/src/asm/data/rd.rs b/crsn/src/asm/data/rd.rs index 04908fc..774a1d2 100644 --- a/crsn/src/asm/data/rd.rs +++ b/crsn/src/asm/data/rd.rs @@ -12,9 +12,6 @@ impl Rd { pub const fn new(src: RdData) -> Self { Rd(src) } - pub const fn data(self) -> RdData { - self.0 - } pub const fn immediate(val: Value) -> Rd { Rd(RdData::Immediate(val)) } diff --git a/crsn/src/asm/data/rdwr.rs b/crsn/src/asm/data/rdwr.rs new file mode 100644 index 0000000..0ba603f --- /dev/null +++ b/crsn/src/asm/data/rdwr.rs @@ -0,0 +1,59 @@ +use std::fmt::{Debug, Display, Formatter}; +use std::fmt; + +use crate::asm::data::{Rd, WrData, Wr}; + +/// Data destination argument (read-write) +#[derive(Clone, Copy, Eq, PartialEq)] +pub struct RdWr(pub WrData); + +impl RdWr { + pub fn new(dst: WrData) -> Self { + if !dst.is_readable() { + panic!("Not readable: {}", dst); + } + RdWr(dst) + } + pub fn wr(self) -> Wr { + Wr(self.0) + } + pub fn rd(self) -> Rd { + Rd(self.0.into()) + } +} + +impl From for Rd { + fn from(rdwr: RdWr) -> Self { + rdwr.rd() + } +} + +impl From for Wr { + fn from(rdwr: RdWr) -> Self { + rdwr.wr() + } +} + +impl From<&RdWr> for Rd { + fn from(rdwr: &RdWr) -> Self { + rdwr.rd() + } +} + +impl From<&RdWr> for Wr { + fn from(rdwr: &RdWr) -> Self { + rdwr.wr() + } +} + +impl Display for RdWr { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + write!(f, "{}", self.0) + } +} + +impl Debug for RdWr { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + write!(f, "RdWr({})", self.0) + } +} diff --git a/crsn/src/asm/data/wr.rs b/crsn/src/asm/data/wr.rs index e18ec6e..9ff9c40 100644 --- a/crsn/src/asm/data/wr.rs +++ b/crsn/src/asm/data/wr.rs @@ -5,21 +5,22 @@ use crate::asm::data::{Rd, WrData}; /// Data destination argument (read-write) #[derive(Clone, Copy, Eq, PartialEq)] -pub struct Wr(WrData); +pub struct Wr(pub WrData); impl Wr { pub fn new(dst: WrData) -> Self { Wr(dst) } - pub fn d(self) -> WrData { - self.0 + pub fn discard() -> Wr { + Wr(WrData::Discard) } + pub fn as_rd(self) -> Rd { Rd(self.0.into()) } - pub fn discard() -> Wr { - Wr(WrData::Discard) + pub fn is_readable(self) -> bool { + self.0.is_readable() } pub fn is_discard(self) -> bool { diff --git a/crsn/src/asm/parse/arg_parser.rs b/crsn/src/asm/parse/arg_parser.rs index 59cbe7b..4c2385b 100644 --- a/crsn/src/asm/parse/arg_parser.rs +++ b/crsn/src/asm/parse/arg_parser.rs @@ -1,6 +1,6 @@ use sexp::{Sexp, SourcePosition}; -use crate::asm::data::{Rd, RdData, RdObj, Wr}; +use crate::asm::data::{Rd, RdData, RdObj, Wr, RdWr}; use crate::asm::error::CrsnError; use crate::asm::parse::parse_data::{parse_rd, parse_wr}; use crate::asm::parse::ParserContext; @@ -109,4 +109,17 @@ impl<'a> TokenParser<'a> { pub fn next_wr(&mut self) -> Result { parse_wr(self.next_or_err()?, self.pcx) } + + /// Get the next entry as write location + pub fn next_rdwr(&mut self) -> Result { + let next = self.next_or_err()?; + let pos = next.pos().clone(); + let wr = parse_wr(next, self.pcx)?; + + if !wr.is_readable() { + return Err(CrsnError::Parse("Argument is not readable!".into(), pos)); + } + + Ok(RdWr::new(wr.0)) + } } diff --git a/crsn/src/builtin/defs.rs b/crsn/src/builtin/defs.rs index 9c18cf8..607add3 100644 --- a/crsn/src/builtin/defs.rs +++ b/crsn/src/builtin/defs.rs @@ -1,6 +1,6 @@ use sexp::SourcePosition; -use crate::asm::data::{Rd, RdObj, Wr}; +use crate::asm::data::{Rd, RdObj, Wr, RdWr}; use crate::asm::data::literal::{DebugMsg, Label, RoutineName}; use crate::asm::instr::Op; use crate::asm::instr::op::OpKind; @@ -73,7 +73,7 @@ pub enum BuiltinOp { /// Copy value MoveValue { dst: Wr, src: Rd }, /// Swap two registers - SwapValues { a: Wr, b: Wr }, + SwapValues { a: RdWr, b: RdWr }, /// Store runtime status to a register StoreFlags { dst: Wr }, /// Load runtime status from a register diff --git a/crsn/src/builtin/exec.rs b/crsn/src/builtin/exec.rs index bad66b5..91ca27e 100644 --- a/crsn/src/builtin/exec.rs +++ b/crsn/src/builtin/exec.rs @@ -110,10 +110,10 @@ impl OpTrait for BuiltinOp { state.write(*dst, val)?; } BuiltinOp::SwapValues { a, b } => { - let aa = state.read(a.as_rd())?; - let bb = state.read(b.as_rd())?; - state.write(*a, bb)?; - state.write(*b, aa)?; + let aa = state.read(a)?; + let bb = state.read(b)?; + state.write(a, bb)?; + state.write(b, aa)?; } BuiltinOp::StoreFlags { dst } => { let packed = state.frame.status.store(); diff --git a/crsn/src/builtin/parse.rs b/crsn/src/builtin/parse.rs index 101d130..e2dd050 100644 --- a/crsn/src/builtin/parse.rs +++ b/crsn/src/builtin/parse.rs @@ -185,8 +185,8 @@ pub(crate) fn parse_op<'a>(op_pos: &SourcePosition, keyword: &str, mut args: Tok "swap" => { BuiltinOp::SwapValues { - a: args.next_wr()?, - b: args.next_wr()?, + a: args.next_rdwr()?, + b: args.next_rdwr()?, } } diff --git a/crsn/src/runtime/run_thread/state.rs b/crsn/src/runtime/run_thread/state.rs index 043e8eb..8eec364 100644 --- a/crsn/src/runtime/run_thread/state.rs +++ b/crsn/src/runtime/run_thread/state.rs @@ -93,8 +93,9 @@ impl RunState { } /// Read a `Rd` value - pub fn read(&mut self, rd: Rd) -> Result { - match rd.data() { + pub fn read(&mut self, rd: impl Into) -> Result { + let rd = rd.into(); + match rd.0 { RdData::Register(Register::Gen(rn)) => { if likely(rn < REG_COUNT as u8) { trace!("Rd {:?} = {}", rd, self.frame.gen[rn as usize]); @@ -161,10 +162,12 @@ impl RunState { } /// Write a value to a `Wr` location - pub fn write(&mut self, wr: Wr, val: Value) -> Result<(), Fault> { + pub fn write(&mut self, wr: impl Into, val: Value) -> Result<(), Fault> { + let wr = wr.into(); + trace!("WR {:?} := {}", wr, val); - match wr.d() { + match wr.0 { WrData::Register(Register::Gen(rn)) => { if likely(rn < REG_COUNT as u8) { self.frame.gen[rn as usize] = val; diff --git a/crsn_arith/src/parse.rs b/crsn_arith/src/parse.rs index 626b0bc..d00d1a9 100644 --- a/crsn_arith/src/parse.rs +++ b/crsn_arith/src/parse.rs @@ -30,19 +30,19 @@ pub(crate) fn parse<'a>(pos: &SourcePosition, keyword: &str, mut args: TokenPars } "inc" => { - let dst = args.next_wr()?; + let dst = args.next_rdwr()?; ArithOp::Add { - dst, - a: dst.as_rd(), + dst: dst.wr(), + a: dst.rd(), b: Rd::immediate(1), } } "dec" => { - let dst = args.next_wr()?; + let dst = args.next_rdwr()?; ArithOp::Sub { - dst, - a: dst.as_rd(), + dst: dst.wr(), + a: dst.rd(), b: Rd::immediate(1), } } @@ -57,10 +57,10 @@ pub(crate) fn parse<'a>(pos: &SourcePosition, keyword: &str, mut args: TokenPars } } 2 => { - let dst = args.next_wr()?; + let dst = args.next_rdwr()?; ArithOp::Add { - dst, - a: dst.as_rd(), + dst: dst.wr(), + a: dst.rd(), b: args.next_rd()?, } } @@ -80,10 +80,10 @@ pub(crate) fn parse<'a>(pos: &SourcePosition, keyword: &str, mut args: TokenPars } } 2 => { - let dst = args.next_wr()?; + let dst = args.next_rdwr()?; ArithOp::Sub { - dst, - a: dst.as_rd(), + dst: dst.wr(), + a: dst.rd(), b: args.next_rd()?, } } @@ -103,10 +103,10 @@ pub(crate) fn parse<'a>(pos: &SourcePosition, keyword: &str, mut args: TokenPars } } 2 => { - let dst = args.next_wr()?; + let dst = args.next_rdwr()?; ArithOp::Mul { - dst, - a: dst.as_rd(), + dst: dst.wr(), + a: dst.rd(), b: args.next_rd()?, } } @@ -119,13 +119,13 @@ pub(crate) fn parse<'a>(pos: &SourcePosition, keyword: &str, mut args: TokenPars "divr" => { match args.len() { 3 => { - let dst = args.next_wr()?; + let dst = args.next_rdwr()?; let rem = args.next_wr()?; let div = args.next_rd()?; ArithOp::Div { - dst, + dst: dst.wr(), rem, - a: dst.as_rd(), + a: dst.rd(), div, } } @@ -154,12 +154,12 @@ pub(crate) fn parse<'a>(pos: &SourcePosition, keyword: &str, mut args: TokenPars } } 2 => { - let dst = args.next_wr()?; + let dst = args.next_rdwr()?; let div = args.next_rd()?; ArithOp::Div { - dst, + dst: dst.wr(), rem: Wr::discard(), - a: dst.as_rd(), + a: dst.rd(), div, } } @@ -179,11 +179,11 @@ pub(crate) fn parse<'a>(pos: &SourcePosition, keyword: &str, mut args: TokenPars } } 2 => { - let dst = args.next_wr()?; + let dst = args.next_rdwr()?; let div = args.next_rd()?; ArithOp::Mod { - dst, - a: dst.as_rd(), + dst: dst.wr(), + a: dst.rd(), div, } } @@ -203,10 +203,10 @@ pub(crate) fn parse<'a>(pos: &SourcePosition, keyword: &str, mut args: TokenPars } } 2 => { - let dst = args.next_wr()?; + let dst = args.next_rdwr()?; ArithOp::And { - dst, - a: dst.as_rd(), + dst: dst.wr(), + a: dst.rd(), b: args.next_rd()?, } } @@ -226,10 +226,10 @@ pub(crate) fn parse<'a>(pos: &SourcePosition, keyword: &str, mut args: TokenPars } } 2 => { - let dst = args.next_wr()?; + let dst = args.next_rdwr()?; ArithOp::Or { - dst, - a: dst.as_rd(), + dst: dst.wr(), + a: dst.rd(), b: args.next_rd()?, } } @@ -249,10 +249,10 @@ pub(crate) fn parse<'a>(pos: &SourcePosition, keyword: &str, mut args: TokenPars } } 2 => { - let dst = args.next_wr()?; + let dst = args.next_rdwr()?; ArithOp::Xor { - dst, - a: dst.as_rd(), + dst: dst.wr(), + a: dst.rd(), b: args.next_rd()?, } } @@ -271,10 +271,10 @@ pub(crate) fn parse<'a>(pos: &SourcePosition, keyword: &str, mut args: TokenPars } } 1 => { - let dst = args.next_wr()?; + let dst = args.next_rdwr()?; ArithOp::Cpl { - dst, - a: dst.as_rd(), + dst: dst.wr(), + a: dst.rd(), } } _ => { @@ -293,18 +293,18 @@ pub(crate) fn parse<'a>(pos: &SourcePosition, keyword: &str, mut args: TokenPars } } 2 => { - let dst = args.next_wr()?; + let dst = args.next_rdwr()?; ArithOp::Rol { - dst, - a: dst.as_rd(), + dst: dst.wr(), + a: dst.rd(), n: args.next_rd()?, } } 1 => { - let dst = args.next_wr()?; + let dst = args.next_rdwr()?; ArithOp::Rol { - dst, - a: dst.as_rd(), + dst: dst.wr(), + a: dst.rd(), n: Rd::immediate(1), } } @@ -324,18 +324,18 @@ pub(crate) fn parse<'a>(pos: &SourcePosition, keyword: &str, mut args: TokenPars } } 2 => { - let dst = args.next_wr()?; + let dst = args.next_rdwr()?; ArithOp::Ror { - dst, - a: dst.as_rd(), + dst: dst.wr(), + a: dst.rd(), n: args.next_rd()?, } } 1 => { - let dst = args.next_wr()?; + let dst = args.next_rdwr()?; ArithOp::Ror { - dst, - a: dst.as_rd(), + dst: dst.wr(), + a: dst.rd(), n: Rd::immediate(1), } } @@ -355,18 +355,18 @@ pub(crate) fn parse<'a>(pos: &SourcePosition, keyword: &str, mut args: TokenPars } } 2 => { - let dst = args.next_wr()?; + let dst = args.next_rdwr()?; ArithOp::Lsl { - dst, - a: dst.as_rd(), + dst: dst.wr(), + a: dst.rd(), n: args.next_rd()?, } } 1 => { - let dst = args.next_wr()?; + let dst = args.next_rdwr()?; ArithOp::Lsl { - dst, - a: dst.as_rd(), + dst: dst.wr(), + a: dst.rd(), n: Rd::immediate(1), } } @@ -386,18 +386,18 @@ pub(crate) fn parse<'a>(pos: &SourcePosition, keyword: &str, mut args: TokenPars } } 2 => { - let dst = args.next_wr()?; + let dst = args.next_rdwr()?; ArithOp::Lsr { - dst, - a: dst.as_rd(), + dst: dst.wr(), + a: dst.rd(), n: args.next_rd()?, } } 1 => { - let dst = args.next_wr()?; + let dst = args.next_rdwr()?; ArithOp::Lsr { - dst, - a: dst.as_rd(), + dst: dst.wr(), + a: dst.rd(), n: Rd::immediate(1), } } @@ -417,18 +417,18 @@ pub(crate) fn parse<'a>(pos: &SourcePosition, keyword: &str, mut args: TokenPars } } 2 => { - let dst = args.next_wr()?; + let dst = args.next_rdwr()?; ArithOp::Asr { - dst, - a: dst.as_rd(), + dst: dst.wr(), + a: dst.rd(), n: args.next_rd()?, } } 1 => { - let dst = args.next_wr()?; + let dst = args.next_rdwr()?; ArithOp::Asr { - dst, - a: dst.as_rd(), + dst: dst.wr(), + a: dst.rd(), n: Rd::immediate(1), } }