From e2a09c7742d2b74e3896ef99d3941ab1e46d2a15 Mon Sep 17 00:00:00 2001 From: Andrew Clayton Date: Thu, 13 Apr 2023 19:42:04 +0100 Subject: Convert 0-sized arrays to true flexible array members MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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: Link: Signed-off-by: Andrew Clayton --- src/nxt_http_route.c | 14 +++++++------- src/nxt_upstream.h | 2 +- src/nxt_upstream_round_robin.c | 2 +- src/python/nxt_python.h | 2 +- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/nxt_http_route.c b/src/nxt_http_route.c index 98ff5404..852f5739 100644 --- a/src/nxt_http_route.c +++ b/src/nxt_http_route.c @@ -103,13 +103,13 @@ 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; @@ -117,7 +117,7 @@ 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; @@ -125,7 +125,7 @@ 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 index afc53774..0c3b4c8b 100644 --- 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 index 1274f5a9..96c3e44e 100644 --- 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 index 37e6265e..f5154514 100644 --- 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; -- cgit