WHDLoad MantisBT - WHDLoad
View Issue Details
0006078WHDLoad[All Projects] Generalpublic2023-03-11 22:152023-10-14 23:27
ReporterDJ Mike 
Assigned ToWepl 
PrioritynormalSeveritymajorReproducibilityalways
StatusclosedResolutionfixed 
PlatformOSOS Version
Product Version18.7 
Target Version18.9Fixed in Version18.9beta 
MachineA1200
CPU68030
CPUSpeed50
ChipSetAGA
GFXCardNone
ChipMem2 MB
FastMem0 MB
WorkbenchOS 3.0
KickROM39 - Kick 3.0
KickSoftNone
WHDLoad18.7
Summary0006078: resload_SaveFileOffset causes file corruption if NOWRITECACHE used
DescriptionWhile looking at an old patch for Chuck Yeager AFT 2.0, I noticed that the disk image becomes corrupted when the game/slave tries to save data using resload_SaveFileOffset, if NOWRITECACHE is enabled.

If write cache is enabled, the data saves just fine when WHDLoad exits. The issue seems to be with swapping to OS to do an immediate save.

On further investigation, the issue affects all WHDLoad versions from 18.4 onwards - 18.3 does NOT exhibit this issue (nor does 18.0 which I also tested).

The data stored seems to be mostly random garbage, although it sometimes has interesting results such as the attached screenshot - the WHDLoad filenames present are from LZX that I had just run to extract an older version of WHDLoad as part of my investigation, which may mean the memory being saved to disk is being overwritten by the OS restore?
Steps To Reproduce1. Load Chuck Yeager AFT 2.0 install, with NOWRITECACHE enabled
2. In game, open menu with ESC, select Missions -> Demo Flight
3. Wait for OS swaps to save to disk.
4. Exit, and load Disk.1 into a hex editor
5. Locate offset $DAA00 (where game saves data) - notice the random garbage that has replaced the data that should be there.
TagsNo tags attached.
Attached Filespng nowritecache.png (112,633) 2023-03-11 22:15
http://www.whdload.de/mantis/file_download.php?file_id=1569&type=bug
png

? .whdl_trace (6,362) 2023-03-13 00:15
http://www.whdload.de/mantis/file_download.php?file_id=1570&type=bug
? WHDLoad (153,728) 2023-03-15 22:47
http://www.whdload.de/mantis/file_download.php?file_id=1571&type=bug
? A1200.whdl_trace (10,936) 2023-03-16 18:18
http://www.whdload.de/mantis/file_download.php?file_id=1572&type=bug

Notes
(0012494)
Wepl   
2023-03-12 00:26   
thanks for the report, investigating...
(0012500)
Wepl   
2023-03-12 21:35   
do you have option FullChip active?
(0012501)
DJ Mike   
2023-03-12 21:46   
(Last edited: 2023-03-12 21:48)
No - option is disabled.

If I enable FullChip, that does fix the problem. That's tested both on my real Amiga 1200, and under WinUAE.

(0012503)
Wepl   
2023-03-12 23:49   
Would it be possible to make a run where bad the SaveFileOffset happens with the option TRACE set and attach the created .whdl_trace file?
(0012504)
DJ Mike   
2023-03-13 00:15   
Trace attached.

A1 being close to the end of basemem is interesting - I'm fairly sure I had this same issue months ago patching another game. I was using spare memory close to the end of basemem to save some data, and getting corruption. At the time I just thought it was a WinUAE oddity though, so I changed the slave to use a lower chipmem address and didn't dig deeper.
(0012505)
DJ Mike   
2023-03-13 00:23   
Also trace was taken from WinUAE but I'm happy to take one from real Amiga if needed.
(0012506)
Wepl   
2023-03-13 00:29   
Thanks a lot.
I wonder why such a terrible bug hasn't been discovered earlier...
(0012507)
DJ Mike   
2023-03-13 00:34   
Does this affect all save functions or just SaveFileOffset? I can't imagine many slaves use the latter.
(0012508)
Wepl   
2023-03-14 15:42   
I'm currently trying to write a test to reproduce it. Did not yet succeed.
(0012510)
Wepl   
2023-03-15 22:54   
I'm unable to reproduce your problem, neither with ChuckYeager nor with some synthetic tests.
But I found a bug which corrupts io operations when the used memory lies completely in ShadowHi memory. But this is not the case in your trace where ShadowHi is unused. It also doesn't match that FullChip should avoid the problem.

