From 19c2909193d832a31475d40cf6ce17f7285d0992 Mon Sep 17 00:00:00 2001 From: Jose Rodriguez Date: Tue, 26 May 2026 00:15:37 +0200 Subject: [PATCH] fix: crash un undeclared substring assign --- pyproject.toml | 2 +- src/api/errmsg.py | 7 + src/api/symboltable/symboltable.py | 11 +- src/zxbc/zxbparser.py | 3 +- .../arch/zx48k/letsubstr_undecl.asm | 446 ++++++++++++++++++ .../arch/zx48k/letsubstr_undecl.bas | 2 + tests/functional/cmdline/test_warning.txt | 3 + 7 files changed, 466 insertions(+), 8 deletions(-) create mode 100644 tests/functional/arch/zx48k/letsubstr_undecl.asm create mode 100644 tests/functional/arch/zx48k/letsubstr_undecl.bas diff --git a/pyproject.toml b/pyproject.toml index 73189d425..f52bc577d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -52,7 +52,7 @@ pytest-xdist = "*" setuptools = ">=70.1.1,<79.0.0" [build-system] -requires = ["poetry-core>=2.0.0"] +requires = ["poetry-core>=2.4.1"] build-backend = "poetry.core.masonry.api" [tool.poe.tasks] diff --git a/src/api/errmsg.py b/src/api/errmsg.py index ec143223d..4a87a0cd2 100644 --- a/src/api/errmsg.py +++ b/src/api/errmsg.py @@ -19,6 +19,7 @@ "register_warning", "warning", "warning_not_used", + "warning_uninitalized_string_var", ) @@ -127,6 +128,12 @@ def warning_implicit_type(lineno: int, id_: str, type_: str = None): warning(lineno, "Using default implicit type '%s' for '%s'" % (type_, id_)) +@register_warning("101") +def warning_uninitalized_string_var(lineno: int, id_: str): + """Warning: Accessing uninitialized string variable""" + warning(lineno, f"Accessing uninitialized string variable '{id_}'") + + @register_warning("110") def warning_condition_is_always(lineno: int, cond: bool = False): """Warning: Condition is always false/true""" diff --git a/src/api/symboltable/symboltable.py b/src/api/symboltable/symboltable.py index 70e66ba54..da01eec75 100644 --- a/src/api/symboltable/symboltable.py +++ b/src/api/symboltable/symboltable.py @@ -132,7 +132,7 @@ def declare(self, id_: str, lineno: int, entry: symbols.ID | symbols.TYPE) -> No return entry - def declare_safe(self, id_: str, lineno: int, entry: symbols.ID): + def declare_safe(self, id_: str, lineno: int, entry: symbols.ID) -> symbols.ID | symbols.TYPE: """Like declare, but never returns None""" result = self.declare(id_, lineno, entry) assert result is not None @@ -204,7 +204,7 @@ def check_is_undeclared( ) return False - def check_class(self, id_: str, class_: CLASS, lineno: int, scope: Scope = None, show_error=True) -> bool: + def check_class(self, id_: str, class_: CLASS, lineno: int, scope: Scope | None = None, show_error=True) -> bool: """Check the id is either undefined or defined with the given class. @@ -342,7 +342,7 @@ def access_id( default_class: CLASS = CLASS.unknown, *, ignore_explicit_flag=False, - ): + ) -> symbols.ID | None: """Access a symbol by its identifier and checks if it exists. If not, it's supposed to be an implicitly declared variable. @@ -442,10 +442,9 @@ def access_func(self, id_: str, lineno: int, scope=None, default_type=None): return result - def access_call(self, id_: str, lineno: int, scope=None, type_=None): + def access_call(self, id_: str, lineno: int, scope=None, type_=None) -> symbols.ID | None: """Creates a func/array/string call. Checks if id is callable or not. - An identifier is "callable" if it can be followed by a list of para- - meters. + An identifier is "callable" if it can be followed by a list of parameters. This does not mean the id_ is a function, but that it allows the same syntax a function does: diff --git a/src/zxbc/zxbparser.py b/src/zxbc/zxbparser.py index 0beeb0ebb..c213eb5a1 100755 --- a/src/zxbc/zxbparser.py +++ b/src/zxbc/zxbparser.py @@ -1265,7 +1265,8 @@ def p_substr_assignment_no_let(p): return if entry.class_ == CLASS.unknown: - entry.class_ = CLASS.var + errmsg.warning_uninitalized_string_var(p.lineno(1), entry.name) + entry.to_var() if p[6].type_ != Type.string: errmsg.syntax_error_expected_string(p.lineno(5), p[6].type_) diff --git a/tests/functional/arch/zx48k/letsubstr_undecl.asm b/tests/functional/arch/zx48k/letsubstr_undecl.asm new file mode 100644 index 000000000..594f1e43c --- /dev/null +++ b/tests/functional/arch/zx48k/letsubstr_undecl.asm @@ -0,0 +1,446 @@ + org 32768 +.core.__START_PROGRAM: + di + push ix + push iy + exx + push hl + exx + ld (.core.__CALL_BACK__), sp + ei + call .core.__MEM_INIT + jp .core.__MAIN_PROGRAM__ +.core.__CALL_BACK__: + DEFW 0 +.core.ZXBASIC_USER_DATA: + ; Defines HEAP SIZE +.core.ZXBASIC_HEAP_SIZE EQU 4768 +.core.ZXBASIC_MEM_HEAP: + DEFS 4768 + ; Defines USER DATA Length in bytes +.core.ZXBASIC_USER_DATA_LEN EQU .core.ZXBASIC_USER_DATA_END - .core.ZXBASIC_USER_DATA + .core.__LABEL__.ZXBASIC_USER_DATA_LEN EQU .core.ZXBASIC_USER_DATA_LEN + .core.__LABEL__.ZXBASIC_USER_DATA EQU .core.ZXBASIC_USER_DATA +_S: + DEFB 00, 00 +.core.ZXBASIC_USER_DATA_END: +.core.__MAIN_PROGRAM__: + ld hl, .LABEL.__LABEL0 + push hl + xor a + push af + ld hl, 0 + push hl + ld hl, 0 + push hl + ld hl, (_S) + call .core.__LETSUBSTR + ld a, (_S) & 0xFF + ld (0), a + ld hl, 0 + ld b, h + ld c, l +.core.__END_PROGRAM: + di + ld hl, (.core.__CALL_BACK__) + ld sp, hl + exx + pop hl + exx + pop iy + pop ix + ei + ret +.LABEL.__LABEL0: + DEFW 0001h + DEFB 61h + ;; --- end of user code --- +#line 1 "/zxbasic/src/lib/arch/zx48k/runtime/letsubstr.asm" + ; Substring assigment eg. LET a$(p0 TO p1) = "xxxx" + ; HL = Start of string + ; TOP of the stack -> p1 (16 bit, unsigned) + ; TOP -1 of the stack -> p0 register + ; TOP -2 Flag (popped out in A register) + ; A Register => 0 if HL is not freed from memory + ; => Not 0 if HL must be freed from memory on exit + ; TOP -3 B$ address +#line 1 "/zxbasic/src/lib/arch/zx48k/runtime/mem/free.asm" +; vim: ts=4:et:sw=4: + ; Copyleft (K) by Jose M. Rodriguez de la Rosa + ; (a.k.a. Boriel) +; http://www.boriel.com + ; + ; This ASM library is licensed under the BSD license + ; you can use it for any purpose (even for commercial + ; closed source programs). + ; + ; Please read the BSD license on the internet + ; ----- IMPLEMENTATION NOTES ------ + ; The heap is implemented as a linked list of free blocks. +; Each free block contains this info: + ; + ; +----------------+ <-- HEAP START + ; | Size (2 bytes) | + ; | 0 | <-- Size = 0 => DUMMY HEADER BLOCK + ; +----------------+ + ; | Next (2 bytes) |---+ + ; +----------------+ <-+ + ; | Size (2 bytes) | + ; +----------------+ + ; | Next (2 bytes) |---+ + ; +----------------+ | + ; | | | <-- If Size > 4, then this contains (size - 4) bytes + ; | (0 if Size = 4)| | + ; +----------------+ <-+ + ; | Size (2 bytes) | + ; +----------------+ + ; | Next (2 bytes) |---+ + ; +----------------+ | + ; | | | + ; | (0 if Size = 4)| | + ; +----------------+ | + ; | <-- This zone is in use (Already allocated) + ; +----------------+ <-+ + ; | Size (2 bytes) | + ; +----------------+ + ; | Next (2 bytes) |---+ + ; +----------------+ | + ; | | | + ; | (0 if Size = 4)| | + ; +----------------+ <-+ + ; | Next (2 bytes) |--> NULL => END OF LIST + ; | 0 = NULL | + ; +----------------+ + ; | | + ; | (0 if Size = 4)| + ; +----------------+ + ; When a block is FREED, the previous and next pointers are examined to see + ; if we can defragment the heap. If the block to be breed is just next to the + ; previous, or to the next (or both) they will be converted into a single + ; block (so defragmented). + ; MEMORY MANAGER + ; + ; This library must be initialized calling __MEM_INIT with + ; HL = BLOCK Start & DE = Length. + ; An init directive is useful for initialization routines. + ; They will be added automatically if needed. +#line 1 "/zxbasic/src/lib/arch/zx48k/runtime/mem/heapinit.asm" +; vim: ts=4:et:sw=4: + ; Copyleft (K) by Jose M. Rodriguez de la Rosa + ; (a.k.a. Boriel) +; http://www.boriel.com + ; + ; This ASM library is licensed under the BSD license + ; you can use it for any purpose (even for commercial + ; closed source programs). + ; + ; Please read the BSD license on the internet + ; ----- IMPLEMENTATION NOTES ------ + ; The heap is implemented as a linked list of free blocks. +; Each free block contains this info: + ; + ; +----------------+ <-- HEAP START + ; | Size (2 bytes) | + ; | 0 | <-- Size = 0 => DUMMY HEADER BLOCK + ; +----------------+ + ; | Next (2 bytes) |---+ + ; +----------------+ <-+ + ; | Size (2 bytes) | + ; +----------------+ + ; | Next (2 bytes) |---+ + ; +----------------+ | + ; | | | <-- If Size > 4, then this contains (size - 4) bytes + ; | (0 if Size = 4)| | + ; +----------------+ <-+ + ; | Size (2 bytes) | + ; +----------------+ + ; | Next (2 bytes) |---+ + ; +----------------+ | + ; | | | + ; | (0 if Size = 4)| | + ; +----------------+ | + ; | <-- This zone is in use (Already allocated) + ; +----------------+ <-+ + ; | Size (2 bytes) | + ; +----------------+ + ; | Next (2 bytes) |---+ + ; +----------------+ | + ; | | | + ; | (0 if Size = 4)| | + ; +----------------+ <-+ + ; | Next (2 bytes) |--> NULL => END OF LIST + ; | 0 = NULL | + ; +----------------+ + ; | | + ; | (0 if Size = 4)| + ; +----------------+ + ; When a block is FREED, the previous and next pointers are examined to see + ; if we can defragment the heap. If the block to be breed is just next to the + ; previous, or to the next (or both) they will be converted into a single + ; block (so defragmented). + ; MEMORY MANAGER + ; + ; This library must be initialized calling __MEM_INIT with + ; HL = BLOCK Start & DE = Length. + ; An init directive is useful for initialization routines. + ; They will be added automatically if needed. + ; --------------------------------------------------------------------- + ; __MEM_INIT must be called to initalize this library with the + ; standard parameters + ; --------------------------------------------------------------------- + push namespace core +__MEM_INIT: ; Initializes the library using (RAMTOP) as start, and + ld hl, ZXBASIC_MEM_HEAP ; Change this with other address of heap start + ld de, ZXBASIC_HEAP_SIZE ; Change this with your size + ; --------------------------------------------------------------------- + ; __MEM_INIT2 initalizes this library +; Parameters: +; HL : Memory address of 1st byte of the memory heap +; DE : Length in bytes of the Memory Heap + ; --------------------------------------------------------------------- +__MEM_INIT2: + ; HL as TOP + PROC + dec de + dec de + dec de + dec de ; DE = length - 4; HL = start + ; This is done, because we require 4 bytes for the empty dummy-header block + xor a + ld (hl), a + inc hl + ld (hl), a ; First "free" block is a header: size=0, Pointer=&(Block) + 4 + inc hl + ld b, h + ld c, l + inc bc + inc bc ; BC = starts of next block + ld (hl), c + inc hl + ld (hl), b + inc hl ; Pointer to next block + ld (hl), e + inc hl + ld (hl), d + inc hl ; Block size (should be length - 4 at start); This block contains all the available memory + ld (hl), a ; NULL (0000h) ; No more blocks (a list with a single block) + inc hl + ld (hl), a + ld a, 201 + ld (__MEM_INIT), a; "Pokes" with a RET so ensure this routine is not called again + ret + ENDP + pop namespace +#line 69 "/zxbasic/src/lib/arch/zx48k/runtime/mem/free.asm" + ; --------------------------------------------------------------------- + ; MEM_FREE + ; Frees a block of memory + ; +; Parameters: + ; HL = Pointer to the block to be freed. If HL is NULL (0) nothing + ; is done + ; --------------------------------------------------------------------- + push namespace core +MEM_FREE: +__MEM_FREE: ; Frees the block pointed by HL + ; HL DE BC & AF modified + PROC + LOCAL __MEM_LOOP2 + LOCAL __MEM_LINK_PREV + LOCAL __MEM_JOIN_TEST + LOCAL __MEM_BLOCK_JOIN + ld a, h + or l + ret z ; Return if NULL pointer + dec hl + dec hl + ld b, h + ld c, l ; BC = Block pointer + ld hl, ZXBASIC_MEM_HEAP ; This label point to the heap start +__MEM_LOOP2: + inc hl + inc hl ; Next block ptr + ld e, (hl) + inc hl + ld d, (hl) ; Block next ptr + ex de, hl ; DE = &(block->next); HL = block->next + ld a, h ; HL == NULL? + or l + jp z, __MEM_LINK_PREV; if so, link with previous + or a ; Clear carry flag + sbc hl, bc ; Carry if BC > HL => This block if before + add hl, bc ; Restores HL, preserving Carry flag + jp c, __MEM_LOOP2 ; This block is before. Keep searching PASS the block + ;------ At this point current HL is PAST BC, so we must link (DE) with BC, and HL in BC->next +__MEM_LINK_PREV: ; Link (DE) with BC, and BC->next with HL + ex de, hl + push hl + dec hl + ld (hl), c + inc hl + ld (hl), b ; (DE) <- BC + ld h, b ; HL <- BC (Free block ptr) + ld l, c + inc hl ; Skip block length (2 bytes) + inc hl + ld (hl), e ; Block->next = DE + inc hl + ld (hl), d + ; --- LINKED ; HL = &(BC->next) + 2 + call __MEM_JOIN_TEST + pop hl +__MEM_JOIN_TEST: ; Checks for fragmented contiguous blocks and joins them + ; hl = Ptr to current block + 2 + ld d, (hl) + dec hl + ld e, (hl) + dec hl + ld b, (hl) ; Loads block length into BC + dec hl + ld c, (hl) ; + push hl ; Saves it for later + add hl, bc ; Adds its length. If HL == DE now, it must be joined + or a + sbc hl, de ; If Z, then HL == DE => We must join + pop hl + ret nz +__MEM_BLOCK_JOIN: ; Joins current block (pointed by HL) with next one (pointed by DE). HL->length already in BC + push hl ; Saves it for later + ex de, hl + ld e, (hl) ; DE -> block->next->length + inc hl + ld d, (hl) + inc hl + ex de, hl ; DE = &(block->next) + add hl, bc ; HL = Total Length + ld b, h + ld c, l ; BC = Total Length + ex de, hl + ld e, (hl) + inc hl + ld d, (hl) ; DE = block->next + pop hl ; Recovers Pointer to block + ld (hl), c + inc hl + ld (hl), b ; Length Saved + inc hl + ld (hl), e + inc hl + ld (hl), d ; Next saved + ret + ENDP + pop namespace +#line 11 "/zxbasic/src/lib/arch/zx48k/runtime/letsubstr.asm" + push namespace core +__LETSUBSTR: + PROC + LOCAL __CONT0 + LOCAL __CONT1 + LOCAL __CONT2 + LOCAL __FREE_STR + exx + pop hl ; Return address + pop de ; p1 + pop bc ; p0 + exx + pop af ; Flag + ex af, af' ; Save it for later + pop de ; B$ + exx + push hl ; push ret addr back + exx + push de ; B$ addr to be freed upon return (if A != 0) + ld a, h + or l + jp z, __FREE_STR ; Return if null + ld c, (hl) + inc hl + ld b, (hl) ; BC = Str length + inc hl ; HL = String start + push bc + exx + ex de, hl + or a + sbc hl, bc ; HL = Length of string requested by user + inc hl ; len (a$(p0 TO p1)) = p1 - p0 + 1 + ex de, hl ; Saves it in DE + pop hl ; HL = String length + exx + jp c, __FREE_STR ; Return if p0 > p1 + exx + or a + sbc hl, bc ; P0 >= String length? + exx + jp z, __FREE_STR ; Return if equal + jp c, __FREE_STR ; Return if greater + exx + add hl, bc ; Add it back + sbc hl, de ; Length of substring > string => Truncate it + add hl, de ; add it back + jr nc, __CONT0 ; Length of substring within a$ + ld d, h + ld e, l ; Truncate length of substring to fit within the strlen +__CONT0: ; At this point DE = Length of substring to copy + ; BC = start of char to copy + push de + push bc + exx + pop bc + add hl, bc ; Start address (within a$) so copy from b$ (in DE) + push hl + exx + pop hl ; Start address (within a$) so copy from b$ (in DE) + ld b, d ; Length of string + ld c, e + ld (hl), ' ' + ld d, h + ld e, l + inc de + dec bc + ld a, b + or c + jr z, __CONT2 + ; At this point HL = DE = Start of Write zone in a$ + ; BC = Number of chars to write + ldir +__CONT2: + pop bc ; Recovers Length of string to copy + exx + ex de, hl ; HL = Source, DE = Target + ld a, h + or l + jp z, __FREE_STR ; Return if B$ is NULL + ld c, (hl) + inc hl + ld b, (hl) + inc hl + ld a, b + or c + jp z, __FREE_STR ; Return if len(b$) = 0 + ; Now if len(b$) < len(char to copy), copy only len(b$) chars + push de + push hl + push bc + exx + pop hl ; LEN (b$) + or a + sbc hl, bc + add hl, bc + jr nc, __CONT1 + ; If len(b$) < len(to copy) + ld b, h ; BC = len(to copy) + ld c, l +__CONT1: + pop hl + pop de + ldir ; Copy b$ into a$(x to y) +__FREE_STR: + pop hl + ex af, af' + or a ; If not 0, free + jp nz, __MEM_FREE + ret + ENDP + pop namespace +#line 32 "arch/zx48k/letsubstr_undecl.bas" + END diff --git a/tests/functional/arch/zx48k/letsubstr_undecl.bas b/tests/functional/arch/zx48k/letsubstr_undecl.bas new file mode 100644 index 000000000..1f5b1adf0 --- /dev/null +++ b/tests/functional/arch/zx48k/letsubstr_undecl.bas @@ -0,0 +1,2 @@ +S$(0) = "a" +POKE 0, @S diff --git a/tests/functional/cmdline/test_warning.txt b/tests/functional/cmdline/test_warning.txt index 18d5f2697..d8aa00f86 100644 --- a/tests/functional/cmdline/test_warning.txt +++ b/tests/functional/cmdline/test_warning.txt @@ -12,3 +12,6 @@ doloop2.bas:4: warning: [W150] Variable 'a' is never used >>> process_file('arch/zx48k/warn_brk.bas', ['-S', '-q', '-O --enable-break']) >>> process_file('arch/zx48k/warn_lbl.bas', ['-S', '-q']) + +>>> process_file('arch/zx48k/letsubstr_undecl.bas', ['-S', '-q']) +letsubstr_undecl.bas:1: warning: [W101] Accessing uninitialized string variable 'S'