NetBSD Problem Report #45138

From ws@netbsd.org  Tue Jul 12 12:46:17 2011
Return-Path: <ws@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [204.152.190.11])
	by www.NetBSD.org (Postfix) with ESMTP id 200A963BAC6
	for <gnats-bugs@gnats.NetBSD.org>; Tue, 12 Jul 2011 12:46:17 +0000 (UTC)
Message-Id: <20110712124617.1091514A142@mail.netbsd.org>
Date: Tue, 12 Jul 2011 12:46:17 +0000 (UTC)
From: ws@NetBSD.org
Reply-To: ws@NetBSD.org
To: gnats-bugs@gnats.NetBSD.org
Subject: stdarg.h: Deficiency in va_arg handling of array arguments.
X-Send-Pr-Version: 3.95

>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:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue Jul 12 12:50:00 +0000 2011
>Closed-Date:    Sun Nov 13 18:40:28 +0000 2011
>Last-Modified:  Sun Nov 13 18:40:28 +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:/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);
 }

>Release-Note:

>Audit-Trail:

State-Changed-From-To: open->feedback
State-Changed-By: dholland@NetBSD.org
State-Changed-When: Tue, 04 Oct 2011 07:21:54 +0000
State-Changed-Why:
Does gcc 4.5 need this patch? If so, please send it upstream...


From: Wolfgang Solfrank <Wolfgang@Solfrank.net>
To: gnats-bugs@NetBSD.org
Cc: dholland@NetBSD.org
Subject: Re: toolchain/45138 (stdarg.h: Deficiency in va_arg handling of array
 arguments.)
Date: Tue, 04 Oct 2011 16:05:17 +0200

 Hi,

 > Does gcc 4.5 need this patch? If so, please send it upstream...

 Yes, gcc 4.5 still has the problem.

 I did already submit a PR to gcc bugzilla (see
 http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50581), however,
 it seems that they don't want to change gcc in this direction,
 as behaviour when passing an array type to va_arg is obviously
 undefined according to the standard.  They might add a
 warning/error if array types are passed to va_arg.

 My real problem with this is that this prohibits portable code
 from passing some va_list recursively, as va_list is defines as
 a one element array on some architectures.

 Ciao,
 Wolfgang
 -- 
 Wolfgang@Solfrank.net				Wolfgang Solfrank

From: David Holland <dholland-bugs@netbsd.org>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: toolchain/45138 (stdarg.h: Deficiency in va_arg handling of
 array arguments.)
Date: Sat, 5 Nov 2011 16:21:50 +0000

 On Tue, Oct 04, 2011 at 02:10:06PM +0000, Wolfgang Solfrank wrote:
  >  > Does gcc 4.5 need this patch? If so, please send it upstream...
  >  
  >  Yes, gcc 4.5 still has the problem.
  >  
  >  I did already submit a PR to gcc bugzilla (see
  >  http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50581), however,
  >  it seems that they don't want to change gcc in this direction,
  >  as behaviour when passing an array type to va_arg is obviously
  >  undefined according to the standard.  They might add a
  >  warning/error if array types are passed to va_arg.
  >  
  >  My real problem with this is that this prohibits portable code
  >  from passing some va_list recursively, as va_list is defines as
  >  a one element array on some architectures.

 Unfortunately, if they don't want it, we probably shouldn't diverge...

 -- 
 David A. Holland
 dholland@netbsd.org

State-Changed-From-To: feedback->closed
State-Changed-By: dholland@NetBSD.org
State-Changed-When: Sun, 13 Nov 2011 18:40:28 +0000
State-Changed-Why:
conclusion is that we shouldn't diverge from upstream on this.


>Unformatted:

Home	PR Database Search