robhagemans / pcbasic

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

PC-BASIC crashes with --max-memory > 66062 #144

Closed rbergen closed 2 years ago

rbergen commented 2 years ago

Bug report

Problem When starting PC-BASIC with a --max-memory argument set to a value larger than 66062, it crashes with a PC-BASIC "blue screen".

Steps

  1. Install PC-BASIC on Windows 10
  2. Open PowerShell
  3. Execute the command pcbasic --max-memory=66063

Crash log

PC-BASIC crash log
====================================================================================================
FATAL ERROR
version   2.0.3 [v2.0.2-1-820-g6807f35bb 2020-09-26 16:12:18.217000]
python    2.7.18 [32bit WindowsPE] True
platform  Windows-10-10.0.19041
interface VideoSDL2, AudioSDL2
statement :KEY ON:PRINT "PC-BASIC 2.0.3":PRINT "(C) Copyright 2013--2020 Rob Hagemans.":PRINT USING "##### Bytes free"; FRE(0)

expressions.py:308, parse
expressions.py:340, read_string_literal
values.py:247, from_str_at
strings.py:51, from_pointer
error: ushort format requires 0 <= number <= USHRT_MAX

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:
    C:\Users\rberg\AppData\Roaming\pcbasic-2.0\crash-20211017-zksdxu.log
Traceback (most recent call last):
  File "pcbasic\guard.py", line 51, in protect
  File "pcbasic\main.py", line 112, in _run_session
  File "pcbasic\basic\api.py", line 143, in greet
  File "pcbasic\basic\implementation.py", line 252, in execute
  File "pcbasic\basic\interpreter.py", line 122, in loop
  File "pcbasic\basic\interpreter.py", line 112, in parse
  File "pcbasic\basic\parser\statements.py", line 82, in parse_statement
  File "pcbasic\basic\devices\files.py", line 498, in print_
  File "pcbasic\basic\devices\formatter.py", line 39, in format
  File "pcbasic\basic\parser\statements.py", line 1465, in _parse_print
  File "pcbasic\basic\parser\statements.py", line 98, in parse_expression
  File "pcbasic\basic\parser\expressions.py", line 236, in parse_expression
  File "pcbasic\basic\parser\expressions.py", line 308, in parse
  File "pcbasic\basic\parser\expressions.py", line 340, in read_string_literal
  File "pcbasic\basic\values\values.py", line 247, in from_str_at
  File "pcbasic\basic\values\strings.py", line 51, in from_pointer
error: ushort format requires 0 <= number <= USHRT_MAX

==== Screen Pages ==================================================================================
   +--------------------------------------------------------------------------------+
 0 |                                                                                |  0
 1 |                                                                                |  0
 2 |                                                                                |  0
 3 |                                                                                |  0
 4 |                                                                                |  0
 5 |                                                                                |  0
 6 |                                                                                |  0
 7 |                                                                                |  0
 8 |                                                                                |  0
 9 |                                                                                |  0
10 |                                                                                |  0
11 |                                                                                |  0
12 |                                                                                |  0
13 |                                                                                |  0
14 |                                                                                |  0
15 |                                                                                |  0
16 |                                                                                |  0
17 |                                                                                |  0
18 |                                                                                |  0
19 |                                                                                |  0
20 |                                                                                |  0
21 |                                                                                |  0
22 |                                                                                |  0
23 |                                                                                |  0
24 | 1LIST   2RUN   3LOAD"  4SAVE"  5CONT  6,"LPT1 7TRON  8TROFF 9KEY    0SCREEN| 80
   +--------------------------------------------------------------------------------+
   +--------------------------------------------------------------------------------+
 0 |                                                                                |  0
 1 |                                                                                |  0
 2 |                                                                                |  0
 3 |                                                                                |  0
 4 |                                                                                |  0
 5 |                                                                                |  0
 6 |                                                                                |  0
 7 |                                                                                |  0
 8 |                                                                                |  0
 9 |                                                                                |  0
