From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTP id 09A4C3858C39 for ; Wed, 15 Sep 2021 19:00:01 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 09A4C3858C39 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-51-M5i3A_k-M5yfuRxOoAObDQ-1; Wed, 15 Sep 2021 14:59:59 -0400 X-MC-Unique: M5i3A_k-M5yfuRxOoAObDQ-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 6572E1926DA0 for ; Wed, 15 Sep 2021 18:59:58 +0000 (UTC) Received: from tucnak.zalov.cz (unknown [10.39.192.10]) by smtp.corp.redhat.com (Postfix) with ESMTPS id F345360C82; Wed, 15 Sep 2021 18:59:57 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.16.1/8.16.1) with ESMTPS id 18FIxtUY3072424 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Wed, 15 Sep 2021 20:59:56 +0200 Received: (from jakub@localhost) by tucnak.zalov.cz (8.16.1/8.16.1/Submit) id 18FIxrux3072423; Wed, 15 Sep 2021 20:59:53 +0200 Date: Wed, 15 Sep 2021 20:59:53 +0200 From: Jakub Jelinek To: Jason Merrill Cc: gcc-patches@gcc.gnu.org Subject: [PATCH] c++: Fix handling of decls with flexible array members initialized with side-effects [PR88578] Message-ID: <20210915185953.GC304296@tucnak> Reply-To: Jakub Jelinek References: <20210914090206.GV304296@tucnak> MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Spam-Status: No, score=-5.8 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 15 Sep 2021 19:00:02 -0000 On Tue, Sep 14, 2021 at 10:50:32AM -0400, Jason Merrill wrote: > > Note, if the flexible array member is initialized only with non-constant > > initializers, we have a worse bug that this patch doesn't solve, the > > splitting of initializers into constant and dynamic initialization removes > > the initializer and we don't have just wrong DECL_*SIZE, but nothing is > > emitted when emitting those vars into assembly either and so the dynamic > > initialization clobbers other vars that may overlap the variable. > > I think we need keep an empty CONSTRUCTOR elt in DECL_INITIAL for the > > flexible array member in that case. > > Makes sense. So, the following patch fixes that. The typeck2.c change makes sure we keep those CONSTRUCTORs around (although they should be empty because all their elts had side-effects/was non-constant if it was removed earlier), and the varasm.c change is to avoid ICEs on those as well as ICEs on other flex array members that had some initializers without side-effects, but not on the last array element. The code was already asserting that the (index of the last elt in the CONSTRUCTOR + 1) times elt size is equal to TYPE_SIZE_UNIT of the local->val type, which is true for C flex arrays or for C++ if they don't have any side-effects or the last elt doesn't have side-effects, this patch changes that to assertion that the TYPE_SIZE_UNIT is greater than equal to the offset of the end of last element in the CONSTRUCTOR and uses TYPE_SIZE_UNIT (int_size_in_bytes) in the code later on. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2021-09-15 Jakub Jelinek PR c++/88578 PR c++/102295 gcc/ * varasm.c (output_constructor_regular_field): Instead of assertion that array_size_for_constructor result is equal to size of TREE_TYPE (local->val) in bytes, assert that the type size is greater or equal to array_size_for_constructor result and use type size as fieldsize. gcc/cp/ * typeck2.c (split_nonconstant_init_1): Don't throw away empty initializers of flexible array members if they have non-zero type size. gcc/testsuite/ * g++.dg/ext/flexary39.C: New test. * g++.dg/ext/flexary40.C: New test. --- gcc/varasm.c.jj 2021-05-10 12:22:30.437451816 +0200 +++ gcc/varasm.c 2021-09-15 13:19:02.841554574 +0200 @@ -5531,14 +5531,15 @@ output_constructor_regular_field (oc_loc && (!TYPE_DOMAIN (TREE_TYPE (local->field)) || !TYPE_MAX_VALUE (TYPE_DOMAIN (TREE_TYPE (local->field))))) { - fieldsize = array_size_for_constructor (local->val); + unsigned HOST_WIDE_INT fldsize + = array_size_for_constructor (local->val); + fieldsize = int_size_in_bytes (TREE_TYPE (local->val)); + gcc_checking_assert (fieldsize >= fldsize); /* Given a non-empty initialization, this field had better be last. Given a flexible array member, the next field on the chain is a TYPE_DECL of the enclosing struct. */ const_tree next = DECL_CHAIN (local->field); gcc_assert (!fieldsize || !next || TREE_CODE (next) != FIELD_DECL); - tree size = TYPE_SIZE_UNIT (TREE_TYPE (local->val)); - gcc_checking_assert (compare_tree_int (size, fieldsize) == 0); } else fieldsize = tree_to_uhwi (DECL_SIZE_UNIT (local->field)); --- gcc/cp/typeck2.c.jj 2021-09-01 21:30:30.520306823 +0200 +++ gcc/cp/typeck2.c 2021-09-15 14:15:59.049388381 +0200 @@ -524,7 +524,20 @@ split_nonconstant_init_1 (tree dest, tre sub = build3 (COMPONENT_REF, inner_type, dest, field_index, NULL_TREE); - if (!split_nonconstant_init_1 (sub, value, true)) + if (!split_nonconstant_init_1 (sub, value, true) + /* For flexible array member with initializer we + can't remove the initializer, because only the + initializer determines how many elements the + flexible array member has. */ + || (!array_type_p + && TREE_CODE (inner_type) == ARRAY_TYPE + && TYPE_DOMAIN (inner_type) == NULL + && TREE_CODE (TREE_TYPE (value)) == ARRAY_TYPE + && COMPLETE_TYPE_P (TREE_TYPE (value)) + && !integer_zerop (TYPE_SIZE (TREE_TYPE (value))) + && idx == CONSTRUCTOR_NELTS (init) - 1 + && TYPE_HAS_TRIVIAL_DESTRUCTOR + (strip_array_types (inner_type)))) complete_p = false; else { --- gcc/testsuite/g++.dg/ext/flexary39.C.jj 2021-09-15 14:01:33.811320756 +0200 +++ gcc/testsuite/g++.dg/ext/flexary39.C 2021-09-15 14:04:05.962221748 +0200 @@ -0,0 +1,65 @@ +// PR c++/88578 +// { dg-do run } +// { dg-options -Wno-pedantic } + +#define STR(s) #s +#define ASSERT(exp) \ + ((exp) ? (void)0 : (void)(__builtin_printf ("%s:%i: assertion %s failed\n", \ + __FILE__, __LINE__, STR(exp)), \ + __builtin_abort ())) + +typedef int int32_t __attribute__((mode (__SI__))); + +struct Ax { int32_t n, a[]; }; +struct AAx { int32_t i; Ax ax; }; + +int32_t i = 12345678; + +void +test () +{ + { + // OK. Does not assign any elements to flexible array. + Ax s = { 0 }; + ASSERT (s.n == 0); + } + { + // OK only for statically allocated objects, otherwise error. + static Ax s = { 0, { } }; + ASSERT (s.n == 0); + } + { + static Ax s = { 1, { 2 } }; + ASSERT (s.n == 1 && s.a [0] == 2); + } + { + static Ax s = { 2, { 3, 4 } }; + ASSERT (s.n = 2 && s.a [0] == 3 && s.a [1] == 4); + } + { + static Ax s = { 123, i }; + ASSERT (s.n == 123 && s.a [0] == i); + } + { + static Ax s = { 456, { i } }; + ASSERT (s.n == 456 && s.a [0] == i); + } + { + int32_t j = i + 1, k = j + 1; + static Ax s = { 3, { i, j, k } }; + ASSERT (s.n == 3 && s.a [0] == i && s.a [1] == j && s.a [2] == k); + } + + { + // OK. Does not assign any elements to flexible array. + AAx s = { 1, { 2 } }; + ASSERT (s.i == 1 && s.ax.n == 2); + } +} + +int +main () +{ + test (); + test (); +} --- gcc/testsuite/g++.dg/ext/flexary40.C.jj 2021-09-15 14:03:52.328409836 +0200 +++ gcc/testsuite/g++.dg/ext/flexary40.C 2021-09-15 13:58:40.888706320 +0200 @@ -0,0 +1,50 @@ +// PR c++/102295 +// { dg-do run } +// { dg-options "" } + +struct A { int a; int b[]; }; +struct B { B (); int k; }; +struct C { int l; B m[]; }; + +int x[4]; +A c = { 42, { ++x[0], ++x[1], ++x[2], ++x[3] } }; +A d = { 43, { 0, ++x[0], ++x[1], ++x[2], ++x[3] } }; +A e = { 44, { ++x[0], ++x[1], ++x[2], 17 } }; +A f = { 45 }; +C n = { 50, { B (), B () } }; +C o = { 51, {} }; + +int +main () +{ + static A g = { 46, { ++x[0], ++x[1], ++x[2], ++x[3] } }; + static A h = { 47, { 0, ++x[0], ++x[1], ++x[2], ++x[3] } }; + static A i = { 48, { ++x[0], ++x[1], ++x[2], 18 } }; + static A j = { 49 }; + if (c.a != 42 || c.b[0] != 1 || c.b[1] != 1 || c.b[2] != 1 || c.b[3] != 1) + __builtin_abort (); + if (d.a != 43 || d.b[0] != 0 || d.b[1] != 2 || d.b[2] != 2 || d.b[3] != 2 || d.b[4] != 2) + __builtin_abort (); + if (e.a != 44 || e.b[0] != 3 || e.b[1] != 3 || e.b[2] != 3 || e.b[3] != 17) + __builtin_abort (); + if (f.a != 45) + __builtin_abort (); + if (g.a != 46 || g.b[0] != 4 || g.b[1] != 4 || g.b[2] != 4 || g.b[3] != 3) + __builtin_abort (); + if (h.a != 47 || h.b[0] != 0 || h.b[1] != 5 || h.b[2] != 5 || h.b[3] != 5 || h.b[4] != 4) + __builtin_abort (); + if (i.a != 48 || i.b[0] != 6 || i.b[1] != 6 || i.b[2] != 6 || i.b[3] != 18) + __builtin_abort (); + if (j.a != 49) + __builtin_abort (); + if (n.l != 50 || n.m[0].k != 42 || n.m[1].k != 42) + __builtin_abort (); + if (o.l != 51) + __builtin_abort (); + if (x[0] != 6 || x[1] != 6 || x[2] != 6 || x[3] != 4) + __builtin_abort (); +} + +B::B () : k (42) +{ +} Jakub