From patchwork Tue Mar 29 12:40:06 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nathan Sidwell X-Patchwork-Id: 602883 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3qZ9Nf5zSqz9s6n for ; Tue, 29 Mar 2016 23:40:32 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=pvih2H+8; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:to:cc :from:subject:message-id:date:mime-version:content-type; q=dns; s=default; b=oDGNaIapQtdsHo+st7ly/tgk0CBu9PAAg6cDseUbd+PA+xlBsa 782QrnwC8fwGw1U3u51OXe5PmBpu410AFv6A2Ok+AVndbzb6VCvmmfnZhJCBoKvj 9qZK5apWl36jR0YSXMeRrjgCI3FyquHUzurXUbtBoqkEw1sagGZ3bfP7w= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:to:cc :from:subject:message-id:date:mime-version:content-type; s= default; bh=Z/vpiwAZ2Rji4ORuTZZRX6JIrHY=; b=pvih2H+8GWlnOBu7OjyM mmG9NuID2B8S7BcB/RnP4phxFkTrtYN3QkD9481HBeSeFoZevyx9QPIZZgoFvmBs fXrfBw+yjcYOyMxsAobto0MemPOU+pnS5VdOMnbhFzjYQZmKdndP3VFcH9r4/OLu p+TkSQkWinlHWbDWex1KIiE= Received: (qmail 45714 invoked by alias); 29 Mar 2016 12:40:20 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 45699 invoked by uid 89); 29 Mar 2016 12:40:20 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=BAYES_00, FREEMAIL_FROM, KAM_ASCII_DIVIDERS, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=no version=3.3.2 spammy=modulo, cep, initializes, Must X-HELO: mail-qk0-f175.google.com Received: from mail-qk0-f175.google.com (HELO mail-qk0-f175.google.com) (209.85.220.175) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Tue, 29 Mar 2016 12:40:10 +0000 Received: by mail-qk0-f175.google.com with SMTP id i4so5100699qkc.3 for ; Tue, 29 Mar 2016 05:40:10 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:sender:to:cc:from:subject:message-id:date :user-agent:mime-version; bh=jSJMw+cQmBFsNVYLG5KyYS3NVHaObQwl78S4JF42CAw=; b=mP290DU0aUCfJu4xtejCUaJj/fc3AKZGOPHy//INyuVdRE7E3bZuuQnzTlBvh7pseD YEQ/L6308wkdoedRuOdK5++zH6G+oCEx2vQYZJd4X5+wJynbsIqI95jdVjwM0/V1lAe4 AqIj9ROcAbLBWLOk6fbW49sj5fKVjiQq73N5ClEcgTlPDw5cv4/m8E9Ny/O4FgkO/yXQ smsUNGrM68TsrZyY4li7SV7uZiHuJHH3a/d+LxNn+XF9w4J89C3+EUlm221+pFgMuvus 34v9Zwfbq2dzzaoR9Er0u4RfpSGW38PWvTJaE1HWe8szNz6A7dk2HgA2THoRt+gBOT6I 49rA== X-Gm-Message-State: AD7BkJLTNyShIq2ZtHo/E/xefBMFudx6i3ISXJ8TQ0SAp6/DZMLsmKTbeNckBvyDzO2Y9g== X-Received: by 10.55.41.205 with SMTP id p74mr2376588qkp.65.1459255208181; Tue, 29 Mar 2016 05:40:08 -0700 (PDT) Received: from ?IPv6:2601:181:c000:c497:a2a8:cdff:fe3e:b48? ([2601:181:c000:c497:a2a8:cdff:fe3e:b48]) by smtp.googlemail.com with ESMTPSA id z65sm13683008qhb.36.2016.03.29.05.40.07 (version=TLSv1/SSLv3 cipher=OTHER); Tue, 29 Mar 2016 05:40:07 -0700 (PDT) To: Jason Merrill Cc: GCC Patches From: Nathan Sidwell Subject: [C++/70393] constexpr constructor Message-ID: <56FA77A6.8040408@acm.org> Date: Tue, 29 Mar 2016 08:40:06 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.7.1 MIME-Version: 1.0 This patch fixes 70393 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70393 'ab's construction used to be dynamic but becomes static with C++11 constexpr constructors (I'm not sure whether we're doing more than the std requires, but that's not important). However, 'AB's bases are not laid out in program declaration order. B is chosen as the primary base, and A is placed after that. But the constructor creates A first and then B. We're simply appending new elements onto AB's CONSTRUCTOR, and that's not something varasm is prepared to deal with. First, I add an assert to output_constructor_regular_field so the silent bad code generation turns into an ICE. This discovered the problem in g++.dg/cpp0x/constexpr-virtual[34].C testcases too. I moved the flushing of bitfields earlier so as to move the offset calculation nearer its use (and to permit some jump threading in output_constructor_regular_field's code). Second, the fix itself is in cxx_eval_store_expression. Currently we scan the CONSTRUCTOR ELTS to see if we've met this field before, otherwise we append. I modified the scanning loop to also iterate over the FIELD_DECLS of the record itself, and detect when we find the wanted field in the struct before the current CTOR ELT. Then we do an insert at the current point. (inserting at the end is valid, btw). I did consider just checking 'int_byte_position (local->field) == 0' to detect the primary base case, but that struck me as rather fragile, and as constexprs get more powerful, likely to break for cases with virtual bases. (right now virtual bases still cause dynamic initialization) built & tested on x86_64-linux nathan 2016-03-29 Nathan Sidwell PR c++/70393 * varasm.c (output_constructor_regular_field): Flush bitfield earlier. Assert we don't want to move backwards. gcc/ PR c++/70393 * constexpr.c (cxx_eval_store_expression): Keep CONSTRUCTOR elements in field order. PR c++/70393 * g++.dg/cpp0x/constexpr-virtual6.C: New. Index: varasm.c =================================================================== --- varasm.c (revision 234503) +++ varasm.c (working copy) @@ -4929,6 +4929,14 @@ output_constructor_regular_field (oc_loc unsigned int align2; + /* Output any buffered-up bit-fields preceding this element. */ + if (local->byte_buffer_in_use) + { + assemble_integer (GEN_INT (local->byte), 1, BITS_PER_UNIT, 1); + local->total_bytes++; + local->byte_buffer_in_use = false; + } + if (local->index != NULL_TREE) { /* Perform the index calculation in modulo arithmetic but @@ -4945,22 +4953,19 @@ output_constructor_regular_field (oc_loc else fieldpos = 0; - /* Output any buffered-up bit-fields preceding this element. */ - if (local->byte_buffer_in_use) - { - assemble_integer (GEN_INT (local->byte), 1, BITS_PER_UNIT, 1); - local->total_bytes++; - local->byte_buffer_in_use = false; - } - /* Advance to offset of this element. Note no alignment needed in an array, since that is guaranteed if each element has the proper size. */ - if ((local->field != NULL_TREE || local->index != NULL_TREE) - && fieldpos > local->total_bytes) + if (local->field != NULL_TREE || local->index != NULL_TREE) { - assemble_zeros (fieldpos - local->total_bytes); - local->total_bytes = fieldpos; + if (fieldpos > local->total_bytes) + { + assemble_zeros (fieldpos - local->total_bytes); + local->total_bytes = fieldpos; + } + else + /* Must not go backwards. */ + gcc_assert (fieldpos == local->total_bytes); } /* Find the alignment of this element. */ Index: cp/constexpr.c =================================================================== --- cp/constexpr.c (revision 234503) +++ cp/constexpr.c (working copy) @@ -2959,16 +2959,39 @@ cxx_eval_store_expression (const constex else { gcc_assert (TREE_CODE (index) == FIELD_DECL); - for (unsigned HOST_WIDE_INT idx = 0; + + /* We must keep the CONSTRUCTOR's ELTS in FIELD order. + Usually we meet initializers in that order, but it is + possible for base types to be placed not in program + order. */ + tree fields = TYPE_FIELDS (DECL_CONTEXT (index)); + unsigned HOST_WIDE_INT idx; + + for (idx = 0; vec_safe_iterate (CONSTRUCTOR_ELTS (*valp), idx, &cep); idx++) - if (index == cep->index) - break; - if (!cep) { - constructor_elt ce = { index, NULL_TREE }; - cep = vec_safe_push (CONSTRUCTOR_ELTS (*valp), ce); + if (index == cep->index) + goto found; + + /* The field we're initializing must be on the field + list. Look to see if it is present before the + field the current ELT initializes. */ + for (; fields != cep->index; fields = DECL_CHAIN (fields)) + if (index == fields) + goto insert; } + + /* We fell off the end of the CONSTRUCTOR, so insert a new + entry at the end. */ + insert: + { + constructor_elt ce = { index, NULL_TREE }; + + vec_safe_insert (CONSTRUCTOR_ELTS (*valp), idx, ce); + cep = CONSTRUCTOR_ELT (*valp, idx); + } + found:; } valp = &cep->value; } Index: testsuite/g++.dg/cpp0x/constexpr-virtual6.C =================================================================== --- testsuite/g++.dg/cpp0x/constexpr-virtual6.C (nonexistent) +++ testsuite/g++.dg/cpp0x/constexpr-virtual6.C (working copy) @@ -0,0 +1,49 @@ +// PR c++/70393 +// { dg-do run { target c++11 } } + +/* 'ab' has a static initializer, but we flubbed the initializer, + because of B being the primary base. */ + +struct A +{ + int a = 1; +}; + +struct B +{ + B *element = (B*)2; + + virtual int vfunc() = 0; + + int call_element() + { + return element->vfunc(); + } + + void set_element() + { + element = this; + } +}; + +struct AB : public A, public B +{ + int vfunc() + { + return 0; + } +}; + +static AB ab; + +int main() +{ + if (ab.a != 1) + return 1; + if (ab.element != (void*)2) + return 2; + + ab.set_element(); + return ab.call_element(); +} +