10 |                                                                                |  0
11 |                                                                                |  0
12 |                                                                                |  0
13 |                                                                                |  0
14 |                                                                                |  0
15 |                                                                                |  0
16 |                                                                                |  0
17 |                                                                                |  0
18 |                                                                                |  0
19 |                                                                                |  0
20 |                                                                                |  0
21 |                                                                                |  0
22 |                                                                                |  0
23 |                                                                                |  0
24 |                                                                                |  0
   +--------------------------------------------------------------------------------+
   +--------------------------------------------------------------------------------+
 0 |                                                                                |  0
 1 |                                                                                |  0
 2 |                                                                                |  0
 3 |                                                                                |  0
 4 |                                                                                |  0
 5 |                                                                                |  0
 6 |                                                                                |  0
 7 |                                                                                |  0
 8 |                                                                                |  0
 9 |                                                                                |  0
10 |                                                                                |  0
11 |                                                                                |  0
12 |                                                                                |  0
13 |                                                                                |  0
14 |                                                                                |  0
15 |                                                                                |  0
16 |                                                                                |  0
17 |                                                                                |  0
18 |                                                                                |  0
19 |                                                                                |  0
20 |                                                                                |  0
21 |                                                                                |  0
22 |                                                                                |  0
23 |                                                                                |  0
24 |                                                                                |  0
   +--------------------------------------------------------------------------------+
   +--------------------------------------------------------------------------------+
 0 |                                                                                |  0
 1 |                                                                                |  0
 2 |                                                                                |  0
 3 |                                                                                |  0
 4 |                                                                                |  0
 5 |                                                                                |  0
 6 |                                                                                |  0
 7 |                                                                                |  0
 8 |                                                                                |  0
 9 |                                                                                |  0
10 |                                                                                |  0
11 |                                                                                |  0
12 |                                                                                |  0
13 |                                                                                |  0
14 |                                                                                |  0
15 |                                                                                |  0
16 |                                                                                |  0
17 |                                                                                |  0
18 |                                                                                |  0
19 |                                                                                |  0
20 |                                                                                |  0
21 |                                                                                |  0
22 |                                                                                |  0
23 |                                                                                |  0
24 |                                                                                |  0
   +--------------------------------------------------------------------------------+
==== Scalars =======================================================================================

==== Arrays ========================================================================================

==== Strings =======================================================================================
10000: bytearray(b'PC-BASIC 2.0.3')
==== Program Buffer ================================================================================
00 0000 (ENDS)  
==== Program =======================================================================================
==== Options =======================================================================================
[u'--max-memory=66063']

Notes

PC-BASIC version: 2.0.3 Operating system version: Windows 10, Version 21H1, OS Build 19043.1288

The "blue screen" triggered by this issue looks as follows: pc-basic_max-memory_crash .

Marrin commented 2 years ago

Just checked that with original GW-BASIC and that clears the screen and writes the word "Overflow" and the the DOS prompt is back on the next line.

I would say the bug here is not checking and rejecting this out of range value when processing the command line options.

Just out of curiosity: Why trying this exact value?

rbergen commented 2 years ago

I would say the bug here is not checking and rejecting this out of range value when processing the command line options.

And I would say that's fair enough. What I was trying was a long shot anyway.

Just out of curiosity: Why trying this exact value?

I wasn't, in the sense that I was originally trying much larger values. When I got the blue screen on those, I figured it might help the maintainers to find the exact value at which things break when I filed the bug report.

