Convert 0-sized arrays to true flexible array members

Declaring a 0-sized array (e.g 'char arr[0];') as the last member of a structure is a GNU extension that was used to implement flexible array members (FAMs) before they were standardised in C99 as simply '[]'. The GNU extension itself was introduced to work around a hack of declaring 1-sized arrays to mean a variable-length object. The advantage of the 0-sized (and true FAMs) is that they don't count towards the size of the structure. Unit already declares some true FAMs, but it also declared some 0-sized arrays. Converting these 0-sized arrays to true FAMs is not only good for consistency but will also allow better compiler checks now (as in a C99 FAM *must* be the last member of a structure and the compiler will warn otherwise) and in the future as doing this fixes a bunch of warnings (treated as errors in Unit by default) when compiled with -O2 -Warray-bounds -Wstrict-flex-arrays -fstrict-flex-arrays=3 (Note -Warray-bounds is enabled by -Wall and -Wstrict-flex-arrays seems to also be enabled via -Wall -Wextra, the -02 is required to make -fstrict-flex-arrays more effective, =3 is the default on at least GCC 14) such as CC build/src/nxt_upstream.o src/nxt_upstream.c: In function ‘nxt_upstreams_create’: src/nxt_upstream.c:56:18: error: array subscript i is outside array bounds of ‘nxt_upstream_t[0]’ {aka ‘struct nxt_upstream_s[]’} [-Werror=array-bounds=] 56 | string = nxt_str_dup(mp, &upstreams->upstream[i].name, &name); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ In file included from src/nxt_upstream.c:9: src/nxt_upstream.h:55:48: note: while referencing ‘upstream’ 55 | nxt_upstream_t upstream[0]; | ^~~~~~~~ Making our flexible array members proper C99 FAMs and ensuring any >0 sized trailing arrays in structures are really normal arrays will allow to enable various compiler options (such as the above and more) that will help keep our array usage safe. Changing 0-sized arrays to FAMs should have no effect on structure layouts/sizes (they both have a size of 0, although doing a sizeof() on a FAM will result in a compiler error). Looking at pahole(1) output for the nxt_http_route_ruleset_t structure for the [0] and [] cases... $ pahole -C nxt_http_route_ruleset_t /tmp/build/src/nxt_http_route.o typedef struct { uint32_t items; /* 0 4 */ /* XXX 4 bytes hole, try to pack */ nxt_http_route_rule_t * rule[]; /* 8 0 */ /* size: 8, cachelines: 1, members: 2 */ /* sum members: 4, holes: 1, sum holes: 4 */ /* last cacheline: 8 bytes */ } nxt_http_route_ruleset_t; $ pahole -C nxt_http_route_ruleset_t build/src/nxt_http_route.o typedef struct { uint32_t items; /* 0 4 */ /* XXX 4 bytes hole, try to pack */ nxt_http_route_rule_t * rule[]; /* 8 0 */ /* size: 8, cachelines: 1, members: 2 */ /* sum members: 4, holes: 1, sum holes: 4 */ /* last cacheline: 8 bytes */ } nxt_http_route_ruleset_t; Also checking with the size(1) command on the effected object files shows no changes to their sizes $ for file in build/src/nxt_upstream.o \ build/src/nxt_upstream_round_robin.o \ build/src/nxt_h1proto.o \ build/src/nxt_http_route.o \ build/src/nxt_http_proxy.o \ build/src/python/*.o; do \ size -G /tmp/${file} $file; echo; done text data bss total filename 640 418 0 1058 /tmp/build/src/nxt_upstream.o 640 418 0 1058 build/src/nxt_upstream.o text data bss total filename 929 351 0 1280 /tmp/build/src/nxt_upstream_round_robin.o 929 351 0 1280 build/src/nxt_upstream_round_robin.o text data bss total filename 11707 8281 16 20004 /tmp/build/src/nxt_h1proto.o 11707 8281 16 20004 build/src/nxt_h1proto.o text data bss total filename 8319 3101 0 11420 /tmp/build/src/nxt_http_route.o 8319 3101 0 11420 build/src/nxt_http_route.o text data bss total filename 1495 1056 0 2551 /tmp/build/src/nxt_http_proxy.o 1495 1056 0 2551 build/src/nxt_http_proxy.o text data bss total filename 4321 2895 0 7216 /tmp/build/src/python/nxt_python_asgi_http-python.o 4321 2895 0 7216 build/src/python/nxt_python_asgi_http-python.o text data bss total filename 4231 2266 0 6497 /tmp/build/src/python/nxt_python_asgi_lifespan-python.o 4231 2266 0 6497 build/src/python/nxt_python_asgi_lifespan-python.o text data bss total filename 12051 6090 8 18149 /tmp/build/src/python/nxt_python_asgi-python.o 12051 6090 8 18149 build/src/python/nxt_python_asgi-python.o text data bss total filename 28 1963 432 2423 /tmp/build/src/python/nxt_python_asgi_str-python.o 28 1963 432 2423 build/src/python/nxt_python_asgi_str-python.o text data bss total filename 5818 3518 0 9336 /tmp/build/src/python/nxt_python_asgi_websocket-python.o 5818 3518 0 9336 build/src/python/nxt_python_asgi_websocket-python.o text data bss total filename 4391 2089 168 6648 /tmp/build/src/python/nxt_python-python.o 4391 2089 168 6648 build/src/python/nxt_python-python.o text data bss total filename 9095 5909 152 15156 /tmp/build/src/python/nxt_python_wsgi-python.o 9095 5909 152 15156 build/src/python/nxt_python_wsgi-python.o Link: <https://lwn.net/Articles/908817/> Link: <https://people.kernel.org/kees/bounded-flexible-arrays-in-c> Signed-off-by: Andrew Clayton <[email protected]>
nginx · May 7, 2024 · e2a09c7 · e2a09c7
1 parent 5b01bd6
commit e2a09c7
Show file tree