Can you please try if the attached WHDLoad fixes your problem?
If not please make a trace file and attach it.
(0012511)
DJ Mike   
2023-03-16 02:42   
(Last edited: 2023-03-16 02:52)
With your new WHDLoad the problem is fixed!

Tested on both my WinUAE and real Amiga setups.

Would you like me to do any further checks? If the change shouldn't really have fixed the problem it might be useful to know why it worked anyway.

(0012512)
Wepl   
2023-03-16 11:35   
I don't know what could be tested more.
I will create a new beta release containing the fix.
Thanks :)
(0012513)
DJ Mike   
2023-03-16 11:39   
Well it's more you mention the fix pertains to ShadowHi but my trace indicated ShadowHi was not used. I was wondering if something might be wrong there - perhaps ShadowHi is being used and misreported by trace?

Either way I'm glad the fix has worked, so I'm happy if you're happy. :-)

Cheers Bert!
(0012514)
Wepl   
2023-03-16 11:54   
No, the trace function is correct.
Was the trace really from a run where the Save corrupted the file?
The fix/bug was located in a virtual function which handles all io functions. So all read/write operations were affected. If you use only NoWriteCache then only write operations are affected because all reads will go through the filecache. Using NoFileCache also reads should be affected in the non fixed WHDLoad.
(0012516)
DJ Mike   
2023-03-16 13:01   
I ran the trace up to the point WHDL writes to the file, then exited. I then confirmed corruption in the resulting file.

I will try a trace on my real Amiga tonight (using unfixed WHDL) and see if that yields a different result.
(0012517)
DJ Mike   
2023-03-16 18:18   
Attached trace file from my real A1200, running WHDLoad 18.6.

There are two runs of the slave - first is with FullChip disabled (where Disk.1 becomes corrupt), second with FullChip enabled (Disk.1 is OK).
(0012518)
Wepl   
2023-03-16 21:54   
Fine. That fully matches the bug fixed now.
Without FullChip the save lies inside Hi.
With FullChip a larger block is choosen for AbsMem which is located above $80000 and so the save lies inside Lo and the bug is not active.
The first trace was misleading.
(0012519)
Wepl   
2023-03-16 23:34   
new beta is released: https://whdload.de/whdload/whd189dev.lha
(0012521)
DJ Mike   
2023-03-17 00:03   
OK great. Thanks a lot!

Issue History
2023-03-11 22:15DJ MikeNew Issue
2023-03-11 22:15DJ MikeFile Added: nowritecache.png
2023-03-11 22:19DJ MikeAssigned To => Wepl
2023-03-11 22:19DJ MikeStatusnew => assigned
2023-03-12 00:26WeplNote Added: 0012494
2023-03-12 16:05WeplTarget Version => 18.9
2023-03-12 21:35WeplNote Added: 0012500
2023-03-12 21:46DJ MikeNote Added: 0012501
2023-03-12 21:48DJ MikeNote Edited: 0012501bug_revision_view_page.php?bugnote_id=12501#r1606
2023-03-12 23:49WeplNote Added: 0012503
2023-03-13 00:15DJ MikeFile Added: .whdl_trace
2023-03-13 00:15DJ MikeNote Added: 0012504
2023-03-13 00:23DJ MikeNote Added: 0012505
2023-03-13 00:29WeplNote Added: 0012506
2023-03-13 00:34DJ MikeNote Added: 0012507
2023-03-14 15:42WeplNote Added: 0012508
2023-03-15 22:47WeplFile Added: WHDLoad
2023-03-15 22:54WeplNote Added: 0012510
2023-03-15 22:54WeplProduct Version18.8 => 18.7
2023-03-16 02:42DJ MikeNote Added: 0012511
2023-03-16 02:52DJ MikeNote Edited: 0012511bug_revision_view_page.php?bugnote_id=12511#r1608
2023-03-16 11:35WeplNote Added: 0012512
2023-03-16 11:39DJ MikeNote Added: 0012513
2023-03-16 11:54WeplNote Added: 0012514
2023-03-16 13:01DJ MikeNote Added: 0012516
2023-03-16 18:18DJ MikeFile Added: A1200.whdl_trace
2023-03-16 18:18DJ MikeNote Added: 0012517
2023-03-16 21:54WeplNote Added: 0012518
2023-03-16 23:34WeplNote Added: 0012519
2023-03-16 23:34WeplStatusassigned => resolved
2023-03-16 23:34WeplResolutionopen => fixed
2023-03-16 23:34WeplFixed in Version => 18.9beta
2023-03-17 00:03DJ MikeNote Added: 0012521
2023-10-14 23:27WeplStatusresolved => closed