To answer the question "but what on earth are you tinkering with --max-memory for?" that has not been asked (yet), I need to first explain that I practically started my professional career with GW-BASIC at the age of 12 (we're talking 1989-1990). When I later switched over to QuickBASIC (not to be confused with QBasic), I wrote a text adventure that I then a) finally ended up writing in C, b) later ported to C++, c) published in a GitHub repo and then d) rewrote in C#. Basically, this game has become a pet project for me trying silly things without hurting anyone in the process (...because nobody else cares). When I found out PC-BASIC existed, I instantly decided I wanted to backport R136 to the language with which it all started, but also estimated my chances of running out of GW-BASIC memory at about 99.6%. That's why I figured I'd see how much room to play I could give myself. As it stands, the answer seems to be "60828 Bytes free".

rbergen commented 2 years ago

Oh, I forgot to mention that starting PC-BASIC for the first time literally gave me goosebumps. The image of that screen, including the font and the F-key bar at the bottom, instantly put me back behind the first computer I ever got to play with (read: program on). Regardless of the issue I opened (which could justifiably be considered more a matter of invalid command-line option handling than anything else, as @Marrin pointed out), I think PC-BASIC is an absolutely brilliant thing. I love when old things run in/on modern contexts, and "GW-BASIC" implemented in Python is an absolutely perfect rendition of that, in my view.

Marrin commented 2 years ago

The exact value isn't that interesting here IMHO, other than the fact that it is actually that high. Maximum is the default 65534, i.e. almost the full 64 KiB of the data segment. The option to adjust that is actually there to lower the value. For instance for putting some machine language routines into that freed space. Which doesn't make much sense with PC-BASIC.

Removing the in PC-BASIC unused reserved-memory and lowering max-files to 1:

PC-BASIC 2.0.3
(C) Copyright 2013--2020 Rob Hagemans.
64373 Bytes free
Ok

But that of course would not work with the original GW-BASIC any more and there's just one file buffer. Maybe there are some extra bytes possible using the CLEAR command to lower the stack size.

Is space really an issue? Even if you split the adventure up into different programs, i.e. separating the intro from the main program, and then off-loading texts into data files, shouldn't the game logic itself fit into that space? Maybe even split into more program parts and use CHAIN and COMMON to pass on/preserve variables.

Regarding goosebumps: Putting GWBASIC/QBasic/QuickBasic/Borland C++ … into DOSBox might trigger that feeling too. ;-)

On the other hand those interface(s) can be quite uncomfortable. Having a decent text editor and multiple programs/windows at the same time, is a blessing. Even for retro cross development.

robhagemans commented 2 years ago

Hi, thanks @rbergen for reporting with full stack trace, and @Marrin for checking what GW-BASIC does. Keeping in mind taht GW-BASIC doesn't actually start either with this setting the best solution is probably to exit with an error message - obviously we shouldn't crash.

As you probably know, the main reason there is a memory limitation at all is strings, as they are implemented (in both GW-BASIC and PC-BASIC) as a byte length field followed by a 2-byte pointer to the start of the string in memory - so that can index 64k of memory at most.

Incidentally this is what the Python error in the stack trace hints at - it's trying to fit an overly large number in a 16-bit field.

rbergen commented 2 years ago

Hi, thanks @rbergen for reporting with full stack trace

You're welcome. It's always nice to see an application that goes through the trouble of creating such a digestible crash log in the first place.

As you probably know, the main reason there is a memory limitation at all is strings, as they are implemented (in both GW-BASIC and PC-BASIC) as a byte length field followed by a 2-byte pointer to the start of the string in memory - so that can index 64k of memory at most.

I actually didn't know this. When I used GW-BASIC its inner workings were black magic to me, and I never brought myself up-to-speed on this. But now that you describe it like you do, it makes perfect sense.

Is space really an issue? Even if you split the adventure up into different programs, i.e. separating the intro from the main program, and then off-loading texts into data files, shouldn't the game logic itself fit into that space? Maybe even split into more program parts and use CHAIN and COMMON to pass on/preserve variables.

Oh yes, that would definitely be possible. The issue is largely my very personal and peculiar definition of "nostalgic programming" in GW-BASIC. That includes cramming everything in one program, which is what I used to do back in the day. Which I acknowledge is a bit like cutting my own left hand off (the C++ source includes 35 .cpp files, and almost as many headers). Anyway, now that the limitation that applies has been well established, I can focus my thinking on how exactly to deal with that.

Regarding goosebumps: Putting GWBASIC/QBasic/QuickBasic/Borland C++ … into DOSBox might trigger that feeling too. ;-)

I had already done that, and that does bring back feelings of nostalgia. However, the goosebumps are really related to having it run directly on Windows 10, and in Python to boot. :)

Marrin commented 2 years ago

As you probably know, the main reason there is a memory limitation at all is strings, as they are implemented (in both GW-BASIC and PC-BASIC) as a byte length field followed by a 2-byte pointer to the start of the string in memory - so that can index 64k of memory at most.

I would say the main reason is the 64 KiB segment + offset structure of the 16 bit real mode that lead to such decisions. And even then it doesn't have to be the small memory model by putting all of data + stack — so in this case GW-BASIC's own variables + BASIC program + BASIC variables — all into just one 64 KiB segment.

robhagemans commented 2 years ago

Fixed on develop by PR #146, thanks @rbergen !