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_upstream.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/nxt_upstream.h') 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[]; }; -- cgit