Skip to content

Ensure that GLES Canvas batch pointer is never a nullptr#167

Merged
DanielaOrtner merged 1 commit into
RebelToolbox:mainfrom
madmiraal:fix-gles-array-bounds-error
May 5, 2025
Merged

Ensure that GLES Canvas batch pointer is never a nullptr#167
DanielaOrtner merged 1 commit into
RebelToolbox:mainfrom
madmiraal:fix-gles-array-bounds-error

Conversation

@madmiraal
Copy link
Copy Markdown
Contributor

Currently, when requesting a new GLES Canvas Batch, a nullptr will be returned if the array is full. When a nullptr is returned, the Batch arrays are grown, and a new Batch is requested:

Batch* batch = bdata.batches.request();
if (!batch) {
// grow the batches
bdata.batches.grow();
// and the temporary batches (used for color verts)
bdata.batches_temp.reset();
bdata.batches_temp.grow();
// this should always succeed after growing
batch = bdata.batches.request();
RAST_DEBUG_ASSERT(batch);
}

Since, after the array is grown, it can never be full, we can be certain that, when now requesting a new GLES Canvas Batch, a nullptr will never be returned. However, the compiler cannot be certain of this, and it complains that, if a nullptr is returned, we cannot use memset on it:

In function 'void* memset(void*, int, size_t)',
    inlined from 'RasterizerCanvasBatcher<T, T_STORAGE>::Batch* RasterizerCanvasBatcher<T, T_STORAGE>::_batch_request_new(bool) [with T = RasterizerCanvasGLES3; T_STORAGE = RasterizerStorageGLES3]' at ./drivers/gles_common/rasterizer_canvas_batcher.h:840:19,
    inlined from 'void RasterizerCanvasBatcher<T, T_STORAGE>::_prefill_default_batch(FillState&, int, const RasterizerCanvas::Item&) [with T = RasterizerCanvasGLES3; T_STORAGE = RasterizerStorageGLES3]' at ./drivers/gles_common/rasterizer_canvas_batcher.h:1090:63,
    inlined from 'void RasterizerCanvasBatcher<T, T_STORAGE>::_prefill_default_batch(FillState&, int, const RasterizerCanvas::Item&) [with T = RasterizerCanvasGLES3; T_STORAGE = RasterizerStorageGLES3]' at ./drivers/gles_common/rasterizer_canvas_batcher.h:1027:1:
/usr/include/x86_64-linux-gnu/bits/string_fortified.h:59:33: error: 'void* __builtin_memset(void*, int, long unsigned int)' offset [0, 29] is out of the bounds [0, 0] [-Werror=array-bounds=]
   59 |   return __builtin___memset_chk (__dest, __ch, __len,
      |          ~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~
   60 |                                  __glibc_objsize0 (__dest));
      |                                  ~~~~~~~~~~~~~~~~~~~~~~~~~~

Instead of trying to fix the code to ensure a nullptr is never returned, which would require considerable refactoring, and instead of telling the compiler we will crash Rebel Engine if a nullptr is returned after growing the array (which currently only happens in Rebel Editor and only in debug builds) this PR uses a while loop to convince the compiler that the pointer to the new Batch will never be a nullptr.

@madmiraal madmiraal added the PR Type: Bug Fix Your current game should now work as expected. label May 5, 2025
@DanielaOrtner DanielaOrtner self-requested a review May 5, 2025 17:53
Copy link
Copy Markdown
Contributor

@DanielaOrtner DanielaOrtner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@DanielaOrtner DanielaOrtner merged commit 11093b9 into RebelToolbox:main May 5, 2025
15 checks passed
@madmiraal madmiraal deleted the fix-gles-array-bounds-error branch May 6, 2025 06:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PR Type: Bug Fix Your current game should now work as expected.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants