> On Jan 12, 2022, at 4:46 AM, Richard Biener <richard.guent...@gmail.com> > wrote: > > On Tue, Jan 11, 2022 at 5:32 PM Qing Zhao <qing.z...@oracle.com> wrote: >> >>>>> + /* Ignore the call to .DEFERRED_INIT that define the original >>>>> + var itself. */ >>>>> + if (is_gimple_assign (context)) >>>>> + { >>>>> + if (TREE_CODE (gimple_assign_lhs (context)) == VAR_DECL) >>>>> + lhs_var = gimple_assign_lhs (context); >>>>> + else if (TREE_CODE (gimple_assign_lhs (context)) == >>>>> SSA_NAME) >>>>> + lhs_var = SSA_NAME_VAR (gimple_assign_lhs (context)); >>>>> + } >>>>> + if (lhs_var >>>>> + && (lhs_var_name = DECL_NAME (lhs_var)) >>>>> + && (lhs_var_name_str = IDENTIFIER_POINTER (lhs_var_name)) >>>>> + && (strcmp (lhs_var_name_str, var_name_str) == 0)) >>>>> + return; >>>>> >>>>> likewise but I don't really understand what you are doing here. >>>> >>>> The above is to exclude the following case: >>>> >>>> temp = .DEFERRED_INIT (4, 2, “alt_reloc"); >>>> alt_reloc = temp; >>>> >>>> i.e, a call to .DEFERRED_INIT that define the original variable itself. >>> >>> How can this happen? It looks like a bug to me. Do you have a testcase? >> With -ftrivial-auto-var-init, During gimplification phase, almost all >> address taken variables that do not have an explicit initializer will have >> the above IR pattern. >> >> For example, gcc.dg/auto-init-uninit-16.c: >> >> [opc@qinzhao-ol8u3-x86 gcc]$ cat >> /home/opc/Work/GCC/latest-gcc/gcc/testsuite/gcc.dg/auto-init-uninit-16.c >> >> ****With -ftrivial-auto-var-init=zero, the IR after gimplification is: >> >> _1 = .DEFERRED_INIT (4, 2, &"alt_reloc"[0]); >> alt_reloc = _1; >> >> And the IR after SSA is similar as the above: >> >> _1 = .DEFERRED_INIT (4, 2, &"alt_reloc"[0]); >> alt_reloc = _1; >> >> During the early uninitialized analysis phase, the above IR will feed to the >> analyzer, we should exclude such >> IR from issuing fake warnings. > > Yes, but how do we get to a fake warning here? We should eventually > run into the _1 def being used > by alt_reloc = ...; and then by means of using the string "alt_reloc" > warn about an uninitialized use of > alt_reloc? Is it the point of the use that is "misdiagnosed"?
Yes, that’s the issue, the use point would be misdiagnosed without excluding this self definition chain. For the following: cat /home/opc/Work/GCC/latest-gcc/gcc/testsuite/gcc.dg/auto-init-uninit-16.c 1 /* { dg-do compile } */ 2 /* { dg-options "-O2 -Wuninitialized -ftrivial-auto-var-init=zero" } */ 3 4 int foo, bar; 5 6 static 7 void decode_reloc(int reloc, int *is_alt) 8 { 9 if (reloc >= 20) 10 *is_alt = 1; 11 else if (reloc >= 10) 12 *is_alt = 0; 13 } 14 15 void testfunc() 16 { 17 int alt_reloc; 18 19 decode_reloc(foo, &alt_reloc); 20 21 if (alt_reloc) /* { dg-warning "may be used uninitialized" "" } */ 22 bar = 42; 23 } If I deleted the following from tree-ssa-uninit.c: + /* Ignore the call to .DEFERRED_INIT that define the original + var itself. */ + if (is_gimple_assign (context)) + { + if (TREE_CODE (gimple_assign_lhs (context)) == VAR_DECL) + lhs_var = gimple_assign_lhs (context); + else if (TREE_CODE (gimple_assign_lhs (context)) == SSA_NAME) + lhs_var = SSA_NAME_VAR (gimple_assign_lhs (context)); + } + if (lhs_var + && (lhs_var_name = DECL_NAME (lhs_var)) + && (lhs_var_name_str = IDENTIFIER_POINTER (lhs_var_name)) + && (strcmp (lhs_var_name_str, var_name_str) == 0)) + return; We will get the following warning: /home/opc/Work/GCC/latest_gcc/gcc/testsuite/gcc.dg/auto-init-uninit-16.c: In function ‘testfunc’: /home/opc/Work/GCC/latest_gcc/gcc/testsuite/gcc.dg/auto-init-uninit-16.c:17:7: warning: ‘alt_reloc’ is used uninitialized [-Wuninitialized] In which the line number of the usage point “17:7” is wrong, which is the declaration point of the variable. This warning is issued during “pass_early_warn_uninitialized” for the following IR: _1 = .DEFERRED_INIT (4, 2, &"alt_reloc"[0]); alt_reloc = _1; The above usage point “alt_reloc = _1” actually is a fake usage, we should exclude this usage from issuing warning. The correct warning should be issued during “pass_late_warn_uninitialized” and for line 21 “if alt_reloc” for the following IR (the use point is “if (_1 != 0), which is for line 21: _1 = .DEFERRED_INIT (4, 2, &"alt_reloc"[0]); if (_1 != 0) Hope this is clear now. > I was > expecting the first patch to fix this. > > I'll wait for an update to the second patch. I will update it today. thanks. Qing > > Richard. > >> >>> >>>> >>>>> I'm >>>>> also not sure >>>>> I understand the case we try to fix with passing the name - is that >>>>> for VLA decls >>>>> that get replaced by allocation? >>>> >>>> This whole patch is mainly to resolve the issue that has been discussed >>>> last Aug as: >>>> >>>> https://gcc.gnu.org/pipermail/gcc-patches/2021-August/577431.html >>>> >>>> We have agreed at that time to resolve this issue later. >>> >>> Yes, I know. But the patch seems to do multiple things and there's no >>> new testcase >>> and the ChangeLog does not indicate the altered testcases are in any >>> way now fixed. >> >> That’s my bad, I realized this problem and separated the original patch into >> two separate patch and also added more detailed >> Description of the problem, hope this time the patch will be more clearer. >> >> You have approved the 1st patch. I will update it per your suggestion and >> commit to GCC12. >> >> For the 2nd one, I will fix the concern you raised about “repl_var”, and >> resubmit the patch. >> >> Qing