public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Qing Zhao <qing.zhao@oracle.com>
To: Richard Biener <rguenther@suse.de>
Cc: Richard Sandiford <richard.sandiford@arm.com>,
	Jakub Jelinek <jakub@redhat.com>,
	Nick Alcock via Gcc-patches <gcc-patches@gcc.gnu.org>,
	Kees Cook <keescook@chromium.org>
Subject: Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc
Date: Mon, 16 Aug 2021 16:48:38 +0000	[thread overview]
Message-ID: <FDE696B2-7988-42C6-8CE4-C6A6731BEA22@oracle.com> (raw)
In-Reply-To: <nycvar.YFH.7.76.2108160905191.11781@zhemvz.fhfr.qr>



> On Aug 16, 2021, at 2:11 AM, Richard Biener <rguenther@suse.de> wrote:
> 
> On Wed, 11 Aug 2021, Qing Zhao wrote:
> 
>> Hi, 
>> 
>> I met another issue for “address taken” auto variable, see below for details:
>> 
>> **** the testing case: (gcc/testsuite/gcc.dg/uninit-16.c)
>> 
>> int foo, bar;
>> 
>> static
>> void decode_reloc(int reloc, int *is_alt)
>> {
>>  if (reloc >= 20)
>>      *is_alt = 1;
>>  else if (reloc >= 10)
>>      *is_alt = 0;
>> }
>> 
>> void testfunc()
>> {
>>  int alt_reloc;
>> 
>>  decode_reloc(foo, &alt_reloc);
>> 
>>  if (alt_reloc) /* { dg-warning "may be used uninitialized" } */
>>    bar = 42;
>> }
>> 
>> ****When compiled with -ftrivial-auto-var-init=zero -O2 -Wuninitialized -fdump-tree-all:
>> 
>> .*************gimple dump:
>> 
>> void testfunc ()
>> { 
>>  int alt_reloc;
>> 
>>  try
>>    {
>>      _1 = .DEFERRED_INIT (4, 2, 0);
>>      alt_reloc = _1;
>>      foo.0_2 = foo;
>>      decode_reloc (foo.0_2, &alt_reloc);
>>      alt_reloc.1_3 = alt_reloc;
>>      if (alt_reloc.1_3 != 0) goto <D.1952>; else goto <D.1953>;
>>      <D.1952>:
>>      bar = 42;
>>      <D.1953>:
>>    }
>>  finally
>>    {
>>      alt_reloc = {CLOBBER};
>>    }
>> }
>> 
>> **************fre1 dump:
>> 
>> void testfunc ()
>> {
>>  int alt_reloc;
>>  int _1;
>>  int foo.0_2;
>> 
>>  <bb 2> :
>>  _1 = .DEFERRED_INIT (4, 2, 0);
>>  foo.0_2 = foo;
>>  if (foo.0_2 > 19)
>>    goto <bb 3>; [50.00%]
>>  else
>>    goto <bb 4>; [50.00%]
>> 
>>  <bb 3> :
>>  goto <bb 7>; [100.00%]
>> 
>>  <bb 4> :
>>  if (foo.0_2 > 9)
>>    goto <bb 5>; [50.00%]
>>  else
>>    goto <bb 6>; [50.00%]
>> 
>>  <bb 5> :
>>  goto <bb 8>; [100.00%]
>> 
>>  <bb 6> :
>>  if (_1 != 0)
>>    goto <bb 7>; [INV]
>>  else
>>    goto <bb 8>; [INV]
>> 
>>  <bb 7> :
>>  bar = 42;
>> 
>>  <bb 8> :
>>  return;
>> 
>> }
>> 
>> From the above IR file after “FRE”, we can see that the major issue with this IR is:
>> 
>> The address taken auto variable “alt_reloc” has been completely replaced by the temporary variable “_1” in all
>> the uses of the original “alt_reloc”. 
> 
> Well, this can happen with regular code as well, there's no need for
> .DEFERRED_INIT.  This is the usual problem with reporting uninitialized
> uses late.
> 
> IMHO this shouldn't be a blocker.  The goal of zero "regressions" wrt
> -Wuninitialized isn't really achievable.

Okay. Sounds reasonable to me too.

> 
>> The major problem with such IR is,  during uninitialized analysis phase, the original use of “alt_reloc” disappeared completely.
>> So, the warning cannot be reported.
>> 
>> 
>> My questions:
>> 
>> 1. Is it possible to get the original “alt_reloc” through the temporary variable “_1” with some available information recorded in the IR?
>> 2. If not, then we have to record the relationship between “alt_reloc” and “_1” when the original “alt_reloc” is replaced by “_1” and get such relationship during
>>    Uninitialized analysis phase.  Is this doable?
> 
> Well, you could add a fake argument to .DEFERRED_INIT for the purpose of
> diagnostics.  The difficulty is to avoid tracking it as actual use so
> you could for example pass a string with the declarations name though
> this wouldn't give the association with the actual decl.
Good suggestion, I can try this a little bit. 

> 
>> 3. Looks like that for “address taken” auto variable, if we have to introduce a new temporary variable and split the call to .DEFERRED_INIT into two:
>> 
>>      temp = .DEFERRED_INIT (4, 2, 0);
>>      alt_reloc = temp;
>> 
>>   More issues might possible.
>> 
>> Any comments and suggestions on this issue?
> 
> I don't see any good possibilities that would not make optimizing code
> as good as w/o .DEFERRED_INIT more difficult.  My stake here is always
> that GCC is an optimizing compiler, not a static analysis engine and
> thus I side with "broken" diagnostics and better optimization.
That’s true and reasonable, too.

thanks.

Qing
> 
> Richard.
> 
>> Qing
>> 
>> j
>>> On Aug 11, 2021, at 11:55 AM, Richard Biener <rguenther@suse.de> wrote:
>>> 
>>> On August 11, 2021 6:22:00 PM GMT+02:00, Qing Zhao <qing.zhao@oracle.com> wrote:
>>>> 
>>>> 
>>>>> On Aug 11, 2021, at 10:53 AM, Richard Biener <rguenther@suse.de> wrote:
>>>>> 
>>>>> On August 11, 2021 5:30:40 PM GMT+02:00, Qing Zhao <qing.zhao@oracle.com> wrote:
>>>>>> I modified the routine “gimple_add_init_for_auto_var” as the following:
>>>>>> ====
>>>>>> /* Generate initialization to automatic variable DECL based on INIT_TYPE.
>>>>>> 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).  */
>>>>>> 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 = TYPE_SIZE_UNIT (TREE_TYPE (decl));
>>>>>> 
>>>>>> tree init_type_node
>>>>>> = build_int_cst (integer_type_node, (int) init_type);
>>>>>> tree is_vla_node
>>>>>> = build_int_cst (integer_type_node, (int) is_vla);
>>>>>> 
>>>>>> tree call = build_call_expr_internal_loc (UNKNOWN_LOCATION, IFN_DEFERRED_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 (decl),
>>>>>>  we should use it as the LHS of the call.  */
>>>>>> 
>>>>>> tree lhs_call
>>>>>> = 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.1) = .DEFERRED_INIT (D.1952, 2, 1);
>>>>>> 
>>>>>> However, there is another new issue:
>>>>>> 
>>>>>> For the following testing case:
>>>>>> 
>>>>>> ======
>>>>>> [opc@qinzhao-ol8u3-x86 gcc]$ cat t.c
>>>>>> 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 = 42; 
>>>>>> }
>>>>>> =====
>>>>>> 
>>>>>> In the above, the auto var “alt_reloc” is address taken, then the gimple dump for it when compiled with -ftrivial-auto-var-init=zero is:
>>>>>> 
>>>>>> void testfunc ()
>>>>>> {
>>>>>> int alt_reloc;
>>>>>> 
>>>>>> try
>>>>>> {
>>>>>>   _1 = .DEFERRED_INIT (4, 2, 0);
>>>>>>   alt_reloc = _1;
>>>>>>   decode_reloc (&alt_reloc);
>>>>>>   alt_reloc.0_2 = alt_reloc;
>>>>>>   if (alt_reloc.0_2 != 0) goto <D.1949>; else goto <D.1950>;
>>>>>>   <D.1949>:
>>>>>>   bar = 42;
>>>>>>   <D.1950>:
>>>>>> }
>>>>>> finally
>>>>>> {
>>>>>>   alt_reloc = {CLOBBER};
>>>>>> }
>>>>>> }
>>>>>> 
>>>>>> I.e, instead of the expected IR:
>>>>>> 
>>>>>> alt_reloc = .DEFERRED_INIT (4, 2, 0);
>>>>>> 
>>>>>> We got the following:
>>>>>> 
>>>>>> _1 = .DEFERRED_INIT (4, 2, 0);
>>>>>>   alt_reloc = _1;
>>>>>> 
>>>>>> I guess the temp “_1” is created because “alt_reloc” is address taken. 
>>>>> 
>>>>> Yes and no. The reason is that alt_reloc is memory (because it is address taken) and that GIMPLE says that register typed stores need to use a is_gimple_val RHS which the call is not.
>>>> 
>>>> Okay.
>>>>> 
>>>>>> My questions:
>>>>>> 
>>>>>> Shall we accept such IR for .DEFERRED_INIT purpose when the auto var is address taken? 
>>>>> 
>>>>> I think so. Note it doesn't necessarily need address taking but any other reason that prevents SSA rewriting the variable suffices. 
>>>> 
>>>> You mean, in addition to “address taken”, there are other situations that will introduce such IR:
>>>> 
>>>> temp = .DEFERRED_INIT();
>>>> auto_var = temp;
>>>> 
>>>> So, such IR is unavoidable and we have to handle it?
>>> 
>>> Yes. 
>>> 
>>>> If we have to handle it,  what’ the best way to do it?
>>>> 
>>>> The solution in my mind is:
>>>> 1. During uninitialized analysis phase, following the data flow to connect .DEFERRED_INIT to “auto_var”, and then decide that “auto_var” is uninitialized.
>>> 
>>> Yes. Basically if there's an artificial variable auto initialized you have to look at its uses. 
>>> 
>>>> 2. During RTL expansion, following the data flow to connect .DEFERRED_INIT to “auto_var”, and then delete “temp”, and then expand .DEFERRED_INIT to auto_var.
>>> 
>>> That shouldn't be necessary. You'd initialize a temporary register which is then copied to the real variable. That's good enough and should be optimized by the RTL pipeline. 
>>> 
>>>> Let me know your comments and suggestions on this.
>>>> 
>>>> 
>>>>> 
>>>>> The only other option is to force. DEFERED_INIT making the LHS address taken which I think could be achieved by passing it the address as argument instead of having a LHS. But let's not go down this route - it will have quite bad behavior on alias analysis and optimization. 
>>>> 
>>>> Okay.
>>>> 
>>>> Qing
>>>>> 
>>>>>> If so, “uninitialized analysis” phase need to be further adjusted to specially handle such IR. 
>>>>>> 
>>>>>> If not, what should we do when the auto var is address taken?
>>>>>> 
>>>>>> Thanks a lot.
>>>>>> 
>>>>>> Qing
>>>>>> 
>>>>>> 
>>>>>>> On Aug 11, 2021, at 8:58 AM, Richard Biener <rguenther@suse.de> wrote:
>>>>>>> 
>>>>>>> On Wed, 11 Aug 2021, Qing Zhao wrote:
>>>>>>> 
>>>>>>>> 
>>>>>>>> 
>>>>>>>>> On Aug 11, 2021, at 8:37 AM, Richard Biener <rguenther@suse.de> wrote:
>>>>>>>>> 
>>>>>>>>> On Wed, 11 Aug 2021, Qing Zhao wrote:
>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>>>>> On Aug 11, 2021, at 2:02 AM, Richard Biener <rguenther@suse.de> wrote:
>>>>>>>>>>> 
>>>>>>>>>>> On Tue, 10 Aug 2021, Qing Zhao wrote:
>>>>>>>>>>> 
>>>>>>>>>>>> 
>>>>>>>>>>>> 
>>>>>>>>>>>>> On Aug 10, 2021, at 3:16 PM, Qing Zhao via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
>>>>>>>>>>>>> 
>>>>>>>>>>>>> Hi, Richard,
>>>>>>>>>>>>> 
>>>>>>>>>>>>>> On Aug 10, 2021, at 10:22 AM, Richard Biener <rguenther@suse.de> 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.
>>>>>>>>>> 
>>>>>>>>>> Yes, it’s an INDIRECT_REF at the point of the failure even though I added a 
>>>>>>>>>> 
>>>>>>>>>> gimplify_var_or_parm_decl  (lhs) 
>>>>>>>>> 
>>>>>>>>> I think the easiest is to build the .DEFERRED_INIT as GENERIC
>>>>>>>>> and use gimplify_assign () to gimplify and add the result
>>>>>>>>> to the sequence.  Thus, build a GENERIC CALL_EXPR and then
>>>>>>>>> gimplify_assign (lhs, call_expr, seq);
>>>>>>>> 
>>>>>>>> Which utility routine is used to build an Internal generic call?
>>>>>>>> Currently, I used “gimple_build_call_internal” to build this internal gimple call.
>>>>>>>> 
>>>>>>>> For the generic call, shall I use “build_call_expr_loc” ? 
>>>>>>> 
>>>>>>> For example look at build_asan_poison_call_expr which does such thing
>>>>>>> for ASAN poison internal function call insertion at gimplification time.
>>>>>>> 
>>>>>>> Richard.
>>>>>>> 
>>>>>>>> Qing
>>>>>>>> 
>>>>>>>>> 
>>>>>>>>> Richard.
>>>>>>>>> 
>>>>>>>>>> Qing
>>>>>>>>>> 
>>>>>>>>>>> 
>>>>>>>>>>>> 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 <rguenther@suse.de>
>>>>>>>>>>> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
>>>>>>>>>>> Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
>>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>>> 
>>>>>>>>> -- 
>>>>>>>>> Richard Biener <rguenther@suse.de>
>>>>>>>>> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
>>>>>>>>> Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
>>>>>>>> 
>>>>>>>> 
>>>>>>> 
>>>>>>> -- 
>>>>>>> Richard Biener <rguenther@suse.de>
>>>>>>> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
>>>>>>> Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
>> 
>> 
> 
> -- 
> Richard Biener <rguenther@suse.de>
> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
> Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)


  reply	other threads:[~2021-08-16 16:48 UTC|newest]

Thread overview: 60+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-27  3:26 Qing Zhao
2021-07-28 20:21 ` Kees Cook
2021-07-28 21:53   ` Qing Zhao
2021-08-09 14:09 ` Richard Biener
2021-08-09 16:38   ` Qing Zhao
2021-08-09 17:14     ` Richard Biener
2021-08-10  7:36     ` Richard Biener
2021-08-10 13:39       ` Qing Zhao
2021-08-10 14:16         ` Richard Biener
2021-08-10 15:02           ` Qing Zhao
2021-08-10 15:22             ` Richard Biener
2021-08-10 15:55               ` Qing Zhao
2021-08-10 20:16               ` Qing Zhao
2021-08-10 22:26                 ` Qing Zhao
2021-08-11  7:02                   ` Richard Biener
2021-08-11 13:33                     ` Qing Zhao
2021-08-11 13:37                       ` Richard Biener
2021-08-11 13:54                         ` Qing Zhao
2021-08-11 13:58                           ` Richard Biener
2021-08-11 14:00                             ` Qing Zhao
2021-08-11 15:30                             ` Qing Zhao
2021-08-11 15:53                               ` Richard Biener
2021-08-11 16:22                                 ` Qing Zhao
2021-08-11 16:55                                   ` Richard Biener
2021-08-11 16:57                                     ` Qing Zhao
2021-08-11 20:30                                     ` Qing Zhao
2021-08-11 22:03                                       ` Qing Zhao
2021-08-16  7:12                                         ` Richard Biener
2021-08-16 14:48                                           ` Qing Zhao
2021-08-16 15:08                                             ` Richard Biener
2021-08-16 15:39                                               ` Qing Zhao
2021-08-16  7:11                                       ` Richard Biener
2021-08-16 16:48                                         ` Qing Zhao [this message]
2021-08-17 15:04                                           ` Qing Zhao
2021-08-17 20:40                                             ` Qing Zhao
2021-08-18  7:19                                               ` Richard Biener
2021-08-18 14:39                                                 ` Qing Zhao
2021-08-11  9:02                   ` Richard Sandiford
2021-08-11 13:44                     ` Qing Zhao
2021-08-11 16:15                       ` Richard Sandiford
2021-08-11 16:29                         ` Qing Zhao
2021-08-12 19:24   ` Qing Zhao
2021-08-12 22:45     ` Qing Zhao
2021-08-16  7:40     ` Richard Biener
2021-08-16 15:45       ` Qing Zhao
2021-08-17  8:29         ` Richard Biener
2021-08-17 14:50           ` Qing Zhao
2021-08-17 16:08             ` Qing Zhao
2021-08-18  7:15               ` Richard Biener
2021-08-18 16:02                 ` Qing Zhao
2021-08-19  9:00                   ` Richard Biener
2021-08-19 13:54                     ` Qing Zhao
2021-08-20 14:52                       ` Qing Zhao
2021-08-23 13:55                       ` Richard Biener
2021-09-02 17:24                         ` Qing Zhao
2021-08-16 19:49       ` Qing Zhao
2021-08-17  8:43         ` Richard Biener
2021-08-17 14:03           ` Qing Zhao
2021-08-17 14:45             ` Richard Biener
2021-08-17 14:53               ` Qing Zhao

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=FDE696B2-7988-42C6-8CE4-C6A6731BEA22@oracle.com \
    --to=qing.zhao@oracle.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jakub@redhat.com \
    --cc=keescook@chromium.org \
    --cc=rguenther@suse.de \
    --cc=richard.sandiford@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).