diff options
| author | Miquel Sabaté Solà <mikisabate@gmail.com> | 2024-12-18 11:55:40 +0100 |
|---|---|---|
| committer | Miquel Sabaté Solà <mikisabate@gmail.com> | 2024-12-18 11:55:40 +0100 |
| commit | dbee63338fda39170118e3ab1f66637a690f5542 (patch) | |
| tree | 2fd5181d02e683decd48c6ae56a197fb9c4ec5a1 /lib/xixanta | |
| parent | 864702da68ec69ee92e47e77a65c5a86ba200201 (diff) | |
| download | tools.nes-dbee63338fda39170118e3ab1f66637a690f5542.tar.gz tools.nes-dbee63338fda39170118e3ab1f66637a690f5542.zip | |
Prevent a crash on bad binary literals
When evaluating binary literals we allowed the shift value to grow as
needed and we checked whether it was a good value after evaluating the
literal. This is bad for performance reasons: if we are expecting an
exact size for a literal (8 digits here), do not even attempt to do
anything at all if the size doesn't match.
Moreover, in some extreme cases this could result into an overflow of
the 'shift' variable, which was promptly catched by Rust's bound
checker.
Signed-off-by: Miquel Sabaté Solà <mikisabate@gmail.com>
Diffstat (limited to 'lib/xixanta')
| -rw-r--r-- | lib/xixanta/src/assembler.rs | 60 |
1 files changed, 33 insertions, 27 deletions
diff --git a/lib/xixanta/src/assembler.rs b/lib/xixanta/src/assembler.rs index 24653e2..f2cd507 100644 --- a/lib/xixanta/src/assembler.rs +++ b/lib/xixanta/src/assembler.rs @@ -601,9 +601,29 @@ impl Assembler { fn evaluate_binary(&mut self, node: &PNode) -> Result<Bundle, EvalError> { let string = node.value.value.as_str(); let mut value = 0; - let mut shift = 0; - for c in string.chars().rev() { + // We are strict with the definition of binary values to avoid gotchas. + // Force the literal to be exactly 8 digits wide. If that's not the + // case, do not even try evaluating it. + match string.len().cmp(&8) { + Ordering::Less => { + return Err(EvalError { + message: "missing binary digits to get a full byte".to_string(), + line: node.value.line, + global: false, + }) + } + Ordering::Greater => { + return Err(EvalError { + message: "too many binary digits for a single byte".to_string(), + line: node.value.line, + global: false, + }) + } + _ => {} + } + + for (shift, c) in string.chars().rev().enumerate() { if c == '1' { let val = 1 << shift; value += val; @@ -624,30 +644,16 @@ impl Assembler { global: false, }); } - - shift += 1; } - match shift.cmp(&8) { - Ordering::Less => Err(EvalError { - message: "missing binary digits to get a full byte".to_string(), - line: node.value.line, - global: false, - }), - Ordering::Greater => Err(EvalError { - message: "too many binary digits for a single byte".to_string(), - line: node.value.line, - global: false, - }), - Ordering::Equal => Ok(Bundle { - bytes: [value as u8, 0, 0], - size: 1, - address: 0, - cycles: 0, - affected_on_page: false, - resolved: true, - }), - } + Ok(Bundle { + bytes: [value as u8, 0, 0], + size: 1, + address: 0, + cycles: 0, + affected_on_page: false, + resolved: true, + }) } fn evaluate_decimal(&mut self, node: &PNode) -> Result<Bundle, EvalError> { @@ -1373,9 +1379,9 @@ mod tests { ); assert_error( r#" -Variable = 42 -adc %Variable -"#, + Variable = 42 + adc %Variable + "#, "Evaluation", 3, false, |
