Bug 64867

From https://gcc.gnu.org/gcc-4.5/changes.html:
Diagnostics that used to complain about passing non-POD types to ... or jumping past the declaration of a non-POD variable now check for triviality rather than PODness, as per C++0x.


See PR 37907.

And having an example would be useful too.  Because of the change between C++98 and C++11.

Jason informs me it's now a warning enabled by -Wconditionally-supported

Passing a non-POD to a varargs function is conditionally-supported, with implementation-defined semantics.  In GCC 5 it's supported and treated like normal pass-by-value.  You can get a diagnostic about it with -Wconditionally-supported.

Thanks, I'll give it a try.

Here's my test case FWIW and a short demo showing what clang does:

pokyo. cat q.cc
#include <stdarg.h>

class ConstUTF8CharsZ
{
    const char *mData;

  public:
    ConstUTF8CharsZ() : mData(0)
    {
    }

    explicit ConstUTF8CharsZ(const char *aBytes)
      : mData(aBytes)
    {
    }

    const void *get() const { return mData; }

    const char *c_str() const { return mData; }

    operator bool() const { return mData != 0; }
};


void zzz(const char *x, ...) {
}

void m(const char *m) {
  ConstUTF8CharsZ cu(m);
  zzz(m, cu);
}
pokyo. clang++ q.cc
q.cc:30:10: error: cannot pass object of non-POD type 'ConstUTF8CharsZ' through
      variadic function; call will abort at runtime [-Wnon-pod-varargs]
  zzz(m, cu);
         ^
1 error generated.

