[Bug c/50581] New: stdarg doesn't support array types

[Bug c/50581] New: stdarg doesn't support array types

[Wolfgang at Solfrank dot net]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50581

             Bug #: 50581
           Summary: stdarg doesn't support array types
    Classification: Unclassified
           Product: gcc
           Version: 4.5.3
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: Wolfgang@Solfrank.net


__builtin_va_arg doesn't work correctly if given an array type, as it doesn't
take the promotion of array to pointer into account.  When called without some
-std=xxx argument, gcc complains with the message "invalid use of non-lvalue
array".  If it is called with e.g. -std=c99, it doesn't complain, but generates
incorrect code nevertheless.

The problem can be reproduced by the following program:
-------------------------------------------
typedef char array[10];
array arr;

void
f(char *fmt, ...)
{
        __builtin_va_list ap;

        __builtin_va_start(ap, fmt);
        __builtin_memcpy(arr, __builtin_va_arg(ap, array), sizeof arr);
        __builtin_va_end(ap);
}
-------------------------------------------
Fixing this is especially useful for types the user doesn't even know whether
it is an array or not, like e.g. __builtin_va_list itself to allow for
recursive variadic argument passing:
-------------------------------------------
void
f(char *fmt, ...)
{
        __builtin_va_list ap, ap1;

        __builtin_va_start(ap, fmt);
        __builtin_va_copy(ap1, __builtin_va_arg(ap, __builtin_va_list));
        __builtin_va_end(ap);
        __builtin_va_copy(ap, ap1);
        __builtin_va_end(ap1);
        __builtin_va_end(ap);
}
-------------------------------------------
The fix is quite easy.  Just apply the following diff to gcc/c-common.c:
-------------------------------------------
Index: gcc/c-common.c
===================================================================
RCS file: /cvsroot/src/external/gpl3/gcc/dist/gcc/c-common.c,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 c-common.c
--- gcc/c-common.c      21 Jun 2011 01:20:03 -0000      1.1.1.1
+++ gcc/c-common.c      30 Sep 2011 15:12:23 -0000
@@ -5124,6 +5124,14 @@
 tree
 build_va_arg (location_t loc, 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));
+
   expr = build1 (VA_ARG_EXPR, type, expr);
   SET_EXPR_LOCATION (expr, loc);
   return expr;
-------------------------------------------

[Bug c/50581] stdarg doesn't support array types

