1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
|
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();
|
