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

Added UBSAN complaint fix to rLoadTexture #1891 #3321

Merged
merged 1 commit into from
Sep 18, 2023
Merged

Conversation

Codom
Copy link
Contributor

@Codom Codom commented Sep 17, 2023

I managed to trigger the ubsan by simply calling LoadRenderTexture(1280, 720); from zig. Looking at recent PR's and issues it seems like this issue has already been mitigated elsewhere, but still exists in the codepath for LoadRenderTexture.

All I did was apply a similar fix (as seen in 30a9a24) for #1891 to the rLoadTextures function. This seems to fix the issue.

@raysan5
Copy link
Owner

raysan5 commented Sep 17, 2023

Please note that there is an error, mipOffset += mipSize; is called later in the loop, so dataPtr += mipSize should be added.

@Codom
Copy link
Contributor Author

Codom commented Sep 17, 2023

Ah, missed that. I just recompute the offset from data on every iteration, dataPtr = (unsigned char*)data + mipOffset which is more in line with the previous version.

@raysan5
Copy link
Owner

raysan5 commented Sep 18, 2023

@Codom did you add the chnage? I can't see it.

@Codom
Copy link
Contributor Author

Codom commented Sep 18, 2023

It should be here: f0e7511
Apologies for any confusion.

@raysan5 raysan5 merged commit eb46151 into raysan5:master Sep 18, 2023
@raysan5
Copy link
Owner

raysan5 commented Sep 18, 2023

@Codom I'm merging it but I'm reviewing this implementation, I'll take the variable definition out of the loop and also the conditional check.

raysan5 added a commit that referenced this pull request Sep 18, 2023
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

Successfully merging this pull request may close these issues.

2 participants