Subject: Bug in va_arg
To: None <port-mips@netbsd.org>
From: Uros Prestor <uros.prestor@nexsi.com>
List: port-cobalt
Date: 06/27/2001 20:27:42
This is a multi-part message in MIME format.
--------------0AAD1312F167ACA0525766E6
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit

Hello,

There is a problem with <stdarg.h> header file on MIPS platforms (I
tested my example on a Cobalt RaQ but I believe this discussion applies
to other MIPS platforms).

The gist of the problem is that va_arg() incorrectly adjusts the
argument pointer if you try to pass a structure whose size is larger
than an int and whose alignment is that of an int.  As currently
implemented, va_arg tests if the size of the type is larger than an int,
and if so, it adjusts the va_list to have it 8-byte aligned.  This blows
up in case where you're passing a struct which is aligned on an int
boundary.  (Yeah, I know, passing struct as an argument to a varargs
function is asking for trouble...  All I can say is I didn't write the
code ;)

The solution is to replace sizeof(T) with __alignof__(T) in the
definition of va_args.  This of course only applies to gcc and should be
#ifdef __GNUC__ 'd.  I don't have an solution for other compilers.

Please find attached a test program with a testcase and the proposed new
definition of va_args.  On my Cobalt RaQ, the output is:

     bug: bar = 0x89abcdef  baz = 0x7fffeff0
     fix: bar = 0x1234567  baz = 0x89abcdef
     dbl: dbl = 3.141592653589793

Thanks,
Uros

--
Uros Prestor
uros.prestor@nexsi.com



--------------0AAD1312F167ACA0525766E6
Content-Type: text/plain; charset=us-ascii;
 name="bug.c"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="bug.c"


#include <stdio.h>
#include <stdarg.h>

struct foo {
    int bar;
    int baz;
};

/*
 * Fixed va_arg
 */

#if BYTE_ORDER == LITTLE_ENDIAN
#define	VA_ARG(ap, T)							\
	(((T *)(							\
	    (ap) += (/*CONSTCOND*/ __alignof__(T) <= sizeof(int)	\
		? sizeof(int) : ((long)(ap) & 4) + sizeof(T)),		\
	    (ap) - (/*CONSTCOND*/ __alignof__(T) <= sizeof(int)		\
		? sizeof(int) : sizeof(T))				\
 	))[0])
#else
#define	VA_ARG(ap, T)							\
	(((T *)(							\
	    (ap) += (/*CONSTCOND*/ __alignof__(T) <= sizeof(int)	\
		? sizeof(int) : ((long)(ap) & 4) + sizeof(T))		\
 	))[-1])
#endif

void
bug(const char* fmt, ...)
{
    va_list ap;
    struct foo f;

    va_start(ap, fmt);
    f = va_arg(ap, struct foo);
    va_end(ap);

    printf(fmt, f.bar, f.baz);
}

void
fix(const char* fmt, ...)
{
    va_list ap;
    struct foo f;

    va_start(ap, fmt);
    f = VA_ARG(ap, struct foo);
    va_end(ap);

    printf(fmt, f.bar, f.baz);
}

void
dbl(const char* fmt, ...)
{
    va_list ap;
    double dbl;

    va_start(ap, fmt);
    dbl = VA_ARG(ap, double);
    va_end(ap);

    printf(fmt, dbl);
}

int
main(void)
{
    struct foo f;

    f.bar = 0x01234567;
    f.baz = 0x89abcdef;

    bug("bug: bar = %#x  baz = %#x\n", f);
    fix("fix: bar = %#x  baz = %#x\n", f);
    dbl("dbl: dbl = %.15f\n", 3.141592653589793);

    return 0;
}

--------------0AAD1312F167ACA0525766E6--