nocrew / ostis

Atari ST Emulator
GNU Lesser General Public License v2.1
20 stars 4 forks source link

FNIL causes segfault #111

Closed stefanberndtsson closed 8 years ago

stefanberndtsson commented 8 years ago

Running FNIL.MSA gives segfault with a rasterpos out of bounds.

$ gdb ./ostis-gdb core
GNU gdb (Ubuntu 7.10-1ubuntu2) 7.10
Copyright (C) 2015 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
<http://www.gnu.org/software/gdb/documentation/>.
For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from ./ostis-gdb...done.
[New LWP 32001]
[New LWP 32003]
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
Core was generated by `./ostis-gdb /var/tmp/demos/FNIL.MSA'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0  0x0000000000412b02 in screen_draw (r=0, g=0, b=0) at screen.c:317
317   *rasterpos++ = r;
[Current thread is 1 (Thread 0x7fa81e90d7c0 (LWP 32001))]
(gdb) print rasterpos
$1 = 0x7fa81e8ee001 "ELF\002\001\001"
(gdb) bt
#0  0x0000000000412b02 in screen_draw (r=0, g=0, b=0) at screen.c:317
#1  0x000000000041104d in shifter_draw_high () at shifter.c:260
#2  0x00000000004112fe in shifter_clock () at shifter.c:334
#3  0x00000000004093f7 in cpu_do_cycle (cnt=12) at cpu.c:369
#4  0x0000000000409162 in cpu_step_instr (trace=0) at cpu.c:259
#5  0x000000000040930d in cpu_run (cpu_run_state=0) at cpu.c:320
#6  0x0000000000403301 in main (argc=2, argv=0x7ffd6149db78) at main.c:251
(gdb) frame 5
#5  0x000000000040930d in cpu_run (cpu_run_state=0) at cpu.c:320
320       ret = cpu_step_instr(tmp_run_state);
(gdb) print cpu->pc
$2 = 68452
(gdb) 

68452 == $10b64

No interaction is required, it crashes pretty much immediately after showing the loading screen.

larsbrinkhoff commented 8 years ago

It's trying to draw 501 lines, because it's in high resolution mode near the end of the screen. But there is only memory allocated for 313 lines when a colour monitor is attached.

I'm not sure how the real hardware handles this. I guess no signal is sent to RGB pins.

I think just allocating more memory is the most elegant solution. We're not trying to save RAM.

stefanberndtsson commented 8 years ago

Yep. Give it enough to handle all possible cases without crashing. It's better than segfaults :)

larsbrinkhoff commented 8 years ago

Updated with comment in the code.