Fix BitField slice and off-by-one bugs in zobjectparser.py

Property parsing had 4 classes of bug, all producing wrong addresses
for object property data:

1. Off-by-one in shortname skip: addr += 2*len missed the length byte
   itself, causing property table scan to start 1 byte too early.
   Affected get_prop_addr_len and get_next_property.

2. V3 property number slices bf[4:0] extracted 4 bits not 5.
   Property numbers 16-31 were read as 0-15.

3. V3 property size slices bf[7:5] extracted 2 bits not 3.
   Properties larger than 4 bytes got wrong sizes.

4. V5 property number/size slices bf[5:0] extracted 5 bits not 6.

Also fixed get_property_length reading size from the wrong byte in
V5 two-byte headers (was reading first byte which has property
number, instead of second byte which has size).

Root cause for all slice bugs: BitField uses Python [start:stop)
semantics but code was written as [high:low] inclusive notation.
Same class of bug as the write_global fix in commit e5329d6.
This commit is contained in:
Jared Miller 2026-02-09 22:51:40 -05:00
parent 5ffbe660fb
commit 127ca5f56e
Signed by: shmup
GPG key ID: 22B5C6D66A38B06C
3 changed files with 24 additions and 22 deletions

View file

@ -278,7 +278,7 @@ class ZCpu:
val = self._read_variable(variable) val = self._read_variable(variable)
val = (val - 1) % 65536 val = (val - 1) % 65536
self._write_result(val, store_addr=variable) self._write_result(val, store_addr=variable)
self._branch(val < test_value) self._branch(self._make_signed(val) < self._make_signed(test_value))
def op_inc_chk(self, variable, test_value): def op_inc_chk(self, variable, test_value):
"""Increment the variable, and branch if the value becomes """Increment the variable, and branch if the value becomes
@ -286,7 +286,7 @@ class ZCpu:
val = self._read_variable(variable) val = self._read_variable(variable)
val = (val + 1) % 65536 val = (val + 1) % 65536
self._write_result(val, store_addr=variable) self._write_result(val, store_addr=variable)
self._branch(val > test_value) self._branch(self._make_signed(val) > self._make_signed(test_value))
def op_jin(self, obj1, obj2): def op_jin(self, obj1, obj2):
"""Branch if obj1's parent equals obj2.""" """Branch if obj1's parent equals obj2."""
@ -499,7 +499,8 @@ class ZCpu:
def op_print_paddr(self, string_paddr): def op_print_paddr(self, string_paddr):
"""Print the string at the given packed address.""" """Print the string at the given packed address."""
zstr_address = self._memory.packed_address(string_paddr) zstr_address = self._memory.packed_address(string_paddr)
self._ui.screen.write(self._string.get(zstr_address)) text = self._string.get(zstr_address)
self._ui.screen.write(text)
def op_load(self, variable): def op_load(self, variable):
"""Load the value of the given variable and store it.""" """Load the value of the given variable and store it."""
@ -530,7 +531,8 @@ class ZCpu:
def op_print(self): def op_print(self):
"""Print the embedded ZString.""" """Print the embedded ZString."""
zstr_address = self._opdecoder.get_zstring() zstr_address = self._opdecoder.get_zstring()
self._ui.screen.write(self._string.get(zstr_address)) text = self._string.get(zstr_address)
self._ui.screen.write(text)
def op_print_ret(self): def op_print_ret(self):
"""TODO: Write docstring here.""" """TODO: Write docstring here."""
@ -662,6 +664,7 @@ class ZCpu:
max_words = self._memory[parse_buffer_addr] max_words = self._memory[parse_buffer_addr]
tokens = self._lexer.parse_input(text) tokens = self._lexer.parse_input(text)
num_words = min(len(tokens), max_words) num_words = min(len(tokens), max_words)
self._memory[parse_buffer_addr + 1] = num_words self._memory[parse_buffer_addr + 1] = num_words
offset = 0 offset = 0
for i in range(num_words): for i in range(num_words):

View file

