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 469D0383A81E for ; Wed, 11 Aug 2021 15:53:12 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 469D0383A81E Received: from imap1.suse-dmz.suse.de (imap1.suse-dmz.suse.de [192.168.254.73]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id 4B0B51FEDA; Wed, 11 Aug 2021 15:53:11 +0000 (UTC) Received: from imap1.suse-dmz.suse.de (imap1.suse-dmz.suse.de [192.168.254.73]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap1.suse-dmz.suse.de (Postfix) with ESMTPS id 2DAA813969; Wed, 11 Aug 2021 15:53:11 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap1.suse-dmz.suse.de with ESMTPSA id oeFaCmfyE2FmYAAAGKfGzw (envelope-from ); Wed, 11 Aug 2021 15:53:11 +0000 Date: Wed, 11 Aug 2021 17:53:08 +0200 From: Richard Biener To: Qing Zhao , Richard Sandiford CC: Jakub Jelinek , Nick Alcock via Gcc-patches , Kees Cook Subject: =?US-ASCII?Q?Re=3A_=5Bpatch=5D=5Bversion_6=5D_add_-ftrivial-auto-var-in?= =?US-ASCII?Q?it_and_variable_attribute_=22uninitialized=22_to_gcc?= User-Agent: K-9 Mail for Android In-Reply-To: References: <52E29277-1403-4755-901A-528116C43FB8@oracle.com> <58ADBC0C-9D44-485B-BB5A-B072664B9C4F@oracle.com> <6FD42B95-F73D-4B75-B83A-BAC4925B1714@oracle.com> Message-ID: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-11.4 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 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 15:53:22 -0000 On August 11, 2021 5:30:40 PM GMT+02:00, Qing Zhao wrote: >I modified the routine =E2=80=9Cgimple_add_init_for_auto_var=E2=80=9D as = the following: >=3D=3D=3D=3D >/* Generate initialization to automatic variable DECL based on INIT_TYPE= =2E > Build a call to internal const function DEFERRED_INIT: > 1st argument: SIZE of the DECL; > 2nd argument: INIT_TYPE; > 3rd argument: IS_VLA, 0 NO, 1 YES; > > as DEFERRED_INIT (SIZE of the DECL, INIT_TYPE, IS_VLA)=2E */ >static void >gimple_add_init_for_auto_var (tree decl, > enum auto_init_type init_type, > bool is_vla, > gimple_seq *seq_p) >{ > gcc_assert (VAR_P (decl) && !DECL_EXTERNAL (decl) && !TREE_STATIC (decl= )); > gcc_assert (init_type > AUTO_INIT_UNINITIALIZED); > tree decl_size =3D TYPE_SIZE_UNIT (TREE_TYPE (decl)); > > tree init_type_node > =3D build_int_cst (integer_type_node, (int) init_type); > tree is_vla_node > =3D build_int_cst (integer_type_node, (int) is_vla); > > tree call =3D build_call_expr_internal_loc (UNKNOWN_LOCATION, IFN_DEFER= RED_INIT, > TREE_TYPE (decl), 3, > decl_size, init_type_node, > is_vla_node); > > /* If this DECL is a VLA, a temporary address variable for it has been > created, the replacement for DECL is recorded in DECL_VALUE_EXPR (de= cl), > we should use it as the LHS of the call=2E */ > > tree lhs_call > =3D is_vla ? DECL_VALUE_EXPR (decl) : decl; > gimplify_assign (lhs_call, call, seq_p); >} > >With this change, the current issue is resolved, the gimple dump now is: > > (*arr=2E1) =3D =2EDEFERRED_INIT (D=2E1952, 2, 1); > >However, there is another new issue: > >For the following testing case: > >=3D=3D=3D=3D=3D=3D >[opc@qinzhao-ol8u3-x86 gcc]$ cat t=2Ec >int bar; > >extern void decode_reloc(int *); > >void testfunc() >{ > int alt_reloc; > > decode_reloc(&alt_reloc); > > if (alt_reloc) /* { dg-warning "may be used uninitialized" } */ > bar =3D 42;=20 >} >=3D=3D=3D=3D=3D > >In the above, the auto var =E2=80=9Calt_reloc=E2=80=9D is address taken, = then the gimple dump for it when compiled with -ftrivial-auto-var-init=3Dze= ro is: > >void testfunc () >{ > int alt_reloc; > > try > { > _1 =3D =2EDEFERRED_INIT (4, 2, 0); > alt_reloc =3D _1; > decode_reloc (&alt_reloc); > alt_reloc=2E0_2 =3D alt_reloc; > if (alt_reloc=2E0_2 !=3D 0) goto ; else goto ; > : > bar =3D 42; > : > } > finally > { > alt_reloc =3D {CLOBBER}; > } >} > >I=2Ee, instead of the expected IR: > >alt_reloc =3D =2EDEFERRED_INIT (4, 2, 0); > >We got the following: > > _1 =3D =2EDEFERRED_INIT (4, 2, 0); > alt_reloc =3D _1; > >I guess the temp =E2=80=9C_1=E2=80=9D is created because =E2=80=9Calt_rel= oc=E2=80=9D is address taken=2E=20 Yes and no=2E The reason is that alt_reloc is memory (because it is addres= s taken) and that GIMPLE says that register typed stores need to use a is_g= imple_val RHS which the call is not=2E >My questions: > >Shall we accept such IR for =2EDEFERRED_INIT purpose when the auto var is= address taken?=20 I think so=2E Note it doesn't necessarily need address taking but any othe= r reason that prevents SSA rewriting the variable suffices=2E=20 The only other option is to force=2E DEFERED_INIT making the LHS address t= aken which I think could be achieved by passing it the address as argument = instead of having a LHS=2E But let's not go down this route - it will have = quite bad behavior on alias analysis and optimization=2E=20 >If so, =E2=80=9Cuninitialized analysis=E2=80=9D phase need to be further = adjusted to specially handle such IR=2E=20 > >If not, what should we do when the auto var is address taken? > >Thanks a lot=2E > >Qing > > >> On Aug 11, 2021, at 8:58 AM, Richard Biener wrote= : >>=20 >> On Wed, 11 Aug 2021, Qing Zhao wrote: >>=20 >>>=20 >>>=20 >>>> On Aug 11, 2021, at 8:37 AM, Richard Biener wro= te: >>>>=20 >>>> On Wed, 11 Aug 2021, Qing Zhao wrote: >>>>=20 >>>>>=20 >>>>>=20 >>>>>> On Aug 11, 2021, at 2:02 AM, Richard Biener w= rote: >>>>>>=20 >>>>>> On Tue, 10 Aug 2021, Qing Zhao wrote: >>>>>>=20 >>>>>>>=20 >>>>>>>=20 >>>>>>>> On Aug 10, 2021, at 3:16 PM, Qing Zhao via Gcc-patches wrote: >>>>>>>>=20 >>>>>>>> Hi, Richard, >>>>>>>>=20 >>>>>>>>> On Aug 10, 2021, at 10:22 AM, Richard Biener wrote: >>>>>>>>>>>=20 >>>>>>>>>>> Especially in the VLA case but likely also in general (though = unlikely >>>>>>>>>>> since usually the receiver of initializations are simple enoug= h)=2E I'd >>>>>>>>>>> expect the VLA case end up as >>>>>>>>>>>=20 >>>>>>>>>>> *ptr_to_decl =3D =2EDEFERRED_INIT (=2E=2E=2E); >>>>>>>>>>>=20 >>>>>>>>>>> where *ptr_to_decl is the DECL_VALUE_EXPR of the decl=2E >>>>>>>>>>=20 >>>>>>>>>> So, for the following small testing case: >>>>>>>>>>=20 >>>>>>>>>> =3D=3D=3D=3D >>>>>>>>>> extern void bar (int); >>>>>>>>>>=20 >>>>>>>>>> void foo(int n) >>>>>>>>>> { >>>>>>>>>> int arr[n]; >>>>>>>>>> bar (arr[2]); >>>>>>>>>> return; >>>>>>>>>> } >>>>>>>>>> =3D=3D=3D=3D=3D >>>>>>>>>>=20 >>>>>>>>>> If I compile it with -ftrivial-auto-var-init=3Dzero -fdump-tree= -gimple -S -o auto-init-11=2Es -fdump-rtl-expand, the *=2Egimple dump is: >>>>>>>>>>=20 >>>>>>>>>> =3D=3D=3D=3D=3D >>>>>>>>>> void foo (int n) >>>>>>>>>> { >>>>>>>>>> int n=2E0; >>>>>>>>>> sizetype D=2E1950; >>>>>>>>>> bitsizetype D=2E1951; >>>>>>>>>> sizetype D=2E1952; >>>>>>>>>> bitsizetype D=2E1953; >>>>>>>>>> sizetype D=2E1954; >>>>>>>>>> int[0:D=2E1950] * arr=2E1; >>>>>>>>>> void * saved_stack=2E2; >>>>>>>>>> int arr[0:D=2E1950] [value-expr: *arr=2E1]; >>>>>>>>>>=20 >>>>>>>>>> saved_stack=2E2 =3D __builtin_stack_save (); >>>>>>>>>> try >>>>>>>>>> { >>>>>>>>>> n=2E0 =3D n; >>>>>>>>>> _1 =3D (long int) n=2E0; >>>>>>>>>> _2 =3D _1 + -1; >>>>>>>>>> _3 =3D (sizetype) _2; >>>>>>>>>> D=2E1950 =3D _3; >>>>>>>>>> _4 =3D (sizetype) n=2E0; >>>>>>>>>> _5 =3D (bitsizetype) _4; >>>>>>>>>> _6 =3D _5 * 32; >>>>>>>>>> D=2E1951 =3D _6; >>>>>>>>>> _7 =3D (sizetype) n=2E0; >>>>>>>>>> _8 =3D _7 * 4; >>>>>>>>>> D=2E1952 =3D _8; >>>>>>>>>> _9 =3D (sizetype) n=2E0; >>>>>>>>>> _10 =3D (bitsizetype) _9; >>>>>>>>>> _11 =3D _10 * 32; >>>>>>>>>> D=2E1953 =3D _11; >>>>>>>>>> _12 =3D (sizetype) n=2E0; >>>>>>>>>> _13 =3D _12 * 4; >>>>>>>>>> D=2E1954 =3D _13; >>>>>>>>>> arr=2E1 =3D __builtin_alloca_with_align (D=2E1954, 32); >>>>>>>>>> arr =3D =2EDEFERRED_INIT (D=2E1952, 2, 1); >>>>>>>>>> _14 =3D (*arr=2E1)[2]; >>>>>>>>>> bar (_14); >>>>>>>>>> return; >>>>>>>>>> } >>>>>>>>>> finally >>>>>>>>>> { >>>>>>>>>> __builtin_stack_restore (saved_stack=2E2); >>>>>>>>>> } >>>>>>>>>> } >>>>>>>>>>=20 >>>>>>>>>> =3D=3D=3D=3D >>>>>>>>>>=20 >>>>>>>>>> You think that the above =2EDEFEERED_INIT is not correct? >>>>>>>>>> It should be: >>>>>>>>>>=20 >>>>>>>>>> *arr=2E1 =3D =2EDEFERRED_INIT (D=2E1952=2E 2, 1); >>>>>>>>>>=20 >>>>>>>>>> ? >>>>>>>>>=20 >>>>>>>>> Yes=2E >>>>>>>>>=20 >>>>>>>>=20 >>>>>>>> I updated gimplify=2Ec for VLA and now it emits the call to =2EDE= FERRED_INIT as: >>>>>>>>=20 >>>>>>>> arr=2E1 =3D __builtin_alloca_with_align (D=2E1954, 32); >>>>>>>> *arr=2E1 =3D =2EDEFERRED_INIT (D=2E1952, 2, 1); >>>>>>>>=20 >>>>>>>> However, this call triggered the assertion failure in verify_gimp= le_call of tree-cfg=2Ec because the LHS is not a valid LHS=2E=20 >>>>>>>> Then I modify tree-cfg=2Ec as: >>>>>>>>=20 >>>>>>>> diff --git a/gcc/tree-cfg=2Ec b/gcc/tree-cfg=2Ec >>>>>>>> index 330eb7dd89bf=2E=2E180d4f1f9e32 100644 >>>>>>>> --- a/gcc/tree-cfg=2Ec >>>>>>>> +++ b/gcc/tree-cfg=2Ec >>>>>>>> @@ -3375,7 +3375,11 @@ verify_gimple_call (gcall *stmt) >>>>>>>> } >>>>>>>>=20 >>>>>>>> tree lhs =3D gimple_call_lhs (stmt); >>>>>>>> + /* For =2EDEFERRED_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=2E */=20 >>>>>>>> if (lhs >>>>>>>> + && (!gimple_call_internal_p (stmt, IFN_DEFERRED_INIT)) >>>>>>>> && (!is_gimple_reg (lhs) >>>>>>>> && (!is_gimple_lvalue (lhs) >>>>>>>> || verify_types_in_gimple_reference >>>>>>>>=20 >>>>>>>> The assertion failure in tree-cfg=2Ec got resolved, but I got ano= ther assertion failure in operands_scanner::get_expr_operands (tree *expr_p= , int flags), line 945: >>>>>>>>=20 >>>>>>>> 939 /* If we get here, something has gone wrong=2E */ >>>>>>>> 940 if (flag_checking) >>>>>>>> 941 { >>>>>>>> 942 fprintf (stderr, "unhandled expression in get_expr_oper= ands():\n"); >>>>>>>> 943 debug_tree (expr); >>>>>>>> 944 fputs ("\n", stderr); >>>>>>>> 945 gcc_unreachable (); >>>>>>>> 946 } >>>>>>>>=20 >>>>>>>> Looks like that the gimple statement: >>>>>>>> *arr=2E1 =3D =2EDEFERRED_INIT (D=2E1952, 2, 1); >>>>>>>>=20 >>>>>>>> Is not valid=2E i=2Ee, the LHS should not be an indirection to a= pointer=2E=20 >>>>>>>>=20 >>>>>>>> How to resolve this issue? >>>>>>=20 >>>>>> 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=2E >>>>>>=20 >>>>>> But I'm just guessing here =2E=2E=2E 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=2E >>>>>=20 >>>>> Yes, it=E2=80=99s an INDIRECT_REF at the point of the failure even t= hough I added a=20 >>>>>=20 >>>>> gimplify_var_or_parm_decl (lhs)=20 >>>>=20 >>>> I think the easiest is to build the =2EDEFERRED_INIT as GENERIC >>>> and use gimplify_assign () to gimplify and add the result >>>> to the sequence=2E Thus, build a GENERIC CALL_EXPR and then >>>> gimplify_assign (lhs, call_expr, seq); >>>=20 >>> Which utility routine is used to build an Internal generic call? >>> Currently, I used =E2=80=9Cgimple_build_call_internal=E2=80=9D to buil= d this internal gimple call=2E >>>=20 >>> For the generic call, shall I use =E2=80=9Cbuild_call_expr_loc=E2=80= =9D ?=20 >>=20 >> For example look at build_asan_poison_call_expr which does such thing >> for ASAN poison internal function call insertion at gimplification time= =2E >>=20 >> Richard=2E >>=20 >>> Qing >>>=20 >>>>=20 >>>> Richard=2E >>>>=20 >>>>> Qing >>>>>=20 >>>>>>=20 >>>>>>> I came up with the following solution: >>>>>>>=20 >>>>>>> Define the IFN_DEFERRED_INIT function as: >>>>>>>=20 >>>>>>> LHS =3D DEFERRED_INIT (SIZE of the DECL, INIT_TYPE, IS_VLA); >>>>>>>=20 >>>>>>> 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 create= d by >>>>>>> gimplify_vla_decl=2E >>>>>>>=20 >>>>>>>=20 >>>>>>> The benefit of this solution are: >>>>>>>=20 >>>>>>> 1=2E Resolved the invalid IR issue; >>>>>>> 2=2E The call stmt carries the address of the VLA natually; >>>>>>>=20 >>>>>>> The issue with this solution is: >>>>>>>=20 >>>>>>> For VLA and non-VLA, the LHS will be different,=20 >>>>>>>=20 >>>>>>> Do you see any other potential issues with this solution? >>>>>>>=20 >>>>>>> thanks=2E >>>>>>>=20 >>>>>>> Qing >>>>>>>=20 >>>>>>>=20 >>>>>>>=20 >>>>>>>=20 >>>>>>>=20 >>>>>>=20 >>>>>> --=20 >>>>>> Richard Biener >>>>>> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuern= berg, >>>>>> Germany; GF: Felix Imend=C3=B6rffer; HRB 36809 (AG Nuernberg) >>>>>=20 >>>>>=20 >>>>=20 >>>> --=20 >>>> Richard Biener >>>> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernbe= rg, >>>> Germany; GF: Felix Imend=C3=B6rffer; HRB 36809 (AG Nuernberg) >>>=20 >>>=20 >>=20 >> --=20 >> Richard Biener >> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg= , >> Germany; GF: Felix Imend=C3=B6rffer; HRB 36809 (AG Nuernberg) >