From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by sourceware.org (Postfix) with ESMTPS id 193CE385840A for ; Wed, 11 Aug 2021 16:55:26 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 193CE385840A 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-out1.suse.de (Postfix) with ESMTPS id DBD18221F0; Wed, 11 Aug 2021 16:55:24 +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 C39CD136D9; Wed, 11 Aug 2021 16:55:24 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap1.suse-dmz.suse.de with ESMTPSA id oWvALvwAFGHebAAAGKfGzw (envelope-from ); Wed, 11 Aug 2021 16:55:24 +0000 Date: Wed, 11 Aug 2021 18:55:22 +0200 From: Richard Biener To: Qing Zhao CC: Richard Sandiford , 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.5 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 16:55:36 -0000 On August 11, 2021 6:22:00 PM GMT+02:00, Qing Zhao wrote: > > >> On Aug 11, 2021, at 10:53 AM, Richard Biener wrot= e: >>=20 >> 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_TY= PE=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; >>>=20 >>> 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 (dec= l)); >>> gcc_assert (init_type > AUTO_INIT_UNINITIALIZED); >>> tree decl_size =3D TYPE_SIZE_UNIT (TREE_TYPE (decl)); >>>=20 >>> 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); >>>=20 >>> tree call =3D build_call_expr_internal_loc (UNKNOWN_LOCATION, IFN_DEFE= RRED_INIT, >>> TREE_TYPE (decl), 3, >>> decl_size, init_type_node, >>> is_vla_node); >>>=20 >>> /* 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 (d= ecl), >>> we should use it as the LHS of the call=2E */ >>>=20 >>> tree lhs_call >>> =3D is_vla ? DECL_VALUE_EXPR (decl) : decl; >>> gimplify_assign (lhs_call, call, seq_p); >>> } >>>=20 >>> With this change, the current issue is resolved, the gimple dump now i= s: >>>=20 >>> (*arr=2E1) =3D =2EDEFERRED_INIT (D=2E1952, 2, 1); >>>=20 >>> However, there is another new issue: >>>=20 >>> For the following testing case: >>>=20 >>> =3D=3D=3D=3D=3D=3D >>> [opc@qinzhao-ol8u3-x86 gcc]$ cat t=2Ec >>> int bar; >>>=20 >>> extern void decode_reloc(int *); >>>=20 >>> void testfunc() >>> { >>> int alt_reloc; >>>=20 >>> decode_reloc(&alt_reloc); >>>=20 >>> if (alt_reloc) /* { dg-warning "may be used uninitialized" } */ >>> bar =3D 42;=20 >>> } >>> =3D=3D=3D=3D=3D >>>=20 >>> In the above, the auto var =E2=80=9Calt_reloc=E2=80=9D is address take= n, then the gimple dump for it when compiled with -ftrivial-auto-var-init= =3Dzero is: >>>=20 >>> void testfunc () >>> { >>> int alt_reloc; >>>=20 >>> 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}; >>> } >>> } >>>=20 >>> I=2Ee, instead of the expected IR: >>>=20 >>> alt_reloc =3D =2EDEFERRED_INIT (4, 2, 0); >>>=20 >>> We got the following: >>>=20 >>> _1 =3D =2EDEFERRED_INIT (4, 2, 0); >>> alt_reloc =3D _1; >>>=20 >>> I guess the temp =E2=80=9C_1=E2=80=9D is created because =E2=80=9Calt_= reloc=E2=80=9D is address taken=2E=20 >>=20 >> Yes and no=2E The reason is that alt_reloc is memory (because it is add= ress taken) and that GIMPLE says that register typed stores need to use a i= s_gimple_val RHS which the call is not=2E > >Okay=2E >>=20 >>> My questions: >>>=20 >>> Shall we accept such IR for =2EDEFERRED_INIT purpose when the auto var= is address taken?=20 >>=20 >> I think so=2E Note it doesn't necessarily need address taking but any o= ther reason that prevents SSA rewriting the variable suffices=2E=20 > >You mean, in addition to =E2=80=9Caddress taken=E2=80=9D, there are other= situations that will introduce such IR: > >temp =3D =2EDEFERRED_INIT(); >auto_var =3D temp; > >So, such IR is unavoidable and we have to handle it? Yes=2E=20 >If we have to handle it, what=E2=80=99 the best way to do it? > >The solution in my mind is: >1=2E During uninitialized analysis phase, following the data flow to conn= ect =2EDEFERRED_INIT to =E2=80=9Cauto_var=E2=80=9D, and then decide that = =E2=80=9Cauto_var=E2=80=9D is uninitialized=2E Yes=2E Basically if there's an artificial variable auto initialized you ha= ve to look at its uses=2E=20 >2=2E During RTL expansion, following the data flow to connect =2EDEFERRED= _INIT to =E2=80=9Cauto_var=E2=80=9D, and then delete =E2=80=9Ctemp=E2=80=9D= , and then expand =2EDEFERRED_INIT to auto_var=2E That shouldn't be necessary=2E You'd initialize a temporary register which= is then copied to the real variable=2E That's good enough and should be op= timized by the RTL pipeline=2E=20 >Let me know your comments and suggestions on this=2E > > >>=20 >> The only other option is to force=2E DEFERED_INIT making the LHS addres= s taken which I think could be achieved by passing it the address as argume= nt instead of having a LHS=2E But let's not go down this route - it will ha= ve quite bad behavior on alias analysis and optimization=2E=20 > >Okay=2E > >Qing >>=20 >>> If so, =E2=80=9Cuninitialized analysis=E2=80=9D phase need to be furth= er adjusted to specially handle such IR=2E=20 >>>=20 >>> If not, what should we do when the auto var is address taken? >>>=20 >>> Thanks a lot=2E >>>=20 >>> Qing >>>=20 >>>=20 >>>> On Aug 11, 2021, at 8:58 AM, Richard Biener wro= te: >>>>=20 >>>> On Wed, 11 Aug 2021, Qing Zhao wrote: >>>>=20 >>>>>=20 >>>>>=20 >>>>>> On Aug 11, 2021, at 8:37 AM, Richard Biener w= rote: >>>>>>=20 >>>>>> On Wed, 11 Aug 2021, Qing Zhao wrote: >>>>>>=20 >>>>>>>=20 >>>>>>>=20 >>>>>>>> On Aug 11, 2021, at 2:02 AM, Richard Biener = wrote: >>>>>>>>=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 (thoug= h unlikely >>>>>>>>>>>>> since usually the receiver of initializations are simple eno= ugh)=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-tr= ee-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 =2E= DEFERRED_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_gi= mple_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 indirectio= n 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 a= nother 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_op= erands():\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_R= EF >>>>>>>> instead=2E >>>>>>>>=20 >>>>>>>> But I'm just guessing here =2E=2E=2E if you are in a debugger the= n 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= though 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 bu= ild 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 ti= me=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 crea= ted 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 Nue= rnberg, >>>>>>>> Germany; GF: Felix Imend=C3=B6rffer; HRB 36809 (AG Nuernberg) >>>>>>>=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 >