From 3d401e71452d890eaf0bc50b11788cb08a6c2fed Mon Sep 17 00:00:00 2001
From: "Benjamin A. Beasley" <code@musicinmybrain.net>
Date: Tue, 17 Aug 2021 21:30:44 -0400
Subject: [PATCH] =?UTF-8?q?Fix=20undefined=20behavior=20from=20array=20?=
=?UTF-8?q?=E2=80=9Cshape-punning=E2=80=9D?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
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.
---
stb_tilemap_editor.h | 35 +++++++++++++++++++----------------
stb_voxel_render.h | 36 +++++++++++++++++++++---------------
tests/caveview/cave_mesher.c | 26 ++++++++++++++------------
tests/resample_test.cpp | 15 +++++++++------
4 files changed, 63 insertions(+), 49 deletions(-)
diff --git a/stb_tilemap_editor.h b/stb_tilemap_editor.h
index fbd3388084..0b8c2ca997 100644
--- a/stb_tilemap_editor.h
+++ b/stb_tilemap_editor.h
@@ -1066,14 +1066,15 @@ stbte_tilemap *stbte_create_map(int map_x, int map_y, int map_layers, int spacin
void stbte_set_background_tile(stbte_tilemap *tm, short id)
{
- int i;
+ int i, j;
STBTE_ASSERT(id >= -1);
// STBTE_ASSERT(id < 32768);
if (id < -1)
return;
- for (i=0; i < STBTE_MAX_TILEMAP_X * STBTE_MAX_TILEMAP_Y; ++i)
- if (tm->data[0][i][0] == -1)
- tm->data[0][i][0] = id;
+ for (i=0; i < STBTE_MAX_TILEMAP_X; ++i)
+ for (j=0; j < STBTE_MAX_TILEMAP_Y; ++j)
+ if (tm->data[i][j][0] == -1)
+ tm->data[i][j][0] = id;
tm->background_tile = id;
}
@@ -1212,18 +1213,20 @@ void stbte_set_dimensions(stbte_tilemap *tm, int map_x, int map_y)
void stbte_clear_map(stbte_tilemap *tm)
{
- int i,j;
- for (i=0; i < STBTE_MAX_TILEMAP_X * STBTE_MAX_TILEMAP_Y; ++i) {
- tm->data[0][i][0] = tm->background_tile;
- for (j=1; j < tm->num_layers; ++j)
- tm->data[0][i][j] = STBTE__NO_TILE;
- for (j=0; j < STBTE_MAX_PROPERTIES; ++j)
- tm->props[0][i][j] = 0;
- #ifdef STBTE_ALLOW_LINK
- tm->link[0][i].x = -1;
- tm->link[0][i].y = -1;
- tm->linkcount[0][i] = 0;
- #endif
+ int i,j,k;
+ for (i=0; i < STBTE_MAX_TILEMAP_X; ++i) {
+ for (j=0; j < STBTE_MAX_TILEMAP_Y; ++j) {
+ tm->data[i][j][0] = tm->background_tile;
+ for (k=1; k < tm->num_layers; ++k)
+ tm->data[i][j][k] = STBTE__NO_TILE;
+ for (k=0; k < STBTE_MAX_PROPERTIES; ++k)
+ tm->props[i][j][k] = 0;
+ #ifdef STBTE_ALLOW_LINK
+ tm->link[i][j].x = -1;
+ tm->link[i][j].y = -1;
+ tm->linkcount[i][j] = 0;
+ #endif
+ }
}
}
diff --git a/stb_voxel_render.h b/stb_voxel_render.h
index 2e7a372f83..51011091f7 100644
--- a/stb_voxel_render.h
+++ b/stb_voxel_render.h
@@ -3126,15 +3126,17 @@ static void stbvox_make_mesh_for_block_with_geo(stbvox_mesh_maker *mm, stbvox_po
stbvox_mesh_vertex vmesh[6][4];
stbvox_rotate rotate = { 0,0,0,0 };
unsigned char simple_rot = rot;
- int i;
+ int i, j;
// we only need to do this for the displayed faces, but it's easier
// to just do it up front; @OPTIMIZE check if it's faster to do it
// for visible faces only
- for (i=0; i < 6*4; ++i) {
- int vert = stbvox_vertex_selector[0][i];
- vert = stbvox_rotate_vertex[vert][rot];
- vmesh[0][i] = stbvox_vmesh_pre_vheight[0][i]
- + stbvox_geometry_vheight[geo][vert];
+ for (i=0; i < 6; ++i) {
+ for (j=0; j < 4; ++j) {
+ int vert = stbvox_vertex_selector[i][j];
+ vert = stbvox_rotate_vertex[vert][rot];
+ vmesh[i][j] = stbvox_vmesh_pre_vheight[i][j]
+ + stbvox_geometry_vheight[geo][vert];
+ }
}
basevert = stbvox_vertex_encode(pos.x, pos.y, pos.z << STBVOX_CONFIG_PRECISION_Z, 0,0);
@@ -3275,11 +3277,13 @@ static void stbvox_make_mesh_for_block_with_geo(stbvox_mesh_maker *mm, stbvox_po
// build vertex mesh
{
- int i;
- for (i=0; i < 6*4; ++i) {
- int vert = stbvox_vertex_selector[0][i];
- vmesh[0][i] = stbvox_vmesh_pre_vheight[0][i]
- + cube[vert];
+ int i, j;
+ for (i=0; i < 6; ++i) {
+ for (j=0; j < 4; ++j) {
+ int vert = stbvox_vertex_selector[i][j];
+ vmesh[i][j] = stbvox_vmesh_pre_vheight[i][j]
+ + cube[vert];
+ }
}
}
@@ -3541,10 +3545,12 @@ int stbvox_get_buffer_size_per_quad(stbvox_mesh_maker *mm, int n)
void stbvox_reset_buffers(stbvox_mesh_maker *mm)
{
- int i;
- for (i=0; i < STBVOX_MAX_MESHES*STBVOX_MAX_MESH_SLOTS; ++i) {
- mm->output_cur[0][i] = 0;
- mm->output_buffer[0][i] = 0;
+ int i, j;
+ for (i=0; i < STBVOX_MAX_MESHES; ++i) {
+ for (j=0; j < STBVOX_MAX_MESH_SLOTS; ++j) {
+ mm->output_cur[i][j] = 0;
+ mm->output_buffer[i][j] = 0;
+ }
}
}
diff --git a/tests/caveview/cave_mesher.c b/tests/caveview/cave_mesher.c
index 1f76c89812..bbf79898b6 100644
--- a/tests/caveview/cave_mesher.c
+++ b/tests/caveview/cave_mesher.c
@@ -802,7 +802,7 @@ void remap_in_place(int bt, int rm)
void mesh_init(void)
{
- int i;
+ int i, j;
chunk_cache_mutex = SDL_CreateMutex();
chunk_get_mutex = SDL_CreateMutex();
@@ -814,17 +814,19 @@ void mesh_init(void)
}
//effective_blocktype[50] = 0; // delete torches
- for (i=0; i < 6*256; ++i) {
- if (minecraft_tex1_for_blocktype[0][i] == 40)
- minecraft_color_for_blocktype[0][i] = 38 | 64; // apply to tex1
- if (minecraft_tex1_for_blocktype[0][i] == 39)
- minecraft_color_for_blocktype[0][i] = 39 | 64; // apply to tex1
- if (minecraft_tex1_for_blocktype[0][i] == 105)
- minecraft_color_for_blocktype[0][i] = 63; // emissive
- if (minecraft_tex1_for_blocktype[0][i] == 212)
- minecraft_color_for_blocktype[0][i] = 63; // emissive
- if (minecraft_tex1_for_blocktype[0][i] == 80)
- minecraft_color_for_blocktype[0][i] = 63; // emissive
+ for (i=0; i < 6; ++i) {
+ for (j=0; j < 256; ++j) {
+ if (minecraft_tex1_for_blocktype[i][j] == 40)
+ minecraft_color_for_blocktype[i][j] = 38 | 64; // apply to tex1
+ if (minecraft_tex1_for_blocktype[i][j] == 39)
+ minecraft_color_for_blocktype[i][j] = 39 | 64; // apply to tex1
+ if (minecraft_tex1_for_blocktype[i][j] == 105)
+ minecraft_color_for_blocktype[i][j] = 63; // emissive
+ if (minecraft_tex1_for_blocktype[i][j] == 212)
+ minecraft_color_for_blocktype[i][j] = 63; // emissive
+ if (minecraft_tex1_for_blocktype[i][j] == 80)
+ minecraft_color_for_blocktype[i][j] = 63; // emissive
+ }
}
for (i=0; i < 6; ++i) {
diff --git a/tests/resample_test.cpp b/tests/resample_test.cpp
index 21f874f18b..bb8ad82ef6 100644
--- a/tests/resample_test.cpp
+++ b/tests/resample_test.cpp
@@ -646,8 +646,9 @@ void verify_box(void)
resample_88(STBIR_FILTER_BOX);
- for (i=0; i < sizeof(image88); ++i)
- STBIR_ASSERT(image88[0][i] == output88[0][i]);
+ for (i=0; i < sizeof(image88) / sizeof(image88[0]); ++i)
+ for (j=0; j < sizeof(image88[0]); ++j)
+ STBIR_ASSERT(image88[i][j] == output88[i][j]);
t = 0;
for (j=0; j < 4; ++j)
@@ -685,12 +686,14 @@ void test_filters(void)
mtsrand(0);
- for (i=0; i < sizeof(image88); ++i)
- image88[0][i] = mtrand() & 255;
+ for (i=0; i < sizeof(image88) / sizeof(image88[0]); ++i)
+ for (j=0; j < sizeof(image88[0]); ++j)
+ image88[i][j] = mtrand() & 255;
verify_box();
- for (i=0; i < sizeof(image88); ++i)
- image88[0][i] = 0;
+ for (i=0; i < sizeof(image88) / sizeof(image88[0]); ++i)
+ for (j=0; j < sizeof(image88[0]); ++j)
+ image88[i][j] = 0;
image88[4][4] = 255;
verify_box();