(In reply to Jason Merrill from comment #3)
> Passing a non-POD to a varargs function is conditionally-supported, with
> implementation-defined semantics.  In GCC 5 it's supported and treated like
> normal pass-by-value.  You can get a diagnostic about it with
> -Wconditionally-supported.

I tried this today and it does not work with the test case in comment #4.

pokyo. gcc --version
gcc (GCC) 5.0.0 20150129 (experimental)
Copyright (C) 2015 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

pokyo. g++ --std=c++11 --syntax-only -Wall -Wconditionally-supported q.cc
pokyo.

Also, -Wconditionally-supported triggers other warning as well.
It would be more convenient if different warnings were individually
controllable; and then something like -Wconditionally-supported
would act as if I'd specified all the sub-options.

(In reply to Tom Tromey from comment #5)
> (In reply to Jason Merrill from comment #3)
> > Passing a non-POD to a varargs function is conditionally-supported, with
> > implementation-defined semantics.  In GCC 5 it's supported and treated like
> > normal pass-by-value.  You can get a diagnostic about it with
> > -Wconditionally-supported.
> 
> I tried this today and it does not work with the test case in comment #4.
> 
> pokyo. gcc --version
> gcc (GCC) 5.0.0 20150129 (experimental)
> Copyright (C) 2015 Free Software Foundation, Inc.
> This is free software; see the source for copying conditions.  There is NO
> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
> 
> pokyo. g++ --std=c++11 --syntax-only -Wall -Wconditionally-supported q.cc
> pokyo.

Are you sure that class is not trivial which is why gcc is not warning about it?  C++11 does not really have pod and non-pod any more but rather trivial and non-trivial and the rules has chnaged with respect of constructures.

See http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2342.htm . It does look like gcc is implementing the correct new pod rules and clang is not.

> Are you sure that class is not trivial which is why gcc is not warning about
> it?  C++11 does not really have pod and non-pod any more but rather trivial
> and non-trivial and the rules has chnaged with respect of constructures.

I hesitate to say I'm sure :)

However my belief is that because this class has a user-provided
default constructor, it is not trivial.

I tested this by adding "#include <type_traits>" and then

static_assert(!std::is_trivial<ConstUTF8CharsZ>::value, "whoops");


Trivial or not, I'd like to get a warning here.  In firefox varargs
is really only used for printf-like things; but for various reasons
there are some custom implementations which can't be marked
with the "printf" attribute.  So while doing a refactoring it turned
out I inadvertently changed code to pass objects rather than char*
to one of these ... but finding the problem locations is proving
to be rather difficult.

FYI, g++ stopped warning on #c4 with r149721.

(In reply to Tom Tromey from comment #9)
> However my belief is that because this class has a user-provided
> default constructor, it is not trivial.

True, but ...
 
> I tested this by adding "#include <type_traits>" and then
> 
> static_assert(!std::is_trivial<ConstUTF8CharsZ>::value, "whoops");

That's the wrong thing to assert:

  Passing a potentially-evaluated argument of class type (Clause 9)
  having a non-trivial copy constructor, a non-trivial move constructor,
  or a non-trivial destructor, with no corresponding parameter, is
  conditionally-supported with implementation-defined semantics.

Your type has a trivial copy constructor and trivial move constructor and trivial destructor, so passing it to a varargs function is well-defined.

Give it a non-trivial destructor and you get a warning:

cs.cc: In function ‘void m(const char*)’:
cs.cc:33:12: warning: passing objects of non-trivially-copyable type ‘class ConstUTF8CharsZ’ through ‘...’ is conditionally supported [-Wconditionally-supported]
   zzz(m, cu);
            ^

N.B. trivially-copyable is not the same as trivial (and there are separate type traits for testing those two properties).

(In reply to Jonathan Wakely from comment #11)

> That's the wrong thing to assert:

Aha, thank you very much.  I obviously did not realize the difference :)

Unfortunately I think even if I made this change I probably wouldn't
want to enable -Wconditionally-supported here, as it enables some
other warning that IIRC isn't desirable here.

I ended up writing a gcc plugin to find places passing an aggregate
to a varargs function.  But I still think some simpler way to get
a warning here would be nice.

FWIW Clang has this warning via -Wnon-pod-varargs.

*** Bug 67559 has been marked as a duplicate of this bug. ***

(In reply to John Steele Scott from comment #14)
> FWIW Clang has this warning via -Wnon-pod-varargs.

Confirmed, gcc having this warning would help avoid: http://gcc.gnu.org/ml/gcc-patches/2017-09/msg00126.html

This adds -Wnon-pod-varargs, enabled by -Wconditionally-supported, allowing e.g.
-Wconditionally-supported -Werror=non-pod-varargs

diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 3435fe90cca..b6ac29cda64 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -714,6 +714,10 @@ Wnamespaces
 C++ ObjC++ Var(warn_namespaces) Warning
 Warn on namespace definition.
 
+Wnon-pod-varargs
+C++ ObjC++ Var(warn_non_pod_varargs) Warning LangEnabledBy(C++ ObjC++,Wconditionally-supported)
+Warn if passing a non-POD object through the \"...\" of a varargs function.
+
 Wpacked-not-aligned
 C ObjC C++ ObjC++ Var(warn_packed_not_aligned) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall)
 Warn when fields in a struct with the packed attribute are misaligned.
diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 9e4a5c1b9ae..fc9f74f5968 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -7164,10 +7164,12 @@ convert_arg_to_ellipsis (tree arg, tsubst_flags_t complain)
              || TYPE_HAS_NONTRIVIAL_DESTRUCTOR (arg_type)))
        {
          if (complain & tf_warning)
-           warning (OPT_Wconditionally_supported,
-                    "passing objects of non-trivially-copyable "
-                    "type %q#T through %<...%> is conditionally supported",
-                    arg_type);
+           {
+             warning (OPT_Wnon_pod_varargs,
+                      "passing objects of non-trivially-copyable "
+                      "type %q#T through %<...%> is conditionally supported",
+                      arg_type);
+           }
          return cp_build_addr_expr (arg, complain);
        }
     }

(In reply to Eric Gallager from comment #16)
> (In reply to John Steele Scott from comment #14)
> > FWIW Clang has this warning via -Wnon-pod-varargs.
> 
> Confirmed, gcc having this warning would help avoid:
> http://gcc.gnu.org/ml/gcc-patches/2017-09/msg00126.html

Yeah.  The situation is similar to the code in comment #4,
which as Jakub says in comment #10 no longer produces a warning
with g++ -Wconditionally-supported.  I had to add a dummy
destructor to get one.

Whether Clang warns or not depends on the -std mode active, as that decides the definition of a POD it uses. GCC always uses the C++11 rule, which I quoted in comment 9.

(In reply to Jonathan Wakely from comment #17)
> This adds -Wnon-pod-varargs, enabled by -Wconditionally-supported, allowing
> e.g.
> -Wconditionally-supported -Werror=non-pod-varargs
> 
> diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
> index 3435fe90cca..b6ac29cda64 100644
> --- a/gcc/c-family/c.opt
> +++ b/gcc/c-family/c.opt
> @@ -714,6 +714,10 @@ Wnamespaces
>  C++ ObjC++ Var(warn_namespaces) Warning
>  Warn on namespace definition.
>  
> +Wnon-pod-varargs
> +C++ ObjC++ Var(warn_non_pod_varargs) Warning LangEnabledBy(C++
> ObjC++,Wconditionally-supported)
> +Warn if passing a non-POD object through the \"...\" of a varargs function.
> +
>  Wpacked-not-aligned
>  C ObjC C++ ObjC++ Var(warn_packed_not_aligned) Warning LangEnabledBy(C ObjC
> C++ ObjC++,Wall)
>  Warn when fields in a struct with the packed attribute are misaligned.
> diff --git a/gcc/cp/call.c b/gcc/cp/call.c
> index 9e4a5c1b9ae..fc9f74f5968 100644
> --- a/gcc/cp/call.c
> +++ b/gcc/cp/call.c
> @@ -7164,10 +7164,12 @@ convert_arg_to_ellipsis (tree arg, tsubst_flags_t
> complain)
>               || TYPE_HAS_NONTRIVIAL_DESTRUCTOR (arg_type)))
>         {
>           if (complain & tf_warning)
> -           warning (OPT_Wconditionally_supported,
> -                    "passing objects of non-trivially-copyable "
> -                    "type %q#T through %<...%> is conditionally supported",
> -                    arg_type);
> +           {
> +             warning (OPT_Wnon_pod_varargs,
> +                      "passing objects of non-trivially-copyable "
> +                      "type %q#T through %<...%> is conditionally
> supported",
> +                      arg_type);
> +           }
>           return cp_build_addr_expr (arg, complain);
>         }
>      }

Has this patch been posted to the gcc-patches mailing list yet?

No, because it doesn't have any tests.

It should probably adjust:

../gcc/testsuite/g++.dg/overload/ellipsis1.C
../gcc/testsuite/g++.dg/overload/ellipsis2.C
../gcc/testsuite/g++.old-deja/g++.pt/vaarg3.C

to use the new flag, and add a test that the new flag is enabled by -Wconditionally-supported

(In reply to Jonathan Wakely from comment #21)
> No, because it doesn't have any tests.
> 
> It should probably adjust:
> 
> ../gcc/testsuite/g++.dg/overload/ellipsis1.C
> ../gcc/testsuite/g++.dg/overload/ellipsis2.C
> ../gcc/testsuite/g++.old-deja/g++.pt/vaarg3.C
> 
> to use the new flag, and add a test that the new flag is enabled by
> -Wconditionally-supported

OK, I think I can do this.

(In reply to Eric Gallager from comment #22)
> (In reply to Jonathan Wakely from comment #21)
> > No, because it doesn't have any tests.
> > 
> > It should probably adjust:
> > 
> > ../gcc/testsuite/g++.dg/overload/ellipsis1.C
> > ../gcc/testsuite/g++.dg/overload/ellipsis2.C
> > ../gcc/testsuite/g++.old-deja/g++.pt/vaarg3.C
> > 
> > to use the new flag, and add a test that the new flag is enabled by
> > -Wconditionally-supported
> 
> OK, I think I can do this.

Actually never mind; I can't test because I'm blocked from bootstrapping by bug 83284; unassigning...

*** Bug 86562 has been marked as a duplicate of this bug. ***

retitling

Martin Sebor has been doing stuff related to warnings about POD-ness lately; cc-ing him

This should really be enabled by -Wall or -Wextra as it generates code that may easily crash or corrupt something.
Clang even turns it into an error by default and inserts a trap to abort at runtime: https://godbolt.org/z/8DAUAr

(In reply to Trass3r from comment #27)
> This should really be enabled by -Wall or -Wextra as it generates code that
> may easily crash or corrupt something.

But GCC supports this fully.  That is why it is/was part of -Wconditionally-supported .  The code if used correctly does not cause crashes or corrupts anything. 

In fact GCC documents this behavior:
https://gcc.gnu.org/onlinedocs/gcc-9.2.0/gcc/Conditionally-supported-behavior.html#Conditionally-supported-behavior
:)

> Clang even turns it into an error by default and inserts a trap to abort at
> runtime: https://godbolt.org/z/8DAUAr
But clang does not support this.  Both compilers are correct; in that this feature is considered as conditionally supported with implementation defined behavior.  So both compilers are prefectly ok in having their own choices.

Ah I see, ok.

(In reply to Trass3r from comment #27)
> This should really be enabled by -Wall or -Wextra as it generates code that
> may easily crash or corrupt something.

+1.

Clang errors, intel warns and for gcc you have to enable the somewhat
obscure flag -Wconditionally-supported to see anything at all.

gcc looks to be a trailer on this issue. It's not about standards conformance,
its about making it easy for users to find bugs.

Source code I used is:

struct S
{
	S();
	S( const S &);
	~S();

	char * p;
};

void f( int, ...);

void g()
{
	S s1;

	f( 3, s1);
};

$ /home/dcb/llvm/results/bin/clang++   sep3f.cc
sep3f.cc:19:8: error: cannot pass object of non-trivial type 'S' through variadic function; call will abort at runtime [-Wnon-pod-varargs]
        f( 3, s1);
              ^
1 error generated.

$ /home/dcb34/intel/oneapi/compiler/2021.3.0/linux/bin/intel64/icpc  sep3f.cc
sep3f.cc(19): warning #1595: a class type that is not trivially copyable passed through ellipsis
  	f( 3, s1);
  	      ^

$ /home/dcb/gcc/results/bin/g++ -c -g -O2 -Wall -Wextra  sep3f.cc
$ /home/dcb/gcc/results/bin/g++ -c -g -O2 -Wall -Wextra -Wconditionally-supported sep3f.cc
sep3f.cc: In function ‘void g()’:
sep3f.cc:19:10: warning: passing objects of non-trivially-copyable type ‘struct S’ through ‘...’ is conditionally supported [-Wconditionally-supported]
   19 |         f( 3, s1);
      |         ~^~~~~~~~


I'll add flag -Wconditionally-supported to a build of Fedora and see
what happens.

(In reply to David Binderman from comment #30)
> gcc looks to be a trailer on this issue. It's not about standards conformance,
> its about making it easy for users to find bugs.

But under GCC it's not a bug, it just works.  Clang is welcome to adopt the same semantics.  But I'm also happy to accept the patch in comment #17 once it includes testing.

And GCC should build with -Wconditionally-supported so we support bootstrapping with compilers that make different choices.

The master branch has been updated by Jason Merrill <jason@gcc.gnu.org>:

https://gcc.gnu.org/g:263c22a95bc9a0d80c4873c0291b0f938cea7310

commit r13-4795-g263c22a95bc9a0d80c4873c0291b0f938cea7310
Author: Jason Merrill <jason@redhat.com>
Date:   Mon Dec 5 10:00:31 2022 -0500

    build: add -Wconditionally-supported to strict_warn [PR64867]
    
    The PR (which isn't resolved by this commit) pointed out to me that GCC
    should build with -Wconditionally-supported to support bootstrapping with a
    C++11 compiler that makes different choices.
    
            PR c++/64867
    
    gcc/ChangeLog:
    
            * configure.ac (strict_warn): Add -Wconditionally-supported.
            * configure: Regenerate.

*** Bug 118435 has been marked as a duplicate of this bug. ***