@ -389,15 +389,15 @@ class ZObjectParser:
# start at the beginning of the object's proptable # start at the beginning of the object's proptable
addr = self._get_proptable_addr(objectnum) addr = self._get_proptable_addr(objectnum)
# skip past the shortname of the object # skip past the shortname of the object
addr += 2 * self._memory[addr] addr += 1 + 2 * self._memory[addr]
pnum = 0 pnum = 0
if 1 <= self._memory.version <= 3: if 1 <= self._memory.version <= 3:
while self._memory[addr] != 0: while self._memory[addr] != 0:
bf = BitField(self._memory[addr]) bf = BitField(self._memory[addr])
addr += 1 addr += 1
pnum = bf[4:0] pnum = bf[0:5]
size = bf[7:5] + 1 size = bf[5:8] + 1
if pnum == propnum: if pnum == propnum:
return (addr, size) return (addr, size)
addr += size addr += size
@ -406,11 +406,11 @@ class ZObjectParser:
while self._memory[addr] != 0: while self._memory[addr] != 0:
bf = BitField(self._memory[addr]) bf = BitField(self._memory[addr])
addr += 1 addr += 1
pnum = bf[5:0] pnum = bf[0:6]
if bf[7]: if bf[7]:
bf2 = BitField(self._memory[addr]) bf2 = BitField(self._memory[addr])
addr += 1 addr += 1
size = bf2[5:0] size = bf2[0:6]
else: else:
size = 2 if bf[6] else 1 size = 2 if bf[6] else 1
if pnum == propnum: if pnum == propnum:
@ -443,8 +443,8 @@ class ZObjectParser:
while self._memory[addr] != 0: while self._memory[addr] != 0:
bf = BitField(self._memory[addr]) bf = BitField(self._memory[addr])
addr += 1 addr += 1
pnum = bf[4:0] pnum = bf[0:5]
size = bf[7:5] + 1 size = bf[5:8] + 1
proplist[pnum] = (addr, size) proplist[pnum] = (addr, size)
addr += size addr += size
@ -508,17 +508,17 @@ class ZObjectParser:
# Return first property number # Return first property number
addr = self._get_proptable_addr(objectnum) addr = self._get_proptable_addr(objectnum)
# Skip past the shortname # Skip past the shortname
addr += 2 * self._memory[addr] addr += 1 + 2 * self._memory[addr]
# Read first property number # Read first property number
if self._memory[addr] == 0: if self._memory[addr] == 0:
return 0 return 0
if 1 <= self._memory.version <= 3: if 1 <= self._memory.version <= 3:
bf = BitField(self._memory[addr]) bf = BitField(self._memory[addr])
return bf[4:0] return bf[0:5]
elif 4 <= self._memory.version <= 5: elif 4 <= self._memory.version <= 5:
bf = BitField(self._memory[addr]) bf = BitField(self._memory[addr])
return bf[5:0] return bf[0:6]
else: else:
raise ZObjectIllegalVersion raise ZObjectIllegalVersion
@ -552,15 +552,14 @@ class ZObjectParser:
if 1 <= self._memory.version <= 3: if 1 <= self._memory.version <= 3:
bf = BitField(self._memory[size_addr]) bf = BitField(self._memory[size_addr])
size = bf[7:5] + 1 size = bf[5:8] + 1
return size return size
elif 4 <= self._memory.version <= 5: elif 4 <= self._memory.version <= 5:
bf = BitField(self._memory[size_addr]) bf = BitField(self._memory[size_addr])
if bf[7]: if bf[7]:
# Two size bytes, look at second byte # Two-byte header: size is in bits 0-5 of this byte
bf2 = BitField(self._memory[data_address - 2]) size = bf[0:6]
size = bf2[5:0]
if size == 0: if size == 0:
size = 64 size = 64
return size return size

View file

@ -126,7 +126,7 @@ class ZStackManager:
current_routine = self._call_stack[-1] current_routine = self._call_stack[-1]
return current_routine.local_vars[varnum] # type: ignore[possibly-missing-attribute] return current_routine.local_vars[varnum]
def set_local_variable(self, varnum, value): def set_local_variable(self, varnum, value):
"""Set value of local variable VARNUM to VALUE in """Set value of local variable VARNUM to VALUE in
@ -141,19 +141,19 @@ class ZStackManager:
current_routine = self._call_stack[-1] current_routine = self._call_stack[-1]
current_routine.local_vars[varnum] = value # type: ignore[possibly-missing-attribute] current_routine.local_vars[varnum] = value
def push_stack(self, value): def push_stack(self, value):
"Push VALUE onto the top of the current routine's data stack." "Push VALUE onto the top of the current routine's data stack."
current_routine = self._call_stack[-1] current_routine = self._call_stack[-1]
current_routine.stack.append(value) # type: ignore[possibly-missing-attribute] current_routine.stack.append(value)
def pop_stack(self): def pop_stack(self):
"Remove and return value from the top of the data stack." "Remove and return value from the top of the data stack."
current_routine = self._call_stack[-1] current_routine = self._call_stack[-1]
return current_routine.stack.pop() # type: ignore[possibly-missing-attribute] return current_routine.stack.pop()
def get_stack_frame_index(self): def get_stack_frame_index(self):
"Return current stack frame number. For use by 'catch' opcode." "Return current stack frame number. For use by 'catch' opcode."