Bug 53220

gcc-compiled code for reference:

(gdb) disas main
Dump of assembler code for function main:
   0x0000000000400540 <+0>:     push   %rbp
   0x0000000000400541 <+1>:     mov    $0x1,%esi
   0x0000000000400546 <+6>:     mov    $0x2,%ebp
   0x000000000040054b <+11>:    push   %rbx
   0x000000000040054c <+12>:    sub    $0x18,%rsp
   0x0000000000400550 <+16>:    movl   $0x1,(%rsp)           <<===
   0x0000000000400557 <+23>:    movl   $0x2,0x4(%rsp)        <<===
   0x000000000040055f <+31>:    mov    %rsp,%rbx
   0x0000000000400562 <+34>:    movl   $0x3,0x8(%rsp)        <<===
   0x000000000040056a <+42>:    movl   $0x0,0xc(%rsp)        <<===
   0x0000000000400572 <+50>:    jmp    0x40057d <main+61>
   0x0000000000400574 <+52>:    nopl   0x0(%rax)
   0x0000000000400578 <+56>:    mov    %ebp,%esi
   0x000000000040057a <+58>:    mov    0x4(%rbx),%ebp
   0x000000000040057d <+61>:    xor    %eax,%eax
   0x000000000040057f <+63>:    mov    $0x40068c,%edi
   0x0000000000400584 <+68>:    add    $0x4,%rbx
   0x0000000000400588 <+72>:    callq  0x4003b8 <printf@plt>
   0x000000000040058d <+77>:    test   %ebp,%ebp
   0x000000000040058f <+79>:    jne    0x400578 <main+56>
   0x0000000000400591 <+81>:    add    $0x18,%rsp
   0x0000000000400595 <+85>:    xor    %eax,%eax
   0x0000000000400597 <+87>:    pop    %rbx
   0x0000000000400598 <+88>:    pop    %rbp
   0x0000000000400599 <+89>:    retq   
End of assembler dump.

It is caused by revision 181332:

http://gcc.gnu.org/ml/gcc-cvs/2011-11/msg00623.html

What is the right scope for the compound literals? Is the clobber correctly added or the program is ill formed?

David

Compound literals are not part of C++, so correctness is a matter of debate.  In C, a compound literal designates an object with automatic storage duration.  In G++, a compound literal designates a temporary object, just like a normal cast or function-like cast.

This is a significant difference in semantics, which leads to the problem encountered here; the temporary object goes out of scope immediately after the initialization of p, so the loop has undefined behavior.

It would be possible for G++ to model the C semantics more closely.

So it is possible either 
1) to keep the current G++ semantics of compound literals, but change its behavior due to the implementation change (with clobber marker); 
or
2) to change hte G++ semantics to match C semantic, but keep the compiler behavior the same

Which way to go? If we go for 1), we probably just need to document this behavior better in GCC, and let user change their code.

David

(In reply to comment #5)

> 1) to keep the current G++ semantics of compound literals, but change its
> behavior due to the implementation change (with clobber marker); 

I would argue that 1 is completely useless for "you can also construct an
array" use case from http://gcc.gnu.org/onlinedocs/gcc/Compound-Literals.html

It always initializes the pointer with dangling storage, and is always a bug.

If "keep the current g++ semantics", then the code should be rejected at
compile time, and should *not* work when built without optimization.

IMO, having this code working in C and not working in C++ is a lousy choice.

Yes, the array case should be warned or disallowed if 1 is the way to go.

I won't call it a lousy choice -- the C++ semantics of the compound literals allow more agressive optimization and smaller stack usage.

David



(In reply to comment #6)
> (In reply to comment #5)
> 
> > 1) to keep the current G++ semantics of compound literals, but change its
> > behavior due to the implementation change (with clobber marker); 
> 
> I would argue that 1 is completely useless for "you can also construct an
> array" use case from http://gcc.gnu.org/onlinedocs/gcc/Compound-Literals.html
> 
> It always initializes the pointer with dangling storage, and is always a bug.
> 
> If "keep the current g++ semantics", then the code should be rejected at
> compile time, and should *not* work when built without optimization.
> 
> IMO, having this code working in C and not working in C++ is a lousy choice.

The thing is, C++11 introduces list-initialized temporaries; I could rewrite the testcase in C++11 as

extern "C" int printf (const char *, ...);

int main()
{
  typedef int AR[4];
  for (int *p = AR{1,2,3,0}; *p; ++p)
    {
      printf ("%d\n", *p);
    }
  return 0;
}

so it made sense to me for compound literals to have the same semantics; otherwise you have a difference in lifetime based on whether or not the type is wrapped in parentheses.

I definitely agree that we need to give a diagnostic about taking the address of a temporary here.

c++11 defines the lifetime of a temporary -- does it match C or g++'s semantics of compound literals or neither?

Note that without your change, the original program may also be subject to runtime failures due to escaped life ranges of the scoped variables leading to bad stack layout -- though such bugs are more subtle and less likely to be triggered.

David


(In reply to comment #8)
> The thing is, C++11 introduces list-initialized temporaries; I could rewrite
> the testcase in C++11 as
> 
> extern "C" int printf (const char *, ...);
> 
> int main()
> {
>   typedef int AR[4];
>   for (int *p = AR{1,2,3,0}; *p; ++p)
>     {
>       printf ("%d\n", *p);
>     }
>   return 0;
> }
> 
> so it made sense to me for compound literals to have the same semantics;
> otherwise you have a difference in lifetime based on whether or not the type is
> wrapped in parentheses.
> 
> I definitely agree that we need to give a diagnostic about taking the address
> of a temporary here.

(In reply to comment #9)
> c++11 defines the lifetime of a temporary -- does it match C or g++'s semantics
> of compound literals or neither?

C++98 and C++11 define the lifetime of a temporary as lasting until the end of the full-expression, unless its lifetime is extended by binding it to a reference.  G++ treats compound literals as temporaries like any other.

(In reply to comment #10)

> C++98 and C++11 define the lifetime of a temporary as lasting until the end of
> the full-expression, unless its lifetime is extended by binding it to a
> reference.  G++ treats compound literals as temporaries like any other.

So, as far as I understand:

1. the example with literal arrays in the documentation is invalid in C++,
2. I should fix C++ code that uses them
3. all that remains in this bug is to issue diagnostics for them

I checked the behavior of clang and icc with the following testcase:

extern "C" int printf (const char *,...);

struct A
{
  int i;
  ~A() { printf ("~A()\n"); }
};
   
int main()
{
  for (A *p = (A[]){ 1, 2, 3, 0}; p->i; ++p) {
    printf("%d\n", p->i);
  }
  return 0;
}

Both compilers match G++ behavior and destroy all the elements of the array before the loop begins; the only difference is that recent G++ is more aggressive about optimizing based on the knowledge that the array is no longer alive.

So yes, what remains for this bug is to complain about undefined behavior due to accessing the value of an object after its lifetime has ended.

(In reply to comment #12)

> So yes, what remains for this bug is to complain about undefined behavior due
> to accessing the value of an object after its lifetime has ended.

The documentation here:
  http://gcc.gnu.org/onlinedocs/gcc/Compound-Literals.html
sould probably be updated to mention that the "You can also construct an
array" part only works in C, and leads to undefined behavior in C++.

(In reply to comment #12)

> So yes, what remains for this bug is to complain about undefined behavior due
> to accessing the value of an object after its lifetime has ended.

The documentation here:
  http://gcc.gnu.org/onlinedocs/gcc/Compound-Literals.html
should probably be updated to mention that the "You can also construct an
array" part only works in C, and leads to undefined behavior in C++.

Author: jason
Date: Sat May 26 21:13:23 2012
New Revision: 187916

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=187916
Log:
	PR c++/53220
gcc/
	* c-typeck.c (array_to_pointer_conversion): Give -Wc++-compat warning
	about array compound literals.
gcc/cp/
	* call.c (convert_like_real) [ck_list]: Take array address directly.
	* typeck.c (decay_conversion): Reject decay of an array compound
	literal.

Added:
    trunk/gcc/testsuite/c-c++-common/array-lit.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/c-typeck.c
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/call.c
    trunk/gcc/cp/typeck.c
    trunk/gcc/doc/extend.texi
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/g++.dg/ext/complit12.C

Author: jason
Date: Wed May 30 14:51:54 2012
New Revision: 188020

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=188020
Log:
	PR c++/53220
gcc/
	* c-typeck.c (array_to_pointer_conversion): Give -Wc++-compat warning
	about array compound literals.
gcc/cp/
	* call.c (convert_like_real) [ck_list]: Take array address directly.
	* typeck.c (decay_conversion): Reject decay of an array compound
	literal.

Added:
    branches/gcc-4_7-branch/gcc/testsuite/c-c++-common/array-lit.c
Modified:
    branches/gcc-4_7-branch/gcc/ChangeLog
    branches/gcc-4_7-branch/gcc/c-typeck.c
    branches/gcc-4_7-branch/gcc/cp/ChangeLog
    branches/gcc-4_7-branch/gcc/cp/call.c
    branches/gcc-4_7-branch/gcc/cp/typeck.c
    branches/gcc-4_7-branch/gcc/doc/extend.texi
    branches/gcc-4_7-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_7-branch/gcc/testsuite/g++.dg/ext/complit12.C

Fixed by making the affected code ill-formed.

FWIW, it appears that the new error is too strict. It rejects this source, which (AFAIU) is entirely kosher:

#include <stdio.h>

void fn(int arr[])
{
  for (int j = 0; j < 5; ++j)
    printf("%d: %d\n", j, arr[j]);
}

int main()
{
  fn((int[]) { 41, 42, 43, 44, 45 } );
  return 0;
}

g++ -c t.cc
t.cc: In function ‘int main()’:
t.cc:11:37: error: taking address of temporary array
   fn((int[]) { 41, 42, 43, 44, 45 } );
                                     ^

Still, I prefer to rewrite the case above, then to debug undiagnosed runtime problems from the original test case.

I agree, but once the array decays to a pointer it's hard to tell whether it's used in a safe or unsafe way.

(In reply to comment #18)

> #include <stdio.h>
> 
> void fn(int arr[])
> {
>   for (int j = 0; j < 5; ++j)
>     printf("%d: %d\n", j, arr[j]);
> }
> 
> int main()
> {
>   fn((int[]) { 41, 42, 43, 44, 45 } );
>   return 0;
> }
> 
> g++ -c t.cc
> t.cc: In function ‘int main()’:
> t.cc:11:37: error: taking address of temporary array
>    fn((int[]) { 41, 42, 43, 44, 45 } );


I wrote some code which compiled perfectly using gcc 4.7. However, now I am using gcc 4.8 and I am encountering exactly the problem mentioned by Paul. Will this be solved or should I look for another solution to overcome this problem? Any suggestions?

(In reply to comment #20)
> I wrote some code which compiled perfectly using gcc 4.7. However, now I am
> using gcc 4.8 and I am encountering exactly the problem mentioned by Paul. Will
> this be solved or should I look for another solution to overcome this problem?
> Any suggestions?

You should probably adjust your code to avoid using array compound literals in C++.

The added diagnostic rejects this, which clang and EDG accept:

extern "C" int printf(const char*, ...);
int main() { 
  using A = int[1];
  printf("%p\n", A{1} );
}

ts.c:4:18: error: taking address of temporary array
   printf("%p\n", A{1} );
                  ^~~~


This doesn't use compound literals, and seems like valid C++ according to [conv.array].

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

https://gcc.gnu.org/g:305a2686c99bf9b57490419f25c79f6fb3ae0feb

commit r14-5941-g305a2686c99bf9b57490419f25c79f6fb3ae0feb
Author: Jason Merrill <jason@redhat.com>
Date:   Tue Nov 28 13:54:47 2023 -0500

    c++: prvalue array decay [PR94264]
    
    My change for PR53220 made array to pointer decay for prvalue arrays
    ill-formed to catch well-defined C code that produces a dangling pointer in
    C++ due to the shorter lifetime of compound literals.  This wasn't really
    correct, but wasn't a problem until C++17 added prvalue arrays, at which
    point it started rejecting valid C++ code.
    
    I wanted to make sure that we still diagnose the problematic code;
    -Wdangling-pointer covers the array-lit.c case, but I needed to extend
    -Wreturn-local-addr to handle the return case.
    
            PR c++/94264
            PR c++/53220
    
    gcc/c/ChangeLog:
    
            * c-typeck.cc (array_to_pointer_conversion): Adjust -Wc++-compat
            diagnostic.
    
    gcc/cp/ChangeLog:
    
            * call.cc (convert_like_internal): Remove obsolete comment.
            * typeck.cc (decay_conversion): Allow array prvalue.
            (maybe_warn_about_returning_address_of_local): Check
            for returning pointer to temporary.
    
    gcc/testsuite/ChangeLog:
    
            * c-c++-common/array-lit.c: Adjust.
            * g++.dg/cpp1z/array-prvalue1.C: New test.
            * g++.dg/ext/complit17.C: New test.