From patchwork Tue May 24 11:48:13 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom de Vries X-Patchwork-Id: 97151 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]) by ozlabs.org (Postfix) with SMTP id BE723B6F93 for ; Tue, 24 May 2011 21:50:48 +1000 (EST) Received: (qmail 30663 invoked by alias); 24 May 2011 11:50:46 -0000 Received: (qmail 30653 invoked by uid 22791); 24 May 2011 11:50:45 -0000 X-SWARE-Spam-Status: No, hits=-1.9 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_NONE, SPF_FAIL, TW_CP X-Spam-Check-By: sourceware.org Received: from smtp-vbr10.xs4all.nl (HELO smtp-vbr10.xs4all.nl) (194.109.24.30) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 24 May 2011 11:50:31 +0000 Received: from [192.168.1.68] (teejay.xs4all.nl [213.84.119.160]) (authenticated bits=0) by smtp-vbr10.xs4all.nl (8.13.8/8.13.8) with ESMTP id p4OBoPv7090014 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Tue, 24 May 2011 13:50:26 +0200 (CEST) (envelope-from vries@codesourcery.com) Message-ID: <4DDB9AFD.9080309@codesourcery.com> Date: Tue, 24 May 2011 13:48:13 +0200 From: Tom de Vries User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.17) Gecko/20110424 Lightning/1.0b2 Thunderbird/3.1.10 MIME-Version: 1.0 To: Nathan Froyd CC: gcc-patches@gcc.gnu.org, Richard Guenther Subject: Re: [PATCH] split tree_type, a.k.a. "tuplifying types" References: <20110510161543.GW23480@codesourcery.com> <4DD954D8.5000307@codesourcery.com> <4DDA7783.6050301@codesourcery.com> In-Reply-To: <4DDA7783.6050301@codesourcery.com> 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 On 05/23/2011 05:04 PM, Nathan Froyd wrote: > On 05/22/2011 02:24 PM, Tom de Vries wrote: >> Now that struct tree_type does not exist anymore, 'sizeof (struct tree_type)' >> generates an error in the following assert in fold_checksum_tree: >> ... >> gcc_assert ((sizeof (struct tree_exp) + 5 * sizeof (tree) >> <= sizeof (struct tree_function_decl)) >> && sizeof (struct tree_type) <= sizeof (struct tree_function_decl)); >> ... >> >> This error is triggered with -enable-checking=fold. > > Doh. Thanks for the report. > > The easy fix is s/tree_type/tree_type_non_common/. But I don't see why the > assert has to even care about tree_type; doesn't: > > gcc_assert ((sizeof (struct tree_exp) + 5 * sizeof (tree) > <= sizeof (union tree_node)); > > accomplish the same thing? > > -Nathan > > I don't know for sure what the assert is trying to check, but I'm guessing it's trying to check that the memcpys are save. A naive implementation would be: But that turns it into a runtime check. On the other hand, I'm not sure the original assert still makes sense. Neither tcc_type nor tcc_declaration have variable size, so the '5 * sizeof (tree)' does not seem applicable anymore. If we want checks cheaper than the naive, but more maintainable than the current, we would want something like: + gcc_assert (tree_class_max_size (tcc_declaration) <= sizeof (buf)); + gcc_assert (tree_class_max_size (tcc_type) <= sizeof (buf)); We would want those checks moved out of the hot path, and we would need to implement and maintain tree_class_max_size alongside tree_size and tree_code_size. But I'm not sure it's worth the effort. Thanks, - Tom Index: fold-const.c =================================================================== --- fold-const.c (revision 173703) +++ fold-const.c (working copy) @@ -13792,6 +13789,7 @@ recursive_label: && DECL_ASSEMBLER_NAME_SET_P (expr)) { /* Allow DECL_ASSEMBLER_NAME to be modified. */ + gcc_assert (tree_size (expr) <= sizeof (buf)); memcpy ((char *) &buf, expr, tree_size (expr)); SET_DECL_ASSEMBLER_NAME ((tree)&buf, NULL); expr = (tree) &buf; @@ -13805,6 +13803,7 @@ recursive_label: { /* Allow these fields to be modified. */ tree tmp; + gcc_assert (tree_size (expr) <= sizeof (buf)); memcpy ((char *) &buf, expr, tree_size (expr)); expr = tmp = (tree) &buf; TYPE_CONTAINS_PLACEHOLDER_INTERNAL (tmp) = 0;