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

Fix undefined behavior from array “shape-punning” #1194

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

musicinmybrain
Copy link

In stb_voxel_render.h, there were three cases where a 2D array of dimension [X][Y] was iterated as a 1D array of dimension [1][X*Y]. While this is clever and is correct in terms of the actual memory layout, a second index outside the corresponding dimension ([i][j], j >= Y]) actually produces undefined behavior and gives the compiler freedom to do all sorts of terrible things.

The same thing happens in stb_tilemap_editor.h, tests/caveview/cave_mesher.c, and tests/resample_test.cpp.

Prior to this commit, a compiler warning regarding the undefined behavior appears on gcc 11.2.1 for at least some of these cases when the tests are compiled with -Waggressive-loop-optimizations (included in -Wall). For example:

test_c_compilation.c: In function ‘stbvox_make_mesh_for_block_with_geo’:
../stb_voxel_render.h:3134:46: warning: iteration 4 invokes undefined behavior [-Waggressive-loop-optimizations]
 3134 |          int vert = stbvox_vertex_selector[0][i];
      |                     ~~~~~~~~~~~~~~~~~~~~~~~~~^~~
../stb_voxel_render.h:3133:19: note: within this loop
 3133 |       for (i=0; i < 6*4; ++i) {
      |                 ~~^~~~~
../stb_voxel_render.h:3280:49: warning: iteration 4 invokes undefined behavior [-Waggressive-loop-optimizations]
 3280 |             int vert = stbvox_vertex_selector[0][i];
      |                        ~~~~~~~~~~~~~~~~~~~~~~~~~^~~

This commit fixes the undefined behavior by iterating these 2D arrays with the conventional nested loops.

(This is an important but conceptually trivial fix, originally suggested by Jerry James. I don’t believe it warrants adding anyone to the list of contributors.)

In stb_voxel_render.h, there were three cases where a 2D array of
dimension [X][Y] was iterated as a 1D array of dimension [1][X*Y]. While
this is clever and is correct in terms of the actual memory layout, a
second index outside the corresponding dimension ([i][j], j >= Y])
actually produces undefined behavior and gives the compiler freedom to
do all sorts of terrible things.

The same thing happens in stb_tilemap_editor.h,
tests/caveview/cave_mesher.c, and tests/resample_test.cpp.

Prior to this commit, a compiler warning regarding the undefined
behavior appears on gcc 11.2.1 for at least some of these cases when the
tests are compiled with -Waggressive-loop-optimizations (included in
-Wall).

This commit fixes the undefined behavior by iterating these 2D arrays
with the conventional nested loops.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants