-
Notifications
You must be signed in to change notification settings - Fork 2.2k
emscripten: Fix undefined behavior in opengles2 renderer #12581
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
Conversation
Merged, thanks! |
A recent change in clang (llvm/llvm-project#130742) has started to exploit this UB, resulting in SDL rendering being broken on Emscripten. What would we need to do to get this backported to SDL2? Do you just use PRs for that? |
Yep, go ahead and create a tested PR and I'll be happy to merge it. |
Hm, I ported this patch more-or-less directly to the SDL2 branch but unfortunately it seems to be breaking our tests (in a way that seems unrelated to the original UB here; it seems to be rendering without any red). I'm wondering if I need to do something to get this to work in SDL2 beyond just applying the change here; before I go digging for that, does this ring any bells? |
SDL/src/render/opengles2/SDL_render_gles2.c Lines 1012 to 1014 in 2442c85
SDL/src/render/opengles2/SDL_render_gles2.c Lines 1062 to 1064 in 4ef8b6c
In SDL2 the color is a |
This fixes undefined behavior resulting from adding offsets to nullptr.
Thanks! I opened #12937 |
This fixes undefined behavior resulting from adding offsets to nullptr.
This fixes undefined behavior resulting from adding offsets to nullptr.
Discovered this while building SDL3 and the Snake example for Emscripten with UBSan. With the sanitizer enabled, running the example logs:
The first error happens because the program is adding an offset to a null pointer, which is UB.
The second and third happen because expressions like
&foo->bar
invoke UB whenfoo
is a null pointer.These only occur on Emscripten, because on Emscripten the renderer uses VBOs instead of client-side arrays and reassigns
vertices
to 0 after uploading the data.By rewriting relevant code to use
uintptr_t
andoffsetof
we get the same results but without invoking UB.From quickly skimming over
SDL_render_gles2.c
in the SDL2 branch it looks like the affected code is present there as well, so if merged this could probably be backported to SDL2 as well.