(Could you CC me on your reply? I'm not subscribed to the ML.)
On 08/01/2018 12:26, Christer Solskogen wrote:
> A oldpos=0 i=0 size=32 name=/home/alarm/amiberry-dev zipname=(null) size=4096 seek=0
> B oldpos=0 i=0 size=32 name=/home/alarm/amiberry-dev zipname=(null) size=4096 seek=0
> A oldpos=0 i=0 size=32 name=/home/alarm/amiberry-dev zipname=(null) size=4096 seek=0
> B oldpos=0 i=0 size=32 name=/home/alarm/amiberry-dev zipname=(null) size=4096 seek=0
>
> (segfaults)
When i=0 and size=32 at the 'B' point, the code eventually calls
kickstart_fix_checksum(mem, size);
HOWEVER, kickstart_fix_checksum() expects AT LEAST a 66-byte buffer
(0x3e + 4). Yet, we pass only a 32-byte buffer... Uh oh.
int ch = size == 524288 ? 0x7ffe8 : (size == 262144 ? 0x3ffe8 : 0x3e);
mem[ch] = 0;
mem[ch + 1] = 0;
mem[ch + 2] = 0;
mem[ch + 3] = 0;
There's an out-of-bound write right there.
> With O2
>
> A oldpos=0 i=0 size=32 name=/home/alarm/amiberry-dev zipname=(null) size=4096 seek=0
> B oldpos=0 i=0 size=32 name=/home/alarm/amiberry-dev zipname=(null) size=4096 seek=0
> A oldpos=0 i=0 size=32 name=/home/alarm/amiberry-dev zipname=(null) size=4096 seek=0
> B oldpos=0 i=0 size=32 name=/home/alarm/amiberry-dev zipname=(null) size=4096 seek=0
> A oldpos=0 i=0 size=32 name=/home/alarm/amiberry-dev zipname=(null) size=4096 seek=0
> B oldpos=0 i=0 size=32 name=/home/alarm/amiberry-dev zipname=(null) size=4096 seek=0
Understanding why O2 works, while O3 crashes requires digging into the generated
assembly code, which can be quite tedious.
> The thing is that the emulator "expects" kickstart rom files in
> kickstart/ - if those files are present there's no segfault with Ofast.
> It only segfaults if there are no kickstart rom files there.
>
> With kickstart files in place, this happens:
>
> A oldpos=0 i=11 size=32 name=KS ROM v1.3 (A500,A1000,A2000) zipname=(null) size=262144 seek=11
> B oldpos=0 i=32 size=32 name=KS ROM v1.3 (A500,A1000,A2000) zipname=(null) size=262144 seek=32
> A oldpos=0 i=11 size=32 name=KS ROM v1.3 (A500,A1000,A2000) zipname=(null) size=262144 seek=11
> B oldpos=0 i=32 size=32 name=KS ROM v1.3 (A500,A1000,A2000) zipname=(null) size=262144 seek=32
> A oldpos=0 i=11 size=32 name=KS ROM v1.3 (A500,A1000,A2000) zipname=(null) size=262144 seek=11
> B oldpos=0 i=32 size=32 name=KS ROM v1.3 (A500,A1000,A2000) zipname=(null) size=262144 seek=32
OK. The proper (obvious?) fix appears to bail from read_kickstart()
if we were not able to actually read the kickstart file.
Can you try the following patch, and run the program using valgrind?
diff --git a/src/memory.cpp b/src/memory.cpp
index b91d6f224f2e..ec2990c31ba4 100644
--- a/src/memory.cpp
+++ b/src/memory.cpp
@@ -578,6 +578,8 @@ static int read_kickstart (struct zfile *f, uae_u8 *mem, int size, int dochecksu
}
oldpos = zfile_ftell (f);
i = zfile_fread (buffer, 1, 11, f);
+ if (i < 11) /* Unable to read kickstart file */
+ return 0;
if (!memcmp(buffer, "KICK", 4)) {
zfile_fseek (f, 512, SEEK_SET);
kickdisk = 1;
Regards.
