2000DTM / bizhawk

Automatically exported from code.google.com/p/bizhawk
0 stars 0 forks source link

GB: libgambatte's RTC is broken due to data type issues #122

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
Run the attached movie on a ROM of Pokémon Crystal

What is the expected output? What do you see instead?
If the RTC worked, the movie would sync and there will be no wild Pokémon 
encounters. Instead, it desyncs at Route 30, encountering a Hoppip.

What version of the product are you using? On what operating system?
Any version of libgambatte since r4469 (the revision that introduced the 
repeatable RTC mechanic).

Please provide any additional information below.
Visual Studio uses 64 bit time_t types which breaks libgambatte's RTC code. In 
Cartridge::loadSavedata(cartrigde.cpp:668) the basetime is loaded as 32bit 
unsigned and then assigned to the 64bit signed time_t. Upon reset, the value 
0xFFFFFFFF is read, which should read as -1, but instead it is read as a large 
positive number. This in turn breaks the Rtc::doLatch(rtc.cpp:41) method: tmp 
becomes negative, and all the dataX variables are assigned negative numbers 
(curse you, "%" mechanics!), which makes their values larger than allowed (e.g. 
the number of seconds is larger than 60). This invalid data is passed on to the 
ROM and potentially crashes it. This problem can also occur when loading a 
saved state, it always happens when the baseTime is larger than the current 
time returned by the RTC callback.

Possible fix:
A clean solution probably requires getting rid of the time_t type altogether, 
as it is awfully defined and not needed anyway. The best way may be to use an 
unsigned 32bit type instead, it eliminates the "modulo of negative numbers" 
issue and matches in length with what is defined in the data structure of the 
loading/saving routines. It will also require changing the RTC callback 
signature to some uint32.

Original issue reported on code.google.com by MrWin...@gmail.com on 9 Oct 2013 at 11:18

Attachments:

GoogleCodeExporter commented 9 years ago
I've created a patch file that fixes the issue using the uint32_t type as I 
suggested. Feel free to use it if you want.

Original comment by MrWin...@gmail.com on 10 Oct 2013 at 4:42

Attachments:

GoogleCodeExporter commented 9 years ago
Committed.

Original comment by goyu...@gmail.com on 24 Nov 2013 at 5:33