[PATCH 0/5] add checking of function array parameters (PR 50584)

[Martin Sebor]

The C99 T[static N] form of declaring function arguments means
the argument must point to an object large enough to fit at least
N elements of T.  Diagnosing calls where the argument is smaller
helps detect buffer overflow.  Similarly, so does diagnosing
accesses by such functions to parameters beyond the N'th element.

Declaring function arguments as VLAs with variable bounds given
by other arguments to the same function (besides other expressions)
is another opportunity to validate accesses.

These standard features are similar to GCC 10 attribute access,
except that in GCC 10 they are independent and there's no checking
for out of bounds accesses.

Although the standard C requirements on [static N] arrays and VLA
arguments are exceedingly loose, letting programs mix and match 
different forms in distinct declarations of the same function with
impunity (or at least without any requirement to diagnose it),
doing so results in mismatches of constraints the different forms
express(*).  For instance, the following are considered compatible
declarations of the same functions in C, even though they obviously
express different requirements:

   void f (int[static 3);
   void f (int[static 7);

   void g (int nelts, int,       int[nelts]);
   void g (int,       int count, int[count]);

To both enable the detection of out-of-bounds accesses involving
these arguments and help reconcile the conflicting constraints
of redeclarations like those above, the attached patch integrates
arrays and VLA function arguments with attribute access.  It has
these effects:

1) For the first declaration of a function that specifies either
    one or more array with constant bounds or VLAs as arguments it
    adds an implicit attribute access specification describing all
    such arguments.
2) Function redeclarations are checked mismtatches in the form of
    their array/VLA/pointer arguments and their bounds and warnings
    are issued when any are found. (-Warray-parameter for ordinary
    arrays and -Wvla-paramater for VLAs).
3) Declarations that mix explicit attribute access with VLAs are
    checked for compatibility and warnings are again issued for
    mismatches.
4) Enables the same checking as for the explicit attribute access
    also for functions taking arrays or VLAs.
5) In the definitions of functions declared with attribute access
    it checks for accesses to their arguments and issues
    -Warray-bounds warnings when they're out of bounds.

C++ has no [static] or VLA arguments, so the initial implementation
only adds support to the C front end for this.  To provide the same
checking for ordinary arrays I plan to extend the -Warray-parameter
option to the C++ front end as well as a follow up enhancement.

Besides bootstrapping/regtesting GCC I built a bunch of packages
to validate the changes.  Some "interesting" results are below.

Martin

Binutils (one instance of -Warray-bounds):
   https://sourceware.org/bugzilla/show_bug.cgi?id=26246

Glibc: one unique instance of -Warray-parameter due to a mismatch.
I considered relaxing the warning to accommodate the T[1] vs T*
difference since it's innocuous in this case.  I decided against
it because it would prevent expressing the pervasive requirement
that a function expects a pointer to an array of exactly one object.
The warning is internal to Glibc and can be easily fixed by making
the declarations consistent.

../sysdeps/nptl/pthread.h:734:47: warning: argument 1 of type ‘struct 
__jmp_buf_tag*’ declared as a pointer  [-Warray-parameter]
../setjmp/setjmp.h:54:46: note: previously declared as an array ‘struct 
__jmp_buf_tag[1]’
    54 | extern int __sigsetjmp (struct __jmp_buf_tag __env[1], int 
__savemask) __THROWNL;

Kernel (3 instances of -Warray-parameter).  This is an inconsitent
use of the idiom to use the array notation to express the requirement
that a function accesses the given number of elements.

lib/crypto/poly1305-donna64.c:15:67: warning: argument 2 of type ‘const 
u8 {aka const unsigned char}[16]’ with mismatched size [-Warray-parameter]
    15 | void poly1305_core_setkey(struct poly1305_core_key *key, const 
u8 raw_key[16])
./include/crypto/internal/poly1305.h:21:68: note: previously declared as 
‘const u8 {aka const unsigned char}*’
    21 | void poly1305_core_setkey(struct poly1305_core_key *key, const 
u8 *raw_key);

arch/x86/lib/msr-smp.c:256:51: warning: argument 2 of type ‘u32 {aka 
unsigned int}*’ declared as a pointer [-Warray-parameter]
   256./arch/x86/include/asm/msr.h:349:50: note: previously declared as 
an array ‘u32 {aka unsigned int}[8]’
   349 | int rdmsr_safe_regs_on_cpu(unsigned int cpu, u32 regs[8]);
  | int rdmsr_safe_regs_on_cpu(unsigned int cpu, u32 *regs)


[*] More details are in n2074 that WG14 discussed in 2016:
     http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2074.htm
The sentiment at the time was that proposals like N2074 would
be considered for C2X.  I still expect to submit an updated
formal proposal into an upcoming WG14 mailing.