Hide file tree

Showing 4 changed files with 10 additions and 10 deletions.
diff --git a/src/nxt_http_route.c b/src/nxt_http_route.c
@@ -103,29 +103,29 @@ struct nxt_http_route_rule_s {
         } name;
     } u;
 
-    nxt_http_route_pattern_t       pattern[0];
+    nxt_http_route_pattern_t       pattern[];
 };
 
 
 typedef struct {
     uint32_t                       items;
-    nxt_http_route_rule_t          *rule[0];
+    nxt_http_route_rule_t          *rule[];
 } nxt_http_route_ruleset_t;
 
 
 typedef struct {
     /* The object must be the first field. */
     nxt_http_route_object_t        object:8;
     uint32_t                       items;
-    nxt_http_route_ruleset_t       *ruleset[0];
+    nxt_http_route_ruleset_t       *ruleset[];
 } nxt_http_route_table_t;
 
 
 struct nxt_http_route_addr_rule_s {
     /* The object must be the first field. */
     nxt_http_route_object_t        object:8;
     uint32_t                       items;
-    nxt_http_route_addr_pattern_t  addr_pattern[0];
+    nxt_http_route_addr_pattern_t  addr_pattern[];
 };
 
 
@@ -139,20 +139,20 @@ typedef union {
 typedef struct {
     uint32_t                       items;
     nxt_http_action_t              action;
-    nxt_http_route_test_t          test[0];
+    nxt_http_route_test_t          test[];
 } nxt_http_route_match_t;
 
 
 struct nxt_http_route_s {
     nxt_str_t                      name;
     uint32_t                       items;
-    nxt_http_route_match_t         *match[0];
+    nxt_http_route_match_t         *match[];
 };
 
 
 struct nxt_http_routes_s {
     uint32_t                       items;
-    nxt_http_route_t               *route[0];
+    nxt_http_route_t               *route[];
 };
 
 

diff --git a/src/nxt_upstream.h b/src/nxt_upstream.h
@@ -52,7 +52,7 @@ struct nxt_upstream_s {
 
 struct nxt_upstreams_s {
     uint32_t                                   items;
-    nxt_upstream_t                             upstream[0];
+    nxt_upstream_t                             upstream[];
 };
 
 

diff --git a/src/nxt_upstream_round_robin.c b/src/nxt_upstream_round_robin.c
@@ -23,7 +23,7 @@ struct nxt_upstream_round_robin_server_s {
 
 struct nxt_upstream_round_robin_s {
     uint32_t                           items;
-    nxt_upstream_round_robin_server_t  server[0];
+    nxt_upstream_round_robin_server_t  server[];
 };
 
 

diff --git a/src/python/nxt_python.h b/src/python/nxt_python.h
@@ -48,7 +48,7 @@ typedef struct {
 
 typedef struct {
     nxt_int_t            count;
-    nxt_python_target_t  target[0];
+    nxt_python_target_t  target[];
 } nxt_python_targets_t;