From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out1.suse.de (smtp-out1.suse.de [IPv6:2a07:de40:b251:101:10:150:64:1]) by sourceware.org (Postfix) with ESMTPS id C471B3858D32 for ; Mon, 15 Apr 2024 11:33:19 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org C471B3858D32 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.de ARC-Filter: OpenARC Filter v1.0.0 sourceware.org C471B3858D32 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2a07:de40:b251:101:10:150:64:1 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1713180803; cv=none; b=SGk7bUxGZr6k0uH0Pk36xkS+fRo8osGWEJ0uCafO5XMwrdZoVJjfuHJdqD+qgZ15nVdrLvU9ia5A+Vc30V38rFPxbWcR7jm3QcLWmWvIy4av7/n2IIpXQEHBtdigFgSpj5xqvVPoC+VMFWwnalt+LE4CUWx9OwJfXYTY/9TeI50= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1713180803; c=relaxed/simple; bh=2+qZTEehJ1HQyyajSGpWGAczTTfo2kehiYm8QH78YSg=; h=DKIM-Signature:DKIM-Signature:DKIM-Signature:DKIM-Signature:Date: From:To:Subject:Message-ID:MIME-Version; b=LrM9Da+RL44LbrOvjZAPo0xSq8zmFDcmP/7RqWLyYP30Us3bn7uMlR1DDd/jEBXP0xyIdthmsxNhEV5ifmtjBRefgNxbMAoyJ9BQJBdcu4NDyfBcLJHCqsZclwqa5c+djGMM3W44pz3p7eN32rLXXoadr45gIsEfGRmICoz1ILs= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from [10.168.5.241] (unknown [10.168.5.241]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id 63E7A347ED; Mon, 15 Apr 2024 11:33:18 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1713180798; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=3hnZLIIOPwf3uVqsFPNKxdpZgxncDnGt2VjriPz9Gv4=; b=gBYUGe2IgO6OiUi0hZ4Z+WZqUtVDYI9HWixz1Oaki+rDpXkb/WOJV875e8/5JsedAzIFNm 85VfZv/Bnn1p7Lfg180ml3Weupabhp+oO9cTGkpBjPMQxldmRWGNqFPO24Bxmtat4xoIKg RNTgId67K2Ica9bDAChaR7C6dPOTPBc= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1713180798; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=3hnZLIIOPwf3uVqsFPNKxdpZgxncDnGt2VjriPz9Gv4=; b=wDDHxEHJQ8U/KqbW+j5NedR4ksNt/eFO8mdlwXE4T77GntqaCVaku1GHwOG6VSOxtGeb1m SutxNJZ1drmPhDCA== Authentication-Results: smtp-out1.suse.de; none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1713180798; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=3hnZLIIOPwf3uVqsFPNKxdpZgxncDnGt2VjriPz9Gv4=; b=gBYUGe2IgO6OiUi0hZ4Z+WZqUtVDYI9HWixz1Oaki+rDpXkb/WOJV875e8/5JsedAzIFNm 85VfZv/Bnn1p7Lfg180ml3Weupabhp+oO9cTGkpBjPMQxldmRWGNqFPO24Bxmtat4xoIKg RNTgId67K2Ica9bDAChaR7C6dPOTPBc= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1713180798; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=3hnZLIIOPwf3uVqsFPNKxdpZgxncDnGt2VjriPz9Gv4=; b=wDDHxEHJQ8U/KqbW+j5NedR4ksNt/eFO8mdlwXE4T77GntqaCVaku1GHwOG6VSOxtGeb1m SutxNJZ1drmPhDCA== Date: Mon, 15 Apr 2024 13:33:18 +0200 (CEST) From: Richard Biener To: Jakub Jelinek cc: Martin Uecker , "Joseph S. Myers" , gcc-patches@gcc.gnu.org Subject: Re: [PATCH] c, v3: Fix ICE with -g and -std=c23 related to incomplete types [PR114361] In-Reply-To: Message-ID: References: <02a9b94e4d653b6f1b9f89a1b62187f46e871738.camel@tugraz.at> <448b0744f89d5c6ba5d6a5bacda154fcb74b7bca.camel@tugraz.at> <71q844s1-p7ps-492r-qp8r-r8nq69s8qp8n@fhfr.qr> <00r63o4n-88ns-0184-n594-55n69769n785@fhfr.qr> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Spam-Score: -4.30 X-Spam-Level: X-Spamd-Result: default: False [-4.30 / 50.00]; BAYES_HAM(-3.00)[100.00%]; NEURAL_HAM_LONG(-1.00)[-1.000]; NEURAL_HAM_SHORT(-0.20)[-1.000]; MIME_GOOD(-0.10)[text/plain]; MISSING_XM_UA(0.00)[]; MIME_TRACE(0.00)[0:+]; TO_DN_SOME(0.00)[]; ARC_NA(0.00)[]; RCVD_COUNT_ZERO(0.00)[0]; FROM_HAS_DN(0.00)[]; DKIM_SIGNED(0.00)[suse.de:s=susede2_rsa,suse.de:s=susede2_ed25519]; FROM_EQ_ENVFROM(0.00)[]; RCPT_COUNT_THREE(0.00)[4]; TO_MATCH_ENVRCPT_ALL(0.00)[]; FUZZY_BLOCKED(0.00)[rspamd.com]; DBL_BLOCKED_OPENRESOLVER(0.00)[suse.de:email] X-Spam-Status: No, score=-4.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Mon, 15 Apr 2024, Jakub Jelinek wrote: > On Mon, Apr 15, 2024 at 10:05:58AM +0200, Jakub Jelinek wrote: > > On Mon, Apr 15, 2024 at 10:02:25AM +0200, Richard Biener wrote: > > > > Though, haven't managed to reproduce it with -O2 -flto -std=c23 > > > > struct S; > > > > typedef struct S **V[10]; > > > > V **foo (int x) { return 0; } > > > > struct S { int s; }; > > > > either. > > > > So, maybe let's drop the ipa-free-lang-data.cc part? > > > > Seems fld_incomplete_type_of uses fld_type_variant which should > > > > copy over TYPE_CANONICAL. > > > > > > If you have a testcase that still triggers it would be nice to see it. > > > > I don't, that is why I'm now suggesting to just drop that hunk. > > Actually no, I've just screwed up something in my testing. > One can reproduce it easily with -O2 -flto 20021205-1.c -std=c23 > if the ipa-free-lang-data.cc hunk is removed. > This happens when fld_incomplete_type_of is called on a POINTER_TYPE > to RECORD_TYPE x, where the RECORD_TYPE x is not the TYPE_MAIN_VARIANT, > but another variant created by set_underlying_type. The > c_update_type_canonical didn't touch TYPE_CANONICAL in those, I was too > afraid I don't know what TYPE_CANONICAL should be for all variant types, > so that TREE_TYPE (t) had TYPE_CANONICAL NULL. But when we call > fld_incomplete_type_of on that TREE_TYPE (t), it sees it isn't > TYPE_MAIN_VARIANT, so calls > return (fld_type_variant > (fld_incomplete_type_of (TYPE_MAIN_VARIANT (t), fld), t, fld)); > but TYPE_MAIN_VARIANT (t) has already TYPE_CANONICAL (TYPE_MAIN_VARIANT (t)) > == TYPE_MAIN_VARIANT (t), that one has been completed on finish_struct. > And so we trigger the assertion, because > TYPE_CANONICAL (t2) == TYPE_CANONICAL (TREE_TYPE (t)) > is no longer true, the former is non-NULL, the latter is NULL. > > But looking at all the build_variant_type_copy callers and the call itself, > the call itself sets TYPE_CANONICAL to the TYPE_CANONICAL of the type on > which it is called and the only caller I can find that changes > TYPE_CANONICAL sometimes is build_qualified_type. > So, I'd hope that normally all variant types of an aggregate type (or > pointer type) have the same TYPE_CANONICAL if they have the same TYPE_QUALS > and if they have it different, they have TYPE_CANONICAL of > build_qualified_type of the base TYPE_CANONICAL. The middle-end assumes that TYPE_CANONICAL of all variant types are the same, for TBAA purposes it immediately "puns" to TYPE_CANONICAL (TYPE_MAIN_VARIANT (..)). It also assumes that the canonical type is not a variant type. Note we never "honor" TYPE_STRUCTURAL_EQUALITY_P on a variant type (because we don't look at it, we only look at whether the main variant has TYPE_STRUCTURAL_EQUALITY_P). Thus, TYPE_CANONICAL of variant types in principle doesn't need to be set (but not all places might go the extra step looking at the main variant before accessing TYPE_CANONICAL). Richard. > With the following updated patch (ipa-free-lang-data.cc hunk removed, > c_update_type_canonical function updated, plus removed trailing whitespace > from tests), > make check-gcc RUNTESTFLAGS="--target_board=unix/-std=gnu23 compile.exp='20021205-1.c 20040214-2.c 20060109-1.c pr113623.c pr46866.c pta-1.c' execute.exp='pr33870-1.c pr33870.c'" > no longer ICEs (have just expected FAILs on 20040214-2.c which isn't > compatible with C23) and make check-gcc -j32 doesn't regress compared > to the unpatched one. > > Is this ok for trunk if it passes full bootstrap/regtest? > > 2024-04-15 Martin Uecker > Jakub Jelinek > > PR lto/114574 > PR c/114361 > gcc/c/ > * c-decl.cc (shadow_tag_warned): For flag_isoc23 and code not > ENUMERAL_TYPE use SET_TYPE_STRUCTURAL_EQUALITY. > (parser_xref_tag): Likewise. > (start_struct): For flag_isoc23 use SET_TYPE_STRUCTURAL_EQUALITY. > (c_update_type_canonical): New function. > (finish_struct): Put NULL as second == operand rather than first. > Assert TYPE_STRUCTURAL_EQUALITY_P. Call c_update_type_canonical. > * c-typeck.cc (composite_type_internal): Use > SET_TYPE_STRUCTURAL_EQUALITY. Formatting fix. > gcc/testsuite/ > * gcc.dg/pr114574-1.c: New test. > * gcc.dg/pr114574-2.c: New test. > * gcc.dg/pr114361.c: New test. > * gcc.dg/c23-tag-incomplete-1.c: New test. > * gcc.dg/c23-tag-incomplete-2.c: New test. > > --- gcc/c/c-decl.cc.jj 2024-04-09 09:29:04.824520299 +0200 > +++ gcc/c/c-decl.cc 2024-04-15 12:26:43.000790475 +0200 > @@ -5051,6 +5051,8 @@ shadow_tag_warned (const struct c_declsp > if (t == NULL_TREE) > { > t = make_node (code); > + if (flag_isoc23 && code != ENUMERAL_TYPE) > + SET_TYPE_STRUCTURAL_EQUALITY (t); > pushtag (input_location, name, t); > } > } > @@ -8809,6 +8811,8 @@ parser_xref_tag (location_t loc, enum tr > the forward-reference will be altered into a real type. */ > > ref = make_node (code); > + if (flag_isoc23 && code != ENUMERAL_TYPE) > + SET_TYPE_STRUCTURAL_EQUALITY (ref); > if (code == ENUMERAL_TYPE) > { > /* Give the type a default layout like unsigned int > @@ -8910,6 +8914,8 @@ start_struct (location_t loc, enum tree_ > if (ref == NULL_TREE || TREE_CODE (ref) != code) > { > ref = make_node (code); > + if (flag_isoc23) > + SET_TYPE_STRUCTURAL_EQUALITY (ref); > pushtag (loc, name, ref); > } > > @@ -9347,6 +9353,45 @@ is_flexible_array_member_p (bool is_last > return false; > } > > +/* Recompute TYPE_CANONICAL for variants of the type including qualified > + versions of the type and related pointer types after an aggregate type > + has been finalized. > + Will not update array types, pointers to array types, function > + types and other derived types created while the type was still > + incomplete, those will remain TYPE_STRUCTURAL_EQUALITY_P. */ > + > +static void > +c_update_type_canonical (tree t) > +{ > + for (tree x = TYPE_MAIN_VARIANT (t); x; x = TYPE_NEXT_VARIANT (x)) > + { > + if (x != t && TYPE_STRUCTURAL_EQUALITY_P (x)) > + { > + if (TYPE_QUALS (x) == TYPE_QUALS (t)) > + TYPE_CANONICAL (x) = TYPE_CANONICAL (t); > + else if (TYPE_CANONICAL (t) != t > + || check_qualified_type (x, t, TYPE_QUALS (x))) > + TYPE_CANONICAL (x) > + = build_qualified_type (TYPE_CANONICAL (t), TYPE_QUALS (x)); > + else > + TYPE_CANONICAL (x) = x; > + } > + else if (x != t) > + continue; > + for (tree p = TYPE_POINTER_TO (x); p; p = TYPE_NEXT_PTR_TO (p)) > + { > + if (!TYPE_STRUCTURAL_EQUALITY_P (p)) > + continue; > + if (TYPE_CANONICAL (x) != x || TYPE_REF_CAN_ALIAS_ALL (p)) > + TYPE_CANONICAL (p) > + = build_pointer_type_for_mode (TYPE_CANONICAL (x), TYPE_MODE (p), > + false); > + else > + TYPE_CANONICAL (p) = p; > + c_update_type_canonical (p); > + } > + } > +} > > /* Fill in the fields of a RECORD_TYPE or UNION_TYPE node, T. > LOC is the location of the RECORD_TYPE or UNION_TYPE's definition. > @@ -9695,11 +9740,12 @@ finish_struct (location_t loc, tree t, t > /* Set type canonical based on equivalence class. */ > if (flag_isoc23) > { > - if (NULL == c_struct_htab) > + if (c_struct_htab == NULL) > c_struct_htab = hash_table::create_ggc (61); > > hashval_t hash = c_struct_hasher::hash (t); > > + gcc_checking_assert (TYPE_STRUCTURAL_EQUALITY_P (t)); > tree *e = c_struct_htab->find_slot_with_hash (t, hash, INSERT); > if (*e) > TYPE_CANONICAL (t) = *e; > @@ -9708,6 +9754,7 @@ finish_struct (location_t loc, tree t, t > TYPE_CANONICAL (t) = t; > *e = t; > } > + c_update_type_canonical (t); > } > > tree incomplete_vars = C_TYPE_INCOMPLETE_VARS (TYPE_MAIN_VARIANT (t)); > --- gcc/c/c-typeck.cc.jj 2024-02-08 11:17:37.035662909 +0100 > +++ gcc/c/c-typeck.cc 2024-04-12 17:17:33.029950025 +0200 > @@ -532,6 +532,7 @@ composite_type_internal (tree t1, tree t > /* Otherwise, create a new type node and link it into the cache. */ > > tree n = make_node (code1); > + SET_TYPE_STRUCTURAL_EQUALITY (n); > TYPE_NAME (n) = TYPE_NAME (t1); > > struct composite_cache cache2 = { t1, t2, n, cache }; > @@ -590,7 +591,8 @@ composite_type_internal (tree t1, tree t > TYPE_STUB_DECL (n) = pushdecl (build_decl (input_location, TYPE_DECL, > NULL_TREE, n)); > > - n = finish_struct(input_location, n, fields, attributes, NULL, &expr); > + n = finish_struct (input_location, n, fields, attributes, NULL, > + &expr); > > n = qualify_type (n, t1); > > --- gcc/testsuite/gcc.dg/pr114574-1.c.jj 2024-04-12 13:57:28.647782827 +0200 > +++ gcc/testsuite/gcc.dg/pr114574-1.c 2024-04-12 13:57:28.647782827 +0200 > @@ -0,0 +1,7 @@ > +/* PR lto/114574 > + * { dg-do compile } > + * { dg-options "-flto" } */ > + > +const struct S * x; > +struct S {}; > +void f(const struct S **); > --- gcc/testsuite/gcc.dg/pr114574-2.c.jj 2024-04-12 13:57:28.647782827 +0200 > +++ gcc/testsuite/gcc.dg/pr114574-2.c 2024-04-12 13:57:28.647782827 +0200 > @@ -0,0 +1,7 @@ > +/* PR lto/114574 > + * { dg-do compile } > + * { dg-options "-flto -std=c23" } */ > + > +const struct S * x; > +struct S {}; > +void f(const struct S **); > --- gcc/testsuite/gcc.dg/pr114361.c.jj 2024-04-12 13:57:28.684782327 +0200 > +++ gcc/testsuite/gcc.dg/pr114361.c 2024-04-12 13:57:28.684782327 +0200 > @@ -0,0 +1,10 @@ > +/* PR c/114361 */ > +/* { dg-do compile } */ > +/* { dg-options "-std=gnu23 -g" } */ > + > +void f() > +{ > + typedef struct foo bar; > + typedef __typeof( ({ (struct foo { bar *x; }){ }; }) ) wuz; > + struct foo { wuz *x; }; > +} > --- gcc/testsuite/gcc.dg/c23-tag-incomplete-1.c.jj 2024-04-12 13:57:28.684782327 +0200 > +++ gcc/testsuite/gcc.dg/c23-tag-incomplete-1.c 2024-04-12 13:57:28.684782327 +0200 > @@ -0,0 +1,11 @@ > +/* { dg-do compile } > + * { dg-options "-std=c23 -g" } */ > + > +struct a; > +typedef struct a b; > + > +void g() { > + struct a { b* x; }; > +} > + > +struct a { b* x; }; > --- gcc/testsuite/gcc.dg/c23-tag-incomplete-2.c.jj 2024-04-12 13:57:28.684782327 +0200 > +++ gcc/testsuite/gcc.dg/c23-tag-incomplete-2.c 2024-04-12 13:57:28.684782327 +0200 > @@ -0,0 +1,11 @@ > +/* { dg-do compile } > + * { dg-options "-std=c23 -g" } */ > + > +struct a; > +typedef struct a b; > + > +void f() { > + extern struct a { b* x; } t; > +} > + > +extern struct a { b* x; } t; > > > Jakub > > -- Richard Biener SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg, Germany; GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)