[joseph at codesourcery dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50581

--- Comment #1 from joseph at codesourcery dot com <joseph at codesourcery dot com> 2011-09-30 19:41:17 UTC ---
There is no possible valid use of passing arrays to va_arg.

In C99, it is never possible for an array to be passed by value to a 
function because it will have decayed to a pointer before the call.  
Thus, any execution of va_arg with such an argument type results in 
undefined behavior, just like calling it with "char" or "float", and 
generating a runtime abort (with a compile-time warning) might be 
appropriate, as is done for "char" and "float".

In C90, it is technically possible to use va_arg in this case without 
undefined behavior.  The argument passed to the function would have to be 
a non-lvalue array - for example, an array in a structure returned from 
another function.  The result of va_arg would itself be a non-lvalue 
array, which it is not possible to convert to a pointer, so it is not 
possible to access the values in the array in any way; all that can be 
done is to discard the value (call va_arg for its side effects) or to pass 
it to another variadic function.

As far as I know this obscure C90-only use case works correctly (so you 
can access arguments after the non-lvalue array using va_arg again, for 
example).  If you have problems with it, please provide a bug report with 
a valid C90 testcase.

E.g.,

#include <stdarg.h>

typedef char array[1];
struct s { array a; };
struct s f (void);
void g (int a, ...);

void
h (void)
{
  g (0, f().a);
}

void
g (int a, ...)
{
  va_list ap;
  va_start (ap, a);
  va_arg (ap, array);
  va_end (ap);
}

[Bug c/50581] stdarg doesn't support array types

[Wolfgang at Solfrank dot net]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50581

--- Comment #2 from Wolfgang at Solfrank dot net 2011-10-01 11:06:48 UTC ---
(In reply to comment #1)

Sorry, I don't quite follow you:

> There is no possible valid use of passing arrays to va_arg.

What makes you think so?  While the 1003.1 definition of va_arg explicitly
specifies that the behaviour when passing a type to va_arg that "is not
compatible with the type of the actual next argument (as promoted according to
the default argument promotions)" is undefined, that doesn't preclude to
implement something reasonable, does it?

> In C90, it is technically possible to use va_arg in this case without 
> undefined behavior.  The argument passed to the function would have to be 
> a non-lvalue array - for example, an array in a structure returned from 
> another function.  The result of va_arg would itself be a non-lvalue 
> array, which it is not possible to convert to a pointer, so it is not 
> possible to access the values in the array in any way; all that can be 
> done is to discard the value (call va_arg for its side effects) or to pass 
> it to another variadic function.

Well, I'm not sure that I buy that.  But even then, the current implementation
in gcc doesn't generate the correct code even only for the side effects.  The
generated code in fact assumes that the array is passed by value, i.e. the
pointer into the argument list (or something equivalent) is incremented by the
size of the array instead of the size of a pointer.

My main use case for this feature isn't with random arrays, but with va_list
itself.  On some architectures (AFAIK all architectures that pass some
arguments in registers) gcc implements va_list as a one element array of some
structure.  Without my proposed change, it isn't possible to have a va_list as
an argument to a variadic function.  This is what my second example in the bug
report was about.  To show it in a somewhat more real live example, let's
assume that vprintf implemented such a feature, something like (using %^ to
mean the next two arguments are a format string and a va_list):
-------------------------------------------
#include <stdarg.h>

int
vprintf(char *fmt, va_list ap)
{
        char *fmt1;
        va_list ap1;

        while (*fmt)
                if (*fmt == '%')
                        switch (*++fmt) {
                        case '^':
                                fmt1 = va_arg(ap, char *);
                                va_copy(ap1, va_arg(ap, va_list));
                                vprintf(fmt1, ap1);
                                va_end(ap1);
                                break;
....
                        }
...
}
-------------------------------------------
With current gcc, code like this works as expected on processors that implement
va_list as a pointer into the parameter list passed on the stack, but doesn't
work for processors passing arguments in registers therefor implement va_list
as thje above mentioned one element array.

Even if I buy your arguments, I consider it a bug that while with -std=c89 the
compiler spits an error message, but with -std=c99 it silently generates
incorrect code.

[Bug c/50581] stdarg doesn't support array types

[joseph at codesourcery dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50581

--- Comment #3 from joseph at codesourcery dot com <joseph at codesourcery dot com> 2011-10-01 14:07:42 UTC ---
On Sat, 1 Oct 2011, Wolfgang at Solfrank dot net wrote:

> > There is no possible valid use of passing arrays to va_arg.
> 
> What makes you think so?  While the 1003.1 definition of va_arg explicitly

That the C semantics mean that:

* for C90 it is impossible to access values in the non-lvalue array passed 
(the existence of the possibility of passing an array by value to a 
variadic function in C90 is one of the most obscure and useless C 
standards corner cases there is);

* for C99, calling va_arg with such a type always results in undefined 
behavior at runtime because a pointer will have been passed rather than an 
array.

So no valid C program can ever actually access a value in this way.  Of 
course, if you tried to pass an *lvalue* array (the normal case for arrays 
in C) then it was converted to a pointer rather than passed by value, for 
both C90 and C99, and so you had undefined behavior at runtime, for both 
C90 and C99.

If you want to access a pointer, you need to pass a pointer type to 
va_arg.  This is just like accessing a promoted float or char where you 
need to specify "double" or "int".  Which is why generating an abort with 
a warning at compile time seems appropriate - it's the well-established 
practice for "char" and "float" here.  (Given the obscurity, there should 
probably be a warning for C90 as well, but without the abort.)

> > In C90, it is technically possible to use va_arg in this case without 
> > undefined behavior.  The argument passed to the function would have to be 
> > a non-lvalue array - for example, an array in a structure returned from 
> > another function.  The result of va_arg would itself be a non-lvalue 
> > array, which it is not possible to convert to a pointer, so it is not 
> > possible to access the values in the array in any way; all that can be 
> > done is to discard the value (call va_arg for its side effects) or to pass 
> > it to another variadic function.
> 
> Well, I'm not sure that I buy that.  But even then, the current implementation
> in gcc doesn't generate the correct code even only for the side effects.  The
> generated code in fact assumes that the array is passed by value, i.e. the
> pointer into the argument list (or something equivalent) is incremented by the
> size of the array instead of the size of a pointer.

Here is an example program that is valid as C90 but not as C99.  It passes 
for me with all the C90 options I tried.  That is, the caller and callee 
are consistent about the space expected to be taken by the array on the 
stack, which is all that's required since there is no way of accessing the 
array's value.  (C ABIs won't generally specify this for interoperation 
between implementations, given that passing by value an array whose value 
you can't access isn't useful and the possibility of doing so has 
disappeared in C99.)

#include <stdarg.h>

extern void abort (void);
extern void exit (int);

typedef char array[10000];
struct s { array a; } x;
void g (int a, ...);

int
main (void)
{
  g (0, (0, x).a, 1234);
  exit (0);
}

void
g (int a, ...)
{
  va_list ap;
  va_start (ap, a);
  va_arg (ap, array);
  if (va_arg (ap, int) != 1234)
    abort ();
  va_end (ap);
}

> My main use case for this feature isn't with random arrays, but with va_list
> itself.  On some architectures (AFAIK all architectures that pass some
> arguments in registers) gcc implements va_list as a one element array of some
> structure.  Without my proposed change, it isn't possible to have a va_list as
> an argument to a variadic function.  This is what my second example in the bug

Passing va_list as a function argument is generally hard, whether or not 
variadic, since you don't know whether it will be passed by reference or 
by value or what the type of the address of a va_list parameter will be.  
Portable code needs to pass a pointer to va_list or a structure containing 
va_list or use some other such means of avoiding dependence on whether 
va_list is an array.

[Bug c/50581] stdarg doesn't support array types

[Wolfgang at Solfrank dot net]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50581

--- Comment #4 from Wolfgang at Solfrank dot net 2011-10-01 14:43:32 UTC ---
(In reply to comment #3)

Thanks for the insight regarding passing arrays by value in C90, I wasn't even
aware of that.

However:

> Passing va_list as a function argument is generally hard, whether or not 
> variadic, since you don't know whether it will be passed by reference or 
> by value or what the type of the address of a va_list parameter will be.  
> Portable code needs to pass a pointer to va_list or a structure containing 
> va_list or use some other such means of avoiding dependence on whether 
> va_list is an array.

Huh?  What about vprintf and friends?  They are defined to take a va_list as
their last parameter.

[Bug c/50581] stdarg doesn't support array types

[joseph at codesourcery dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50581

--- Comment #5 from joseph at codesourcery dot com <joseph at codesourcery dot com> 2011-10-01 18:45:43 UTC ---
On Sat, 1 Oct 2011, Wolfgang at Solfrank dot net wrote:

> > Passing va_list as a function argument is generally hard, whether or not 
> > variadic, since you don't know whether it will be passed by reference or 
> > by value or what the type of the address of a va_list parameter will be.  
> > Portable code needs to pass a pointer to va_list or a structure containing 
> > va_list or use some other such means of avoiding dependence on whether 
> > va_list is an array.
> 
> Huh?  What about vprintf and friends?  They are defined to take a va_list as
> their last parameter.

There are some things you can do - for example, calling those functions in 
accordance with the rules given in the C standard.  There are various 
things that cause problems - for example, taking the address of a 
parameter declared as a va_list (because the parameter type may have been 
changed from va_list to pointer-to-element-of-va_list as part of the 
parameter type adjustment of parameters declared as arrays, so the type of 
&parameter may not be va_list *).

[Bug c/50581] stdarg doesn't support array types

[Wolfgang at Solfrank dot net]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50581

--- Comment #6 from Wolfgang at Solfrank dot net 2011-10-02 16:08:51 UTC ---
(In reply to comment #5)
> On Sat, 1 Oct 2011, Wolfgang at Solfrank dot net wrote:
> 
> > > Passing va_list as a function argument is generally hard, whether or not 
> > > variadic, since you don't know whether it will be passed by reference or 
> > > by value or what the type of the address of a va_list parameter will be.  
> > > Portable code needs to pass a pointer to va_list or a structure containing 
> > > va_list or use some other such means of avoiding dependence on whether 
> > > va_list is an array.
> > 
> > Huh?  What about vprintf and friends?  They are defined to take a va_list as
> > their last parameter.
> 
> There are some things you can do - for example, calling those functions in 
> accordance with the rules given in the C standard.  There are various 
> things that cause problems - for example, taking the address of a 
> parameter declared as a va_list (because the parameter type may have been 
> changed from va_list to pointer-to-element-of-va_list as part of the 
> parameter type adjustment of parameters declared as arrays, so the type of 
> &parameter may not be va_list *).

But I don't want to take the address of a va_list parameter.  I just want to
handle it with stuff defined in stdarg.h.

Actually, I'm not sure why va_list is defined as an array in some of the
architectures.  The problem wouldn't arise if va_list were just defined as the
structure that it's currently defined as an array of.

Anyway, I still consider it a bug that gcc when called with -std=c99 compiles
va_arg with array type without any hint into code that cannot possibly be
useful, as it expects the array to be passed by value to the variadic function.

[Bug c/50581] stdarg doesn't support array types

[svfuerst at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50581

Steven Fuerst <svfuerst at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |svfuerst at gmail dot com

--- Comment #7 from Steven Fuerst <svfuerst at gmail dot com> 2011-10-02 19:47:46 UTC ---
> Actually, I'm not sure why va_list is defined as an array in some of the
architectures.

The underlying issue is that the ABI on some architectures requires it.  They
may use "register windows" instead of stacks to pass parameters to functions. 
i.e. r1-r5 might refer to the registers in the function that called you. 
r6-r10 might refer to the registers in the function that called the function
that called you. etc.  The act of calling a function then changes the values a
set of registers refers to.  So when you call a function, what it sees as
r6-r10 are the values that you see in r1-r5.

With this type of ABI, you access your parameters from i.e. r1-r5.  Any
functions that you call cannot do this though... as the registers are renamed. 
So va_list cannot be a pointer type as there is nothing to point to.

So how do you implement va_args for these arches then?  The trick is to use an
array.  C requires that an array decays to a pointer when it is passed as an
argument.  This means that the array must be stored on the stack, and not in
the renamable registers.  This allows the code to get a handle on how many
stack (and register) frames up the calling sequence it needs to go to find the
parameters.  The array can then store information about i.e. how many integer
and floating point registers have been read from the variable argument list.

[Bug c/50581] stdarg doesn't support array types

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=50581

--- Comment #8 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
I am hearing there is a C23 proposal to that might fix this.