Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

heads-up: possible int overflow in hsync_handler_post() (found & fixed in amiberry) #208

Closed
nuumio opened this issue Jan 9, 2022 · 3 comments

Comments

@nuumio
Copy link

nuumio commented Jan 9, 2022

Hi! I'm just giving a heads-up of possible int overflow in hsync_handler_post(): I experienced occasional freezes in Amiberry which originated from (int) casts in hsync_handler_post() function. See: BlitterStudio/amiberry#880

Corresponding lines in WinUAE would be:
https://github.com/tonioni/WinUAE/blob/master/custom.cpp#L11495
https://github.com/tonioni/WinUAE/blob/master/custom.cpp#L11520 (to line 11524)
https://github.com/tonioni/WinUAE/blob/master/custom.cpp#L11560
https://github.com/tonioni/WinUAE/blob/master/custom.cpp#L11572

The problem in Amiberry was that vsyncmintime and rpt (and read_processor_time()) were unsigned longs (64-bit) and they were cast to ints (32-bit).

Changing casts to (int64_t) fixed those freezes in Amiberry. I'm not sure if this is a problem with WinUAE since Windows data model seems to be LLP64 (long and int both 32-bit) vs Linux LP64 (int 32-bit, long 64-bit). But since the code in question is similar in both I though to give a heads-up just in case :)

@tonioni
Copy link
Owner

tonioni commented Jan 9, 2022

It is always 32-bit in WinUAE (uae_u32). It shouldn't be 64-bit, even under 64-bit OS unless all comparisons are fixed to have correct casts. Or better yet, remove those ugly casts and comparisons because 64-bit counter does not need to handle wrap around (which happens every few seconds if 32-bit). I'll probably update this after 4.9.1.

@nuumio
Copy link
Author

nuumio commented Jan 9, 2022

Thanks for the information! I'll leave it for you to decide if to close this issue or keep it open to track the change. I'll try to keep an eye on this on Amiberry side if you make the change and it gets merged.

@tonioni
Copy link
Owner

tonioni commented May 24, 2022

Since few weeks ago, all cycle counters are 64-bit. No more annoying wrap arounds to handle.

@tonioni tonioni closed this as completed May 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants