nginx / unit

NGINX Unit - universal web app server - a lightweight and versatile open source server that simplifies the application stack by natively executing application code across eight different programming language runtimes.
https://unit.nginx.org
Apache License 2.0
5.37k stars 323 forks source link

Convert 0-sized arrays to true flexible array members #1240

Closed ac000 closed 4 months ago

ac000 commented 5 months ago

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
ac000 commented 4 months ago

Rebased with master