From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 6740 invoked by alias); 28 Apr 2016 10:53:28 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 6722 invoked by uid 89); 28 Apr 2016 10:53:27 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.2 required=5.0 tests=BAYES_50,RP_MATCHES_RCVD,SPF_PASS autolearn=ham version=3.3.2 spammy=ACATS, walking, pr59667c, explains X-HELO: mx2.suse.de Received: from mx2.suse.de (HELO mx2.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (CAMELLIA256-SHA encrypted) ESMTPS; Thu, 28 Apr 2016 10:53:17 +0000 Received: from relay1.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id 844E8AC15; Thu, 28 Apr 2016 10:53:11 +0000 (UTC) Date: Thu, 28 Apr 2016 10:53:00 -0000 From: Richard Biener To: Eric Botcazou cc: gcc-patches@gcc.gnu.org, jason@redhat.com Subject: Re: [PATCH] Fix type field walking in gimplifier unsharing In-Reply-To: <3302327.fD05QFZoa0@polaris> Message-ID: References: <1563866.3PAXIoXYk7@polaris> <3302327.fD05QFZoa0@polaris> User-Agent: Alpine 2.11 (LSU 23 2013-08-11) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-SW-Source: 2016-04/txt/msg01792.txt.bz2 On Thu, 28 Apr 2016, Eric Botcazou wrote: > > Aww, I was hoping for sth that would not require me to fix all > > frontends ... > > I don't really see how this can work without DECL_EXPR though. You need to > define when the variable-sized expressions are evaluated to lay out the type, > otherwise it will be laid out on the first use, which may see a different > value of the expressions than the definition point. The only way to do that > for a locally-defined type is to add a DECL_EXPR in GENERIC, so that the > gimplifier evaluates the expressions at the right spot. Ah, so the C++ FE does this correctly but in addition to that it has /* When the pointed-to type involves components of variable size, care must be taken to ensure that the size evaluation code is emitted early enough to dominate all the possible later uses and late enough for the variables on which it depends to have been assigned. This is expected to happen automatically when the pointed-to type has a name/declaration of it's own, but special attention is required if the type is anonymous. ... if (!TYPE_NAME (type) && (decl_context == NORMAL || decl_context == FIELD) && at_function_scope_p () && variably_modified_type_p (type, NULL_TREE)) /* Force evaluation of the SAVE_EXPR. */ finish_expr_stmt (TYPE_SIZE (type)); so in this case the type doesn't have an associated TYPE_DECL and thus we can't build a DECL_EXPR. To me the correct fix is then to always force a TYPE_DECL for variable-modified types. Jason? Now digging into the Fortran FE equivalent case... Thanks, Richard. > Of course in Ada we have the ACATS testsuite which tests for this kind of > things, this explains why it already works. > > > It seems the C frontend does it correctly already - I hit the > > ubsan issue for c-c++-common/ubsan/pr59667.c and only for the C++ FE > > for example. Notice how only the pointed-to type is variable-size > > here. > > > > C produces > > > > { > > unsigned int len = 1; > > typedef float [0:(sizetype) ((long int) SAVE_EXPR + > > -1)][0:(sizetype) ((long int) SAVE_EXPR + -1)]; > > float[0:(sizetype) ((long int) SAVE_EXPR + -1)][0:(sizetype) > > ((long int) SAVE_EXPR + -1)] * P = 0B; > > > > unsigned int len = 1; > > typedef float [0:(sizetype) ((long int) SAVE_EXPR + > > -1)][0:(sizetype) ((long int) SAVE_EXPR + -1)]; > > SAVE_EXPR ;, (void) SAVE_EXPR ;; > > float[0:(sizetype) ((long int) SAVE_EXPR + -1)][0:(sizetype) > > ((long int) SAVE_EXPR + -1)] * P = 0B; > > (*P)[0][0] = 1.0e+0; > > return 0; > > } > > > > the decl-expr is the 'typedef' line. The C++ FE produces > > > > { > > unsigned int len = 1; > > float[0:(sizetype) (SAVE_EXPR <(ssizetype) len + -1>)][0:(sizetype) > > (SAVE_EXPR <(ssizetype) len + -1>)] * P = 0B; > > > > <>; > > < > (void) (((bitsizetype) ((sizetype) (SAVE_EXPR <(ssizetype) len + -1>) + > > 1) * (bitsizetype) ((sizetype) (SAVE_EXPR <(ssizetype) len + -1>) + 1)) * > > 32) >>>>>; > > < > -1>)][0:(sizetype) (SAVE_EXPR <(ssizetype) len + -1>)] * P = 0B;>>; > > < > (void) ((*P)[0][0] = 1.0e+0) >>>>>; > > return = 0; > > } > > > > notice the lack of a decl-expr here. It has some weird expr_stmt > > here covering the sizes though. Possibly because VLA arrays are a GNU > > extension. > > Indeed. > > > Didn't look into the fortran FE issue but I expect it's similar > > (it only occurs for pointers to VLAs as well). > > > > I'll try to come up with patches. > > > > Thanks for the hint, > > You're welcome. > > -- Richard Biener SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)