Bug 31488 - [4.3/4.4 Regression] va_list considered non-POD
Summary: [4.3/4.4 Regression] va_list considered non-POD
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 4.3.0
: P4 normal
Target Milestone: 4.3.3
Assignee: Jason Merrill
URL:
Keywords: wrong-code
Depends on:
Blocks: 33263 38664
  Show dependency treegraph
 
Reported: 2007-04-05 19:44 UTC by Richard Henderson
Modified: 2009-01-15 22:54 UTC (History)
4 users (show)

See Also:
Host:
Target: alpha*
Build:
Known to work:
Known to fail:
Last reconfirmed: 2009-01-05 20:01:57


Attachments
Fix in pod_type_p (324 bytes, patch)
2009-01-07 20:59 UTC, Jason Merrill
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Richard Henderson 2007-04-05 19:44:06 UTC
.../libjava/classpath/include/jvmti.h:1242: warning: can not pass objects
of non-POD type 'struct va_list' through '...'; call will abort at runtime

Not sure yet whether this is a target problem or not.
Comment 1 Andrew Pinski 2007-04-05 20:28:48 UTC
Most likely this is because va_list is represented as an array for alpha and a couple other targets.  So maybe it might be just the warning is incorrect but it is still undefined.
Comment 2 Richard Henderson 2007-04-05 21:45:04 UTC
It is *not* represented as an array on Alpha.

And even if it were, it wouldn't be undefined.  Passing va_list by
value is non-portable, but certainly not undefined.
Comment 3 Uroš Bizjak 2008-12-29 20:39:29 UTC
The testcase:

#include <stdarg.h>

extern int foo (int a, int b, ...);

int bar (int a, int b, ...)
{
  va_list args;
  va_start (args, b);
  int result = foo (a, b, args);
  va_end (args);
  return result;
}

g++ -O2:

pod.C: In function ‘int bar(int, int, ...)’:
pod.C:9: warning: cannot pass objects of non-POD type ‘struct va_list’ through ‘...’; call will abort at runtime
Comment 4 Uroš Bizjak 2008-12-29 20:42:59 UTC
Preprocessed source, can be compiled with a crosscompiler:

--cut here--
# 1 "pod.C"
# 1 "<built-in>"
# 1 "<command-line>"
# 1 "pod.C"
# 1 "/usr/lib/gcc/alpha-linux-gnu/4.2.4/include/stdarg.h" 1 3 4
# 43 "/usr/lib/gcc/alpha-linux-gnu/4.2.4/include/stdarg.h" 3 4
typedef __builtin_va_list __gnuc_va_list;
# 105 "/usr/lib/gcc/alpha-linux-gnu/4.2.4/include/stdarg.h" 3 4
typedef __gnuc_va_list va_list;
# 2 "pod.C" 2

extern int foo (int a, int b, ...);

int bar (int a, int b, ...)
{
  va_list args;
    __builtin_va_start(args,b);
      int result = foo (a, b, args);
        __builtin_va_end(args);
   return result;
   }
--cut here--

Comment 5 Andrew Pinski 2008-12-29 20:46:53 UTC
sounds like the C++ front-end does not know this struct is a POD.  I am going to mark this as a front-end bug.
Comment 6 Uroš Bizjak 2008-12-29 20:57:27 UTC
(In reply to comment #2)
> It is *not* represented as an array on Alpha.

But as a RECORD_TYPE. Following patch cures the warning:

Index: cp/tree.c
===================================================================
--- cp/tree.c	(revision 142948)
+++ cp/tree.c	(working copy)
@@ -2113,6 +2113,9 @@ pod_type_p (const_tree t)
      argument unmodified and we assign it to a const_tree.  */
   t = strip_array_types (CONST_CAST_TREE(t));
 
+  if (TREE_CODE (t) == RECORD_TYPE)
+    return 1;
+
   if (t == error_mark_node)
     return 1;
   if (INTEGRAL_TYPE_P (t))
Comment 7 Andrew Pinski 2008-12-29 21:02:43 UTC
(In reply to comment #6)
> (In reply to comment #2)
> > It is *not* represented as an array on Alpha.
> 
> But as a RECORD_TYPE. Following patch cures the warning:

But that is not correct as some nonPODs are structs too.  In fact structs containing other nonPODs are still nonPODs.   Structs that have non trivial constructors are also nonPODs.

So the patch is in fact incorrect.

  if (! CLASS_TYPE_P (t))
    return 0; /* other non-class type (reference or function) */
  if (CLASSTYPE_NON_POD_P (t))
    return 0;
  return 1; 

One of those two should be set correctly.
Comment 8 Uroš Bizjak 2008-12-29 23:29:52 UTC
(In reply to comment #7)

>   if (! CLASS_TYPE_P (t))
>     return 0; /* other non-class type (reference or function) */
>   if (CLASSTYPE_NON_POD_P (t))
>     return 0;
>   return 1; 
> 
> One of those two should be set correctly.

In alpha/alpha.c we have:

alpha_build_builtin_va_list (void)
{
  tree base, ofs, space, record, type_decl;

  if (TARGET_ABI_OPEN_VMS || TARGET_ABI_UNICOSMK)
    return ptr_type_node;

  record = (*lang_hooks.types.make_type) (RECORD_TYPE);
  type_decl = build_decl (TYPE_DECL, get_identifier ("__va_list_tag"), record);
  TREE_CHAIN (record) = type_decl;
  TYPE_NAME (record) = type_decl;

  /* C++? SET_IS_AGGR_TYPE (record, 1); */

  ...

So, actually there is no way we can SET_CLASS_TYPE_P (t, 1) on the created RECORD_TYPE. IOW, we should somehow be able to reach cp/lex.c/make_class_type () function from target .c file.
Comment 9 Uroš Bizjak 2008-12-30 14:45:56 UTC
Patch at http://gcc.gnu.org/ml/gcc-patches/2008-12/msg01280.html
Comment 10 Uroš Bizjak 2008-12-31 17:02:06 UTC
Marking as a regression, testsuite failures are always regressions.
Comment 11 Jason Merrill 2009-01-07 20:59:25 UTC
Created attachment 17051 [details]
Fix in pod_type_p

Uros is testing this patch for me.
Comment 12 Uroš Bizjak 2009-01-08 07:04:15 UTC
(In reply to comment #11)
> Created an attachment (id=17051) [edit]
> Fix in pod_type_p
> 
> Uros is testing this patch for me.

This patch bootstraps OK and fixes following libjava FAILs on alphaev56-unknown-linux-gnu:

-FAIL: natgetargssize.cc compilation
-FAIL: natgetlocalvartable.cc compilation
-FAIL: natgetstacktrace.cc compilation
-FAIL: natevents.cc compilation
-FAIL: natgetallthreads.cc compilation
-FAIL: natgeterrorname.cc compilation
-FAIL: natgetmethodname.cc compilation

The patch was also bootstrapped and regression tested on x86_64-pc-linux-gnu {,-m32} without any problems.

BTW: Cleaned testcase can be found at [1].

[1] http://gcc.gnu.org/ml/gcc-patches/2008-12/msg01280.html
Comment 13 Jason Merrill 2009-01-12 21:07:59 UTC
Subject: Bug 31488

Author: jason
Date: Mon Jan 12 21:07:46 2009
New Revision: 143308

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=143308
Log:
        PR c++/31488
        * tree.c (pod_type_p): Return 1 for structs created by the back end.

Added:
    trunk/gcc/testsuite/g++.dg/other/vararg-3.C
Modified:
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/tree.c
    trunk/gcc/testsuite/ChangeLog

Comment 14 Jason Merrill 2009-01-15 22:34:38 UTC
Subject: Bug 31488

Author: jason
Date: Thu Jan 15 22:34:20 2009
New Revision: 143413

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=143413
Log:
        PR c++/36334
        PR c++/37646
        * tree.c (lvalue_p_1): Handle BASELINK.  A COMPONENT_REF to
        a function isn't necessarily an lvalue. Take tree, not const_tree.
        (lvalue_p, real_lvalue_p): Take tree, not const_tree.
        * typeck.c (lvalue_or_else): Likewise.
        * cp-tree.h: Adjust prototypes.

        PR c++/31488
        * tree.c (pod_type_p): Return 1 for structs created by the back end.

Added:
    branches/gcc-4_3-branch/gcc/testsuite/g++.dg/conversion/memfn1.C
      - copied unchanged from r143404, trunk/gcc/testsuite/g++.dg/conversion/memfn1.C
    branches/gcc-4_3-branch/gcc/testsuite/g++.dg/conversion/memfn2.C
      - copied unchanged from r143404, trunk/gcc/testsuite/g++.dg/conversion/memfn2.C
    branches/gcc-4_3-branch/gcc/testsuite/g++.dg/other/vararg-3.C
      - copied unchanged from r143308, trunk/gcc/testsuite/g++.dg/other/vararg-3.C
Modified:
    branches/gcc-4_3-branch/gcc/cp/ChangeLog
    branches/gcc-4_3-branch/gcc/cp/cp-tree.h
    branches/gcc-4_3-branch/gcc/cp/tree.c
    branches/gcc-4_3-branch/gcc/cp/typeck.c
    branches/gcc-4_3-branch/gcc/testsuite/ChangeLog

Comment 15 Jason Merrill 2009-01-15 22:54:51 UTC
Fixed.