emscripten: Fix undefined behavior in opengles2 renderer

[castholm]

Discovered this while building SDL3 and the Snake example for Emscripten with UBSan. With the sanitizer enabled, running the example logs:

<omitted>/src/render/opengles2/SDL_render_gles2.c:1024:74: runtime error: subtraction of unsigned offset from 0x00000000 overflowed to 0x00000000
<omitted>/src/render/opengles2/SDL_render_gles2.c:1025:118: runtime error: member access within null pointer of type 'SDL_VertexSolid' (aka 'struct SDL_VertexSolid')
<omitted>/src/render/opengles2/SDL_render_gles2.c:1026:131: runtime error: member access within null pointer of type 'SDL_VertexSolid' (aka 'struct SDL_VertexSolid')

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 when foo 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 and offsetof 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.

[slouken]

Merged, thanks!

[dschuff]

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?

[slouken]

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.

[dschuff]

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?

[castholm]

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).

SDL/src/render/opengles2/SDL_render_gles2.c

Lines 1012 to 1014 in 2442c85

	SDL_VertexSolid *verts = (SDL_VertexSolid *)(((Uint8 *)vertices) + cmd->data.draw.first);
	data->glVertexAttribPointer(GLES2_ATTRIBUTE_POSITION, 2, GL_FLOAT, GL_FALSE, stride, (const GLvoid *)&verts->position);
	data->glVertexAttribPointer(GLES2_ATTRIBUTE_COLOR, 4, GL_UNSIGNED_BYTE, GL_TRUE /* Normalized */, stride, (const GLvoid *)&verts->color);

SDL/src/render/opengles2/SDL_render_gles2.c

Lines 1062 to 1064 in 4ef8b6c

	uintptr_t base = (uintptr_t)vertices + cmd->data.draw.first; // address of first vertex, or base offset when using VBOs.
	data->glVertexAttribPointer(GLES2_ATTRIBUTE_POSITION, 2, GL_FLOAT, GL_FALSE, stride, (const GLvoid *)(base + offsetof(SDL_VertexSolid, position)));
	data->glVertexAttribPointer(GLES2_ATTRIBUTE_COLOR, 4, GL_FLOAT, GL_TRUE /* Normalized */, stride, (const GLvoid *)(base + offsetof(SDL_VertexSolid, color)));

In SDL2 the color is a GL_UNSIGNED_BYTE, in SDL3 it was changed to GL_FLOAT in 554f062 so if you applied the patch as-is you have to change it back to GL_UNSIGNED_BYTE. I think that's the only affected line that differs between SDL2 and SDL3 but please double check for any similar potential issues.

[dschuff]

Thanks! I opened #12937