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

SDL_BlitSurface segfault #8897

Closed
Starbuck5 opened this issue Jan 22, 2024 · 5 comments
Closed

SDL_BlitSurface segfault #8897

Starbuck5 opened this issue Jan 22, 2024 · 5 comments
Milestone

Comments

@Starbuck5
Copy link
Contributor

Starbuck5 commented Jan 22, 2024

I've been chewing on the SDL 2.29.1 pre-release for a few days now trying to figure out some failures and segfaults in the pygame-ce test suite.

I found that one of our image saving routines was segfaulting, and I tracked it down to SDL_BlitSurface and this commit bb969ac

Here is a minimum reproducible example (this segfaults for me):

SDL_Surface* surface = SDL_CreateRGBSurfaceWithFormat(0, 1, 1, 32, SDL_PIXELFORMAT_ARGB8888);
SDL_Surface* linebuf = SDL_CreateRGBSurfaceWithFormat(0, 1, 1, 32, SDL_PIXELFORMAT_ARGB8888);
SDL_Rect srcrect = {0, 0, 1, 1};
SDL_BlitSurface(surface, &srcrect, linebuf, NULL);

This code lacks a dest rect, and SDL_UpperBlit now sends that NULL through into SDL_LowerBlit, where it didn't previously.

@1bsyl
Copy link
Contributor

1bsyl commented Jan 22, 2024

I see, this should be
return SDL_LowerBlit(src, &r_src, dst, &r_dst);
I'll fix that

@1bsyl
Copy link
Contributor

1bsyl commented Jan 22, 2024

done.
for SDL3, that was actually ok 3639743
issue, was when doing the merge probably.

@1bsyl
Copy link
Contributor

1bsyl commented Jan 22, 2024

@Starbuck5 please give a try

@oddbookworm
Copy link

oddbookworm commented Jan 22, 2024

@1bsyl I'm not @Starbuck5, but I just tried with the SDL 2.29.2 prerelease, and it seems to be working correctly now. The tests that were segfaulting before no longer do so with that prerelease. I've got a draft pull request up that's running through the entire test suite on all of our platforms, so when those all finish I can update this comment with any findings

Update: looks fine so far

Pull Request for reference

Update to the update:
New pull request that has the permissions fixed
All of our unit tests pass, so it looks good!

@Starbuck5
Copy link
Contributor Author

Starbuck5 commented Jan 24, 2024

Thanks for the quick action @1bsyl, I've tested your patch and confirmed it works. (no longer segfaults)

Thanks @oddbookworm for checking up on this too :)

@slouken slouken added this to the 2.30.0 milestone Jan 24, 2024
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

4 participants