robhagemans / pcbasic

PC-BASIC - A free, cross-platform emulator for the GW-BASIC family of interpreters
http://www.pc-basic.org
Other
397 stars 48 forks source link

crash in PUT in EGA modes #128

Closed incanus closed 2 years ago

incanus commented 3 years ago

Bug report

Problem

In both Python 2 and 3, there are crash bugs with using PUT.

Steps

  1. EGA screen modes 7-9
  2. GET a sprite
  3. Try to PUT it

Program

10 SIZE=20
20 DIM S(SIZE*SIZE)
30 SCREEN 7 : CLS
40 LINE(100,100)-(100+SIZE,100+SIZE),4,BF
50 GET(100,100)-(100+SIZE,100+SIZE),S
60 CLS
70 PLAY "p1 p1"
80 PUT(100,100),S

Crash log

Python 2:

PC-BASIC crash log
====================================================================================================
FATAL ERROR
version   2.0.3 [ unreleased ]
python    2.7.16 [64bit ] 
platform  Darwin-20.1.0-x86_64-i386-64bit
interface VideoSDL2, AudioSDL2
statement 80 PUT(100,100),S

statements.py:82, parse_statement
graphics.py:946, put_
framebuffer.py:159, unpack
bytematrix.py:209, frompacked
TypeError: object of type 'generator' has no len()

This is a bug in PC-BASIC.
Sorry about that. You can help improve PC-BASIC:

- Please file a bug report, including this message and the steps you took
  just before the crash. Go to:
    https://github.com/robhagemans/pcbasic/issues

- Please include the full crash log in your report.
  You can paste it from the clipboard or from the file at:
    /Users/incanus/Library/Application Support/pcbasic-2.0/crash-20201119-4uaynZ.log
Traceback (most recent call last):
  File "pcbasic/pcbasic/guard.py", line 51, in protect
    yield
  File "pcbasic/pcbasic/main.py", line 118, in _run_session
    session.execute(cmd)
  File "pcbasic/pcbasic/basic/api.py", line 91, in execute
    self._impl.execute(cmd)
  File "pcbasic/pcbasic/basic/implementation.py", line 252, in execute
    self.interpreter.loop()
  File "pcbasic/pcbasic/basic/interpreter.py", line 122, in loop
    self.parse()
  File "pcbasic/pcbasic/basic/interpreter.py", line 112, in parse
    self.parser.parse_statement(ins)
  File "pcbasic/pcbasic/basic/parser/statements.py", line 82, in parse_statement
    self._callbacks[c](parse_args(ins))
  File "pcbasic/pcbasic/basic/display/graphics.py", line 946, in put_
    sprite = self._mode.sprite_builder.unpack(packed_sprite)
  File "pcbasic/pcbasic/basic/display/framebuffer.py", line 159, in unpack
    packed, height=height*self._number_planes, items_per_byte=8
  File "pcbasic/pcbasic/basic/base/bytematrix.py", line 209, in frompacked
    width = len(packed) // height
TypeError: object of type 'generator' has no len()

Python 3:

PC-BASIC crash log
====================================================================================================
FATAL ERROR
version   2.0.3 [ unreleased ]
python    3.8.6 [64bit ] 
platform  macOS-10.16-x86_64-i386-64bit
interface VideoSDL2, AudioSDL2
statement 80 PUT(100,100),S

statements.py:82, parse_statement
graphics.py:946, put_
framebuffer.py:158, unpack
bytematrix.py:212, frompacked
ValueError: range() arg 3 must not be zero

This is a bug in PC-BASIC.
Sorry about that. You can help improve PC-BASIC:

- Please file a bug report, including this message and the steps you took
  just before the crash. Go to:
    https://github.com/robhagemans/pcbasic/issues

- Please include the full crash log in your report.
  You can paste it from the clipboard or from the file at:
    /Users/incanus/Library/Application Support/pcbasic-2.0/crash-20201119-2_t040qx.log
Traceback (most recent call last):
  File "pcbasic/pcbasic/guard.py", line 51, in protect
    yield
  File "pcbasic/pcbasic/main.py", line 119, in _run_session
    session.interact()
  File "pcbasic/pcbasic/basic/api.py", line 149, in interact
    self._impl.interact()
  File "pcbasic/pcbasic/basic/implementation.py", line 315, in interact
    self.interpreter.loop()
  File "pcbasic/pcbasic/basic/interpreter.py", line 122, in loop
    self.parse()
  File "pcbasic/pcbasic/basic/interpreter.py", line 112, in parse
    self.parser.parse_statement(ins)
  File "pcbasic/pcbasic/basic/parser/statements.py", line 82, in parse_statement
    self._callbacks[c](parse_args(ins))
  File "pcbasic/pcbasic/basic/display/graphics.py", line 946, in put_
    sprite = self._mode.sprite_builder.unpack(packed_sprite)
  File "pcbasic/pcbasic/basic/display/framebuffer.py", line 158, in unpack
    allplanes = bytematrix.ByteMatrix.frompacked(
  File "pcbasic/pcbasic/basic/base/bytematrix.py", line 212, in frompacked
    for _offs in xrange(0, len(packed), width)
ValueError: range() arg 3 must not be zero

Notes

In both cases, it seems to be related to something in the EGA PlanedSpriteBuilder as the CGA PackedSpriteBuilder does not exhibit the problems (e.g. screens 1 & 2). Haven't tested Tandy.

PC-BASIC version: 2.0.3 as well as develop as of 33a20372b2499592863020159bcbb9966135e0ec. Operating system version: macOS 11.0.1

incanus commented 3 years ago

Running a git bisect between v2.0.2-1 (known good) and v2.0.3 for Python 2 goes through ~400 commits and points to 41fc3c7c99bebe68c4ec3f9345cb31abdc67f508 as the problem.

robhagemans commented 3 years ago

Cool, thanks for the bisect and the stack traces. From the trace, it looks like there's some work to do on bytematrix.

For the record, from 2.0.2 to 2.0.3 I think I replaced reams of graphics code as I was getting rid of the dependency on numpy, so it's not completely a surprise that there are some bugs there. Between EGA and CGA a different byte-packing scheme is used so that would explain why you don't see the bug in CGA.

I'll have a look at it.

incanus commented 3 years ago

Yes! I’ve seen the sweeping changes. I appreciate not needing numpy anymore, though.

robhagemans commented 2 years ago

Turns out the whole EGA sprite logic was buggy. Resolved now on develop by commits 5d186592e and 752eb51b0.