From 8b38c88ae55a6edf1028a5aa57b93afdbc4001d6 Mon Sep 17 00:00:00 2001 From: Miquel Sabaté Solà Date: Tue, 3 Feb 2026 10:49:33 +0100 Subject: Avoid invalid identifiers in proc/macro/scope MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This was apparently neglected and you were able to pick invalid identifiers to identify procs, macros and scopes. Ensure this does not happen again and provide tests for it. Signed-off-by: Miquel Sabaté Solà --- lib/xixanta/src/assembler.rs | 65 +++++++++++++++++++++++++++++++++---- scripts/test-e2e.sh | 7 ++++ tests/avoid_bad_macro.s | 16 +++++++++ tests/expected/avoid_bad_macro.nes | Bin 0 -> 18 bytes tests/expected/avoid_bad_macro.txt | 0 5 files changed, 81 insertions(+), 7 deletions(-) create mode 100644 tests/avoid_bad_macro.s create mode 100644 tests/expected/avoid_bad_macro.nes create mode 100644 tests/expected/avoid_bad_macro.txt diff --git a/lib/xixanta/src/assembler.rs b/lib/xixanta/src/assembler.rs index cb992d4..136d416 100644 --- a/lib/xixanta/src/assembler.rs +++ b/lib/xixanta/src/assembler.rs @@ -588,12 +588,25 @@ impl<'a> Assembler<'a> { // illegal definitions. self.macros_seen += 1; - // Insert a reference to this node so it can be - // unrolled whenever we have to perform a macro - // call. - self.macros - .entry(node.left.as_ref().unwrap().value.value.clone()) - .or_insert(node); + let name = &node.left.as_ref().unwrap().value; + if let Err(e) = name.is_valid_identifier(false) { + errors.push(Error { + message: format!( + "'{}' is not a valid macro name: {e}", + name.value + ), + line: node.value.line, + global: false, + expanded_from: self.macro_context.clone(), + source: self.source_for(node), + }); + continue; + } else { + // Insert a reference to this node so it can be + // unrolled whenever we have to perform a macro + // call. + self.macros.entry(name.value.clone()).or_insert(node); + } } ControlType::EndMacro => { if self.macros_seen > 0 { @@ -616,7 +629,19 @@ impl<'a> Assembler<'a> { self.procs_seen += 1; let proc_name = &node.left.as_ref().unwrap(); - if let Err(err) = self.define_variable(proc_name) { + if let Err(err) = proc_name.value.is_valid_identifier(false) { + errors.push(Error { + message: format!( + "'{}' is not a valid proc name: {err}", + proc_name.value.value, + ), + line: node.value.line, + global: false, + expanded_from: self.macro_context.clone(), + source: self.source_for(node), + }); + continue; + } else if let Err(err) = self.define_variable(proc_name) { errors.push(err); } } @@ -638,6 +663,20 @@ impl<'a> Assembler<'a> { }); continue; } + let scope_name = &node.left.as_ref().unwrap(); + if let Err(err) = scope_name.value.is_valid_identifier(false) { + errors.push(Error { + message: format!( + "'{}' is not a valid scope name: {err}", + scope_name.value.value, + ), + line: node.value.line, + global: false, + expanded_from: self.macro_context.clone(), + source: self.source_for(node), + }); + continue; + } } ControlType::StartRepeat => { self.repeats_seen += 1; @@ -4757,6 +4796,18 @@ MACRO Var1 assert_eq!(res[0].bytes[2], 0x00); } + #[test] + fn bad_macro_name() { + assert_error( + r#".macro __fallthrough__ arg + .endmacro + "#, + 1, + false, + "'__fallthrough__' is not a valid macro name: cannot use reserved name '__fallthrough__'", + ); + } + #[test] fn bad_scope_definition_inside_of_proc() { assert_error( diff --git a/scripts/test-e2e.sh b/scripts/test-e2e.sh index 18dbeff..06545b8 100755 --- a/scripts/test-e2e.sh +++ b/scripts/test-e2e.sh @@ -88,6 +88,13 @@ exit_code=$((exit_code + $?)) diff tests/out/fallthrough.nes tests/expected/fallthrough.nes exit_code=$((exit_code + $?)) +echo "test: custom => avoid_bad_macro.nes" +./target/debug/nasm -c empty --asan tests/avoid_bad_macro.s -o tests/out/avoid_bad_macro.nes 2>tests/out/avoid_bad_macro.txt +diff tests/out/avoid_bad_macro.txt tests/expected/avoid_bad_macro.txt +exit_code=$((exit_code + $?)) +diff tests/out/avoid_bad_macro.nes tests/expected/avoid_bad_macro.nes +exit_code=$((exit_code + $?)) + ## # code.nes diff --git a/tests/avoid_bad_macro.s b/tests/avoid_bad_macro.s new file mode 100644 index 0000000..f701c2b --- /dev/null +++ b/tests/avoid_bad_macro.s @@ -0,0 +1,16 @@ +.segment "HEADER" + .byte 'N', 'E', 'S', $1A + .byte $02 + .byte $01 + .byte $00 + .byte $00 + +.segment "CODE" + +.ifndef __NASM__ + .macro __fallthrough__ arg + lda #0 + .endmacro +.endif + +lda #1 diff --git a/tests/expected/avoid_bad_macro.nes b/tests/expected/avoid_bad_macro.nes new file mode 100644 index 0000000..eb99eb1 Binary files /dev/null and b/tests/expected/avoid_bad_macro.nes differ diff --git a/tests/expected/avoid_bad_macro.txt b/tests/expected/avoid_bad_macro.txt new file mode 100644 index 0000000..e69de29 -- cgit v1.2.3