From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) by sourceware.org (Postfix) with ESMTPS id 10C83385741C for ; Wed, 11 Aug 2021 07:02:32 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 10C83385741C Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out2.suse.de (Postfix) with ESMTP id ECF471FE9B; Wed, 11 Aug 2021 07:02:30 +0000 (UTC) Received: from murzim.suse.de (murzim.suse.de [10.160.4.192]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by relay2.suse.de (Postfix) with ESMTPS id E2115A3B87; Wed, 11 Aug 2021 07:02:30 +0000 (UTC) Date: Wed, 11 Aug 2021 09:02:30 +0200 (CEST) From: Richard Biener To: Qing Zhao cc: Jakub Jelinek , Nick Alcock via Gcc-patches , Kees Cook Subject: Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc In-Reply-To: Message-ID: References: <52E29277-1403-4755-901A-528116C43FB8@oracle.com> <58ADBC0C-9D44-485B-BB5A-B072664B9C4F@oracle.com> <6FD42B95-F73D-4B75-B83A-BAC4925B1714@oracle.com> User-Agent: Alpine 2.21 (LSU 202 2017-01-01) MIME-Version: 1.0 X-Spam-Status: No, score=-10.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, SPF_HELO_NONE, SPF_PASS, 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 Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 8BIT X-Content-Filtered-By: Mailman/MimeDel 2.1.29 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, 11 Aug 2021 07:02:42 -0000 On Tue, 10 Aug 2021, Qing Zhao wrote: > > > > On Aug 10, 2021, at 3:16 PM, Qing Zhao via Gcc-patches wrote: > > > > Hi, Richard, > > > >> On Aug 10, 2021, at 10:22 AM, Richard Biener wrote: > >>>> > >>>> Especially in the VLA case but likely also in general (though unlikely > >>>> since usually the receiver of initializations are simple enough). I'd > >>>> expect the VLA case end up as > >>>> > >>>> *ptr_to_decl = .DEFERRED_INIT (...); > >>>> > >>>> where *ptr_to_decl is the DECL_VALUE_EXPR of the decl. > >>> > >>> So, for the following small testing case: > >>> > >>> ==== > >>> extern void bar (int); > >>> > >>> void foo(int n) > >>> { > >>> int arr[n]; > >>> bar (arr[2]); > >>> return; > >>> } > >>> ===== > >>> > >>> If I compile it with -ftrivial-auto-var-init=zero -fdump-tree-gimple -S -o auto-init-11.s -fdump-rtl-expand, the *.gimple dump is: > >>> > >>> ===== > >>> void foo (int n) > >>> { > >>> int n.0; > >>> sizetype D.1950; > >>> bitsizetype D.1951; > >>> sizetype D.1952; > >>> bitsizetype D.1953; > >>> sizetype D.1954; > >>> int[0:D.1950] * arr.1; > >>> void * saved_stack.2; > >>> int arr[0:D.1950] [value-expr: *arr.1]; > >>> > >>> saved_stack.2 = __builtin_stack_save (); > >>> try > >>> { > >>> n.0 = n; > >>> _1 = (long int) n.0; > >>> _2 = _1 + -1; > >>> _3 = (sizetype) _2; > >>> D.1950 = _3; > >>> _4 = (sizetype) n.0; > >>> _5 = (bitsizetype) _4; > >>> _6 = _5 * 32; > >>> D.1951 = _6; > >>> _7 = (sizetype) n.0; > >>> _8 = _7 * 4; > >>> D.1952 = _8; > >>> _9 = (sizetype) n.0; > >>> _10 = (bitsizetype) _9; > >>> _11 = _10 * 32; > >>> D.1953 = _11; > >>> _12 = (sizetype) n.0; > >>> _13 = _12 * 4; > >>> D.1954 = _13; > >>> arr.1 = __builtin_alloca_with_align (D.1954, 32); > >>> arr = .DEFERRED_INIT (D.1952, 2, 1); > >>> _14 = (*arr.1)[2]; > >>> bar (_14); > >>> return; > >>> } > >>> finally > >>> { > >>> __builtin_stack_restore (saved_stack.2); > >>> } > >>> } > >>> > >>> ==== > >>> > >>> You think that the above .DEFEERED_INIT is not correct? > >>> It should be: > >>> > >>> *arr.1 = .DEFERRED_INIT (D.1952. 2, 1); > >>> > >>> ? > >> > >> Yes. > >> > > > > I updated gimplify.c for VLA and now it emits the call to .DEFERRED_INIT as: > > > > arr.1 = __builtin_alloca_with_align (D.1954, 32); > > *arr.1 = .DEFERRED_INIT (D.1952, 2, 1); > > > > However, this call triggered the assertion failure in verify_gimple_call of tree-cfg.c because the LHS is not a valid LHS. > > Then I modify tree-cfg.c as: > > > > diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c > > index 330eb7dd89bf..180d4f1f9e32 100644 > > --- a/gcc/tree-cfg.c > > +++ b/gcc/tree-cfg.c > > @@ -3375,7 +3375,11 @@ verify_gimple_call (gcall *stmt) > > } > > > > tree lhs = gimple_call_lhs (stmt); > > + /* For .DEFERRED_INIT call, the LHS might be an indirection of > > + a pointer for the VLA variable, which is not a valid LHS of > > + a gimple call, we ignore the asssertion on this. */ > > if (lhs > > + && (!gimple_call_internal_p (stmt, IFN_DEFERRED_INIT)) > > && (!is_gimple_reg (lhs) > > && (!is_gimple_lvalue (lhs) > > || verify_types_in_gimple_reference > > > > The assertion failure in tree-cfg.c got resolved, but I got another assertion failure in operands_scanner::get_expr_operands (tree *expr_p, int flags), line 945: > > > > 939 /* If we get here, something has gone wrong. */ > > 940 if (flag_checking) > > 941 { > > 942 fprintf (stderr, "unhandled expression in get_expr_operands():\n"); > > 943 debug_tree (expr); > > 944 fputs ("\n", stderr); > > 945 gcc_unreachable (); > > 946 } > > > > Looks like that the gimple statement: > > *arr.1 = .DEFERRED_INIT (D.1952, 2, 1); > > > > Is not valid. i.e, the LHS should not be an indirection to a pointer. > > > > How to resolve this issue? It sounds like the LHS is an INDIRECT_REF maybe? That means it's still not properly gimplified because it should end up as a MEM_REF instead. But I'm just guessing here ... if you are in a debugger then you can invoke debug_tree (lhs) in the inferior to see what it exactly is at the point of the failure. > I came up with the following solution: > > Define the IFN_DEFERRED_INIT function as: > > LHS = DEFERRED_INIT (SIZE of the DECL, INIT_TYPE, IS_VLA); > > if IS_VLA is false, the LHS is the DECL itself, > if IS_VLA is true, the LHS is the pointer to this DECL that created by > gimplify_vla_decl. > > > The benefit of this solution are: > > 1. Resolved the invalid IR issue; > 2. The call stmt carries the address of the VLA natually; > > The issue with this solution is: > > For VLA and non-VLA, the LHS will be different, > > Do you see any other potential issues with this solution? > > thanks. > > Qing > > > > > -- Richard Biener SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)