Bug 65345 - ICE with _Generic selection on _Atomic int
Summary: ICE with _Generic selection on _Atomic int
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c (show other bugs)
Version: 4.9.2
: P3 normal
Target Milestone: 5.5
Assignee: Marek Polacek
URL:
Keywords:
: 67848 67887 (view as bug list)
Depends on:
Blocks:
 
Reported: 2015-03-07 20:48 UTC by Tijl Coosemans
Modified: 2022-01-01 00:20 UTC (History)
3 users (show)

See Also:
Host:
Target: arm, aarch64, x86_64,i386, sparc, powerpc*
Build:
Known to work:
Known to fail:
Last reconfirmed: 2015-03-07 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Tijl Coosemans 2015-03-07 20:48:35 UTC
gcc49 (FreeBSD Ports Collection) 4.9.3 20150218 (prerelease)

% cat test.c
_Atomic int i;  
int j = _Generic( i+1, int: 1, default: 0 );

% gcc49 -c test.c
tmp.c:2:1: internal compiler error: Segmentation fault
 int j = _Generic( i+1, int: 1, default: 0 );
 ^
no stack trace because unwind library not available
Please submit a full bug report,
with preprocessed source if appropriate.
See <http://gcc.gnu.org/bugs.html> for instructions.


The error only happens when "i" is part of an expression, so _Generic(i,...) works correctly (it returns 1).  Qualifiers like const and volatile instead of _Atomic also work.
Comment 1 Marek Polacek 2015-03-07 20:59:50 UTC
Happens with trunk as well.  Taking.
Comment 2 Marek Polacek 2015-03-07 21:13:16 UTC
Backtrace:

0xcbc8aa crash_signal
	/home/marek/src/gcc/gcc/toplev.c:383
0x978b80 tree_check
	/home/marek/src/gcc/gcc/tree.h:2845
0x978b80 gimple_body(tree_node*)
	/home/marek/src/gcc/gcc/gimple-expr.c:328
0xa18d07 gimple_add_tmp_var(tree_node*)
	/home/marek/src/gcc/gcc/gimplify.c:721
0x97949a create_tmp_var(tree_node*, char const*)
	/home/marek/src/gcc/gcc/gimple-expr.c:522
0x68051f convert_lvalue_to_rvalue(unsigned int, c_expr, bool, bool)
	/home/marek/src/gcc/gcc/c/c-typeck.c:2042
0x6bd29a c_parser_binary_expression
	/home/marek/src/gcc/gcc/c/c-parser.c:6379
0x6bd7d8 c_parser_conditional_expression
	/home/marek/src/gcc/gcc/c/c-parser.c:6031
0x6bdd80 c_parser_expr_no_commas
	/home/marek/src/gcc/gcc/c/c-parser.c:5949
0x6b39e8 c_parser_generic_selection
	/home/marek/src/gcc/gcc/c/c-parser.c:6856
0x6b39e8 c_parser_postfix_expression
	/home/marek/src/gcc/gcc/c/c-parser.c:7665
0x6b5572 c_parser_unary_expression
	/home/marek/src/gcc/gcc/c/c-parser.c:6602
0x6bcaaa c_parser_cast_expression
	/home/marek/src/gcc/gcc/c/c-parser.c:6440
0x6bcc74 c_parser_binary_expression
	/home/marek/src/gcc/gcc/c/c-parser.c:6255
0x6bd7d8 c_parser_conditional_expression
	/home/marek/src/gcc/gcc/c/c-parser.c:6031
0x6bdd80 c_parser_expr_no_commas
	/home/marek/src/gcc/gcc/c/c-parser.c:5949
0x6c7c0a c_parser_initializer
	/home/marek/src/gcc/gcc/c/c-parser.c:4167
0x6cd107 c_parser_declaration_or_fndef
	/home/marek/src/gcc/gcc/c/c-parser.c:1824
0x6d51c7 c_parser_external_declaration
	/home/marek/src/gcc/gcc/c/c-parser.c:1452
0x6d5a49 c_parser_translation_unit
	/home/marek/src/gcc/gcc/c/c-parser.c:1339
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <http://gcc.gnu.org/bugs.html> for instructions.
Comment 3 Marek Polacek 2015-03-07 21:18:53 UTC
Seems to be unrelated to _Generic, the following ICEs as well:

_Atomic int i = 5;
int j = i;
Comment 4 joseph@codesourcery.com 2015-03-09 16:32:20 UTC
Or e.g.

_Atomic int i = 5;
int j = sizeof (i + 1);

which is valid code not involving _Generic.  And, similarly:

_Atomic int i = 5;
int j = sizeof (i = 0);

or

_Atomic int i = 5;
int j = sizeof (++i);

or

_Atomic int i = 5;
int j = sizeof (i--);

This at first suggests (but see below) that the special-case handling of 
atomics on lvalue-to-rvalue conversion, and on assignment / increment / 
decrement / any other cases where creation of temporaries is involved, 
should be disabled when at file scope - either the expression in question 
is in a context such as sizeof or _Generic where its side effects do not 
occur so the special handling is not needed, or an error will occur for 
the expression being non-constant even without the special handling.

Much the same applies at function prototype scope, e.g.:

_Atomic int i = 5;
void f (int a[i + 1]);

(where [i + 1] means the same as [*]).  But in the case of

_Atomic int i = 5;
void f (int a[i = 1]) {}

you have a valid program, where the atomic assignment must be executed on 
function entry (see the pending_sizes handling in c-decl.c).  So to handle 
such cases, maybe the special handling of atomics needs to be partly 
deferred, so that parsing "i = 1" generates some tree for atomic 
assignment and the temporaries only get added at gimplification time.
Comment 5 Marek Polacek 2015-03-09 16:50:26 UTC
(In reply to joseph@codesourcery.com from comment #4)
> Or e.g.
> 
> _Atomic int i = 5;
> int j = sizeof (i + 1);
> 
> which is valid code not involving _Generic.  And, similarly:
> 
> _Atomic int i = 5;
> int j = sizeof (i = 0);
> 
> or
> 
> _Atomic int i = 5;
> int j = sizeof (++i);
> 
> or
> 
> _Atomic int i = 5;
> int j = sizeof (i--);
> 
> This at first suggests (but see below) that the special-case handling of 
> atomics on lvalue-to-rvalue conversion, and on assignment / increment / 
> decrement / any other cases where creation of temporaries is involved, 
> should be disabled when at file scope - either the expression in question 
> is in a context such as sizeof or _Generic where its side effects do not 
> occur so the special handling is not needed, or an error will occur for 
> the expression being non-constant even without the special handling.

Yeah, I had an idea of using create_tmp_var_raw that doesn't push the variable into function's local vars vector.  My thinking was that we don't need such variables to survive into ME anyway in the valid cases, and in invalid cases, the FE just rejects the code...

--- gcc/c/c-typeck.c
+++ gcc/c/c-typeck.c
@@ -2039,7 +2039,12 @@ convert_lvalue_to_rvalue (location_t loc, struct c_expr exp,
       /* Remove the qualifiers for the rest of the expressions and 
         create the VAL temp variable to hold the RHS.  */  
       nonatomic_type = build_qualified_type (expr_type, TYPE_UNQUALIFIED);
-      tmp = create_tmp_var (nonatomic_type);
+      /* If we are outside a function, avoid pushing the variable into the
+        current binding.  */
+      if (current_function_decl)
+        tmp = create_tmp_var (nonatomic_type);
+      else
+        tmp = create_tmp_var_raw (nonatomic_type);
       tmp_addr = build_unary_op (loc, ADDR_EXPR, tmp, 0); 
       TREE_ADDRESSABLE (tmp) = 1;
       TREE_NO_WARNING (tmp) = 1;


> Much the same applies at function prototype scope, e.g.:
> 
> _Atomic int i = 5;
> void f (int a[i + 1]);
> 
> (where [i + 1] means the same as [*]).  But in the case of
> 
> _Atomic int i = 5;
> void f (int a[i = 1]) {}
> 
> you have a valid program, where the atomic assignment must be executed on 
> function entry (see the pending_sizes handling in c-decl.c).  So to handle 
> such cases, maybe the special handling of atomics needs to be partly 
> deferred, so that parsing "i = 1" generates some tree for atomic 
> assignment and the temporaries only get added at gimplification time.

... but my patch wouldn't handle this case.  Oh well.
Comment 6 Marek Polacek 2015-03-09 16:55:18 UTC
FWIW, my testcase was

/* PR c/65345 */
/* { dg-do compile } */
/* { dg-options "" } */

_Atomic int i = 3;
int g1 = sizeof (i + 1) + sizeof (-i);
int g2 = __builtin_constant_p (i + 1);
int g3 = 0 && i;
int g4 = 0 || i; /* { dg-error "initializer element is not constant" } */
int g5 = i; /* { dg-error "initializer element is not constant" } */
int g6[i]; /* { dg-error "variably modified" } */
int g7 = (i ? 1 : 2); /* { dg-error "initializer element is not constant" } */
int g8 = _Alignof (-i);
_Atomic long g10 = i; /* { dg-error "initializer element is not constant" } */

_Static_assert (_Generic (i, int: 1, default: 0) == 1, "");
_Static_assert (_Generic (i + 1, int: 1, default: 0) == 1, "");

void
foo (void)
{
  static int q1 = sizeof (i + 1) + sizeof (-i);
  static int q2 = __builtin_constant_p (i + 1);
  static int q3 = 0 && i;
  static int q4 = 0 || i; /* { dg-error "initializer element is not constant" } */
  static int q5 = i; /* { dg-error "initializer element is not constant" } */
  static int q6[i]; /* { dg-error "storage size" } */
  static int q7 = (i ? 1 : 2); /* { dg-error "initializer element is not constant" } */
  static int q8 = _Alignof (-i);
}
Comment 7 Marek Polacek 2015-03-17 20:28:58 UTC
Patch queued for next stage1:
https://gcc.gnu.org/ml/gcc-patches/2015-03/msg00698.html
Comment 8 Marek Polacek 2015-03-20 17:20:43 UTC
A note to myself: address the TARGET_ATOMIC_ASSIGN_EXPAND_FENV implementations:
<https://gcc.gnu.org/ml/gcc-patches/2015-03/msg01094.html>.
Comment 9 Jakub Jelinek 2015-04-22 12:00:28 UTC
GCC 5.1 has been released.
Comment 10 Marek Polacek 2015-04-23 14:35:44 UTC
Author: mpolacek
Date: Thu Apr 23 14:35:12 2015
New Revision: 222370

URL: https://gcc.gnu.org/viewcvs?rev=222370&root=gcc&view=rev
Log:
	PR c/65345
	* c-decl.c (set_labels_context_r): New function.
	(store_parm_decls): Call it via walk_tree_without_duplicates.
	* c-typeck.c (convert_lvalue_to_rvalue): Use create_tmp_var_raw
	instead of create_tmp_var.  Build TARGET_EXPR instead of
	COMPOUND_EXPR.
	(build_atomic_assign): Use create_tmp_var_raw instead of
	create_tmp_var.  Build TARGET_EXPRs instead of MODIFY_EXPR.

	* gcc.dg/pr65345-1.c: New test.
	* gcc.dg/pr65345-2.c: New test.

Added:
    trunk/gcc/testsuite/gcc.dg/pr65345-1.c
    trunk/gcc/testsuite/gcc.dg/pr65345-2.c
Modified:
    trunk/gcc/c/ChangeLog
    trunk/gcc/c/c-decl.c
    trunk/gcc/c/c-typeck.c
    trunk/gcc/testsuite/ChangeLog
Comment 11 Richard Biener 2015-07-16 09:14:13 UTC
GCC 5.2 is being released, adjusting target milestone to 5.3.
Comment 12 Joseph S. Myers 2015-09-30 15:54:24 UTC
Is there some reason the TARGET_ATOMIC_ASSIGN_EXPAND_FENV patch - https://gcc.gnu.org/ml/gcc-patches/2015-04/msg00682.html - hasn't been committed, minus the XFAILing and with a note to all affected target maintainers who may need to fix their ports as requested in https://gcc.gnu.org/ml/gcc-patches/2015-04/msg01792.html ?
Comment 13 Marek Polacek 2015-09-30 16:01:34 UTC
No, the only problem is that I sort of forgot about this one ;).  I'll get it done tomorrow.
Comment 14 Marek Polacek 2015-10-01 14:53:41 UTC
Author: mpolacek
Date: Thu Oct  1 14:53:10 2015
New Revision: 228343

URL: https://gcc.gnu.org/viewcvs?rev=228343&root=gcc&view=rev
Log:
	PR c/65345
	* config/i386/i386.c (ix86_atomic_assign_expand_fenv): Adjust to use
	create_tmp_var_raw rather than create_tmp_var.

	* gcc.dg/atomic/pr65345-4.c: New test.
	* gcc.dg/pr65345-3.c: New test.

Added:
    trunk/gcc/testsuite/gcc.dg/atomic/pr65345-4.c
    trunk/gcc/testsuite/gcc.dg/pr65345-3.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/i386/i386.c
    trunk/gcc/testsuite/ChangeLog
Comment 15 Marek Polacek 2015-10-01 14:54:36 UTC
Should be fixed, at least for x86_64/i?86.
Comment 16 Eric Botcazou 2015-10-06 09:15:20 UTC
Author: ebotcazou
Date: Tue Oct  6 09:14:48 2015
New Revision: 228516

URL: https://gcc.gnu.org/viewcvs?rev=228516&root=gcc&view=rev
Log:
	PR c/65345
	* config/sparc/sparc.c (sparc_atomic_assign_expand_fenv): Adjust to
	use create_tmp_var_raw rather than create_tmp_var.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/sparc/sparc.c
Comment 17 Ramana Radhakrishnan 2015-10-06 10:26:13 UTC
*** Bug 67848 has been marked as a duplicate of this bug. ***
Comment 18 Ramana Radhakrishnan 2015-10-06 11:13:48 UTC
This is not fixed till all the targets are fixed -

Also the target milestone is set at 5.3 but the fix hasn't been applied to the 5 branch.
Comment 19 David Edelsohn 2015-10-06 13:47:06 UTC
Author: dje
Date: Tue Oct  6 13:46:34 2015
New Revision: 228524

URL: https://gcc.gnu.org/viewcvs?rev=228524&root=gcc&view=rev
Log:
        PR c/65345
        * config/rs6000/rs6000.c (rs6000_atomic_assign_expand_fenv):
        Adjust to use create_tmp_var_raw instead of create_tmp_var.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/rs6000/rs6000.c
Comment 20 Ramana Radhakrishnan 2015-10-06 15:10:15 UTC
Author: ramana
Date: Tue Oct  6 15:09:43 2015
New Revision: 228526

URL: https://gcc.gnu.org/viewcvs?rev=228526&root=gcc&view=rev
Log:
Fix PR c/65345 for AArch64 



2015-10-06  Ramana Radhakrishnan  <ramana.radhakrishnan@arm.com>

	PR c/65345
	* config/aarch64/aarch64-builtins.c (aarch64_atomic_assign_expand_fenv):
	Use create_tmp_var_raw instead of create_tmp_var.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/aarch64/aarch64-builtins.c
Comment 21 uros 2015-10-06 15:31:43 UTC
Author: uros
Date: Tue Oct  6 15:31:11 2015
New Revision: 228527

URL: https://gcc.gnu.org/viewcvs?rev=228527&root=gcc&view=rev
Log:
	PR c/65345
	* config/alpha/alpha.c (alpha_atomic_assign_expand_fenv): Use
	create_tmp_var_raw instead of create_tmp_var.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/alpha/alpha.c
Comment 22 Bernd Edlinger 2015-10-07 06:08:21 UTC
I still have:

arm-linux-gnueabihf-gcc pr65345-3.c 
pr65345-3.c:8:1: internal compiler error: Segmentation fault
 float a3 = sizeof (i++);
 ^
0xaa349f crash_signal
	../../gcc-6-20151004.orig/gcc/toplev.c:353
0x8692c0 tree_check(tree_node*, char const*, int, char const*, tree_code)
	../../gcc-6-20151004.orig/gcc/tree.h:2863
0x8692c0 gimple_body(tree_node*)
	../../gcc-6-20151004.orig/gcc/gimple-expr.c:312
0x88b8e7 gimple_add_tmp_var(tree_node*)
	../../gcc-6-20151004.orig/gcc/gimplify.c:700
0x86999a create_tmp_var(tree_node*, char const*)
	../../gcc-6-20151004.orig/gcc/gimple-expr.c:467
0xdd0610 arm_atomic_assign_expand_fenv(tree_node**, tree_node**, tree_node**)
	../../gcc-6-20151004.orig/gcc/config/arm/arm-builtins.c:2977
0x5de84f build_atomic_assign
	../../gcc-6-20151004.orig/gcc/c/c-typeck.c:3738
0x5dfbf4 build_unary_op(unsigned int, tree_code, tree_node*, int)
	../../gcc-6-20151004.orig/gcc/c/c-typeck.c:4131
0x60300d c_parser_postfix_expression_after_primary
	../../gcc-6-20151004.orig/gcc/c/c-parser.c:8089
0x5fbcd7 c_parser_postfix_expression
	../../gcc-6-20151004.orig/gcc/c/c-parser.c:7806
0x5fe352 c_parser_unary_expression
	../../gcc-6-20151004.orig/gcc/c/c-parser.c:6693
0x5fefbf c_parser_cast_expression
	../../gcc-6-20151004.orig/gcc/c/c-parser.c:6531
0x5ff1a5 c_parser_binary_expression
	../../gcc-6-20151004.orig/gcc/c/c-parser.c:6347
0x5ffdf5 c_parser_conditional_expression
	../../gcc-6-20151004.orig/gcc/c/c-parser.c:6123
0x6003e0 c_parser_expr_no_commas
	../../gcc-6-20151004.orig/gcc/c/c-parser.c:6041
0x600ab2 c_parser_expression
	../../gcc-6-20151004.orig/gcc/c/c-parser.c:8113
0x5fbfc1 c_parser_postfix_expression
	../../gcc-6-20151004.orig/gcc/c/c-parser.c:7309
0x5fe352 c_parser_unary_expression
	../../gcc-6-20151004.orig/gcc/c/c-parser.c:6693
0x5fe8aa c_parser_sizeof_expression
	../../gcc-6-20151004.orig/gcc/c/c-parser.c:6743
0x5fe8aa c_parser_unary_expression
	../../gcc-6-20151004.orig/gcc/c/c-parser.c:6664
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <http://gcc.gnu.org/bugs.html> for instructions.
Comment 23 Ramana Radhakrishnan 2015-10-07 08:38:07 UTC
Author: ramana
Date: Wed Oct  7 08:37:35 2015
New Revision: 228562

URL: https://gcc.gnu.org/viewcvs?rev=228562&root=gcc&view=rev
Log:
Fix PR c/65345 for arm




2015-10-07  Ramana Radhakrishnan  <ramana.radhakrishnan@arm.com>

	PR c/65345
	* config/arm/arm-builtins.c (arm_atomic_assign_expand_fenv):
	Use create_tmp_var_raw instead of create_tmp_var.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/arm/arm-builtins.c
Comment 24 Andrew Pinski 2015-10-08 03:40:00 UTC
*** Bug 67887 has been marked as a duplicate of this bug. ***
Comment 25 Andreas Krebbel 2015-10-08 07:50:13 UTC
Author: krebbel
Date: Thu Oct  8 07:49:41 2015
New Revision: 228594

URL: https://gcc.gnu.org/viewcvs?rev=228594&root=gcc&view=rev
Log:
S/390: Use create_tmp_var_raw in s390_atomic_assign_expand_fenv.

gcc/ChangeLog:

2015-10-08  Andreas Krebbel  <krebbel@linux.vnet.ibm.com>

	PR c/65345
	* config/s390/s390.c (s390_atomic_assign_expand_fenv): Use
	create_tmp_var_raw instead of create_tmp_var.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/s390/s390.c
Comment 26 Richard Biener 2015-12-04 10:47:33 UTC
GCC 5.3 is being released, adjusting target milestone.
Comment 27 Richard Biener 2016-06-03 10:07:27 UTC
GCC 5.4 is being released, adjusting target milestone.
Comment 28 mpf 2016-08-09 12:36:50 UTC
Author: mpf
Date: Tue Aug  9 12:36:18 2016
New Revision: 239278

URL: https://gcc.gnu.org/viewcvs?rev=239278&root=gcc&view=rev
Log:
MIPS: Use create_tmp_var_raw in mips_atomic_assign_expand_fenv

gcc/
	PR c/65345
	* config/mips/mips.c (mips_atomic_assign_expand_fenv):
	Use create_tmp_var_raw instead of create_tmp_var.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/mips/mips.c
Comment 29 Steve Ellcey 2018-01-12 23:22:14 UTC
It looks like this has been fixed on all the different platforms
and my googling hasn't found any pr65345-[12].c failures in
gcc-testresults since 2016.  Should we close this out?
Comment 30 Marek Polacek 2018-01-15 09:41:09 UTC
Closing given Comment 29.