toolchain/45138: stdarg.h: Deficiency in va_arg handling of array arguments.

[ws%NetBSD.org@localhost]

>Number:         45138
>Category:       toolchain
>Synopsis:       stdarg.h: Deficiency in va_arg handling of array arguments.
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    toolchain-manager
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue Jul 12 12:50:00 +0000 2011
>Originator:     Wolfgang Solfrank
>Release:        NetBSD 5.1_STABLE (also in -current)
>Organization:
        NetBSD Foundation
>Environment:
System: NetBSD homeworld.netbsd.org 5.1_STABLE NetBSD 5.1_STABLE (NBMAIL) #0: 
Sat Apr 9 20:09:27 UTC 2011 
root%franklin.NetBSD.org@localhost:/home/netbsd/5/amd64/kern-compile/NBMAIL 
amd64
Architecture: x86_64
Machine: amd64
>Description:
va_arg doesn't take into account the conversion of array (and function)
arguments to pointers.  Therefor it's not possible to handle those
arguments correctly within a variadic function variable.

Arguably, this should be fixed for function type arguments, too, however,
since there is no such thing as a function varibale (only function pointer
variables), this is only of theoretical use (or is it?)
>How-To-Repeat:
The following examples all produce an error message of "invalid use of
non-lvalue array" if compiled with standard gcc args.  If you however
provide the argument -std=c99 to gcc, the error message doesn't show up.
However, the resulting program doesn't work as expected.  (Under some
circumstances it may seem to work, see last example below.)

This code snippet should be allowed and handled as naturally expected:
---------------------------------------------------------
#include <stdarg.h>
#include <string.h>

typedef int field[8][8];

extern void g(field);

void
f(const char *fmt, ...)
{
        va_list ap;
        field fld;

        va_start(ap, fmt);

        while (*fmt)
                switch (*fmt++) {
                case 'F':
                        memcpy(&fld, va_arg(ap, field), sizeof fld);
                        break;
                }
        va_end(ap);
        g(fld);
}
---------------------------------------------------------

Fixing this will also allow for recursive varargs handling on
processors that pass (part of) the parameters in registers and
typically define their va_list type as a one-element array of
structures (like e.g x86_64 or powerpc).

E.g. the following code should (more or less) output the
infamous hello world string:
---------------------------------------------------------
#include <stdarg.h>
#include <stdio.h>
#include <string.h>

typedef char arr[20];

char *
f(const char *fmt, ...)
{
        va_list ap;
        static arr x;

        va_start(ap, fmt);
        memcpy(&x, va_arg(ap, arr), sizeof x);
        va_end(ap);
        return x;
}

int
main(void)
{
        char *cp;
        arr hello = "hello, all in world!";
        arr bye =   "good bye till then!!";
        int i;

        cp = f("", hello);

        for (i = 0; i < sizeof(arr); i++, cp++)
                if (*cp >= ' ' && *cp <= '~')
                        putchar(*cp);
                else
                        printf("\\%03o", *cp & 0x7f);
        putchar('\n');
        return 0;
}
---------------------------------------------------------
Actually, if compiled on x86_64 with -O2 -std=c99, it does output the
expected "hello, all in world!".  However, if you drop the -O2, it
outputs the other string "good bye till then!!".

>Fix:
Index: gnu/dist/gcc4/gcc/c-common.c
===================================================================
RCS file: /cvsroot/src/gnu/dist/gcc4/gcc/c-common.c,v
retrieving revision 1.2
diff -u -r1.2 c-common.c
--- gnu/dist/gcc4/gcc/c-common.c        8 May 2011 01:49:32 -0000       1.2
+++ gnu/dist/gcc4/gcc/c-common.c        12 Jul 2011 12:41:19 -0000
@@ -3427,6 +3427,14 @@
 tree
 build_va_arg (tree expr, tree type)
 {
+  /*
+   * Since arrays are converted to pointers when given as function arguments,
+   * here we have to do the same with the type of the argument.
+   * XXX Does it make sense to do the same with function type arguments?
+   */
+  if (TREE_CODE(type) == ARRAY_TYPE)
+    type = build_pointer_type(strip_array_types(type));
+
   return build1 (VA_ARG_EXPR, type, expr);
 }