> On Jan 14, 2022, at 6:45 AM, Richard Biener <richard.guent...@gmail.com>
> wrote:
>
> On Thu, Jan 13, 2022 at 2:45 AM Qing Zhao <qing.z...@oracle.com> wrote:
>>
>> Hi, Richard,
>>
>> This is the updated version for the second patch, which is mainly the change
>> for "Enable -Wuninitialized + -ftrivial-auto-var-init for address taken
>> variables”.
>>
>> In this update, I mainly made the following change:
>>
>> 1. Delete “repl_var”, use the var_def_stmt, i.e, the call to .DEFERRED_INIT
>> to record the warning suppressed info.
>> 2. Add and change the comments in multiple places to make the change more
>> readable.
>>
>> Now, for the deleted variable, we will get the necessary info to report
>> warning from the call to .DEFERRED_INIT.
>>
>> A. the name string of DECL from the 3rd parameter of the call;
>> B. the location of the DECL from the location of the call;
>> C. the call can also be used to hold the information on whether the
>> warning
>> has been issued or not to suppress warning messages when needed;
>>
>> Please see the detailed description below for the problem and solution of
>> this patch.
>>
>> This patch has been bootstrapped and regressing tested on both X86 and
>> aarch64.
>>
>> Okay for GCC12?
>
> I think the change to split "%qD is used uninitialized" is bad for i8n
> though it seems
> like none of the strings passed to warn_uninit are currently marked
> for localization.
> I suppose there's lazy matching with the exact same strings passed to
> warning_at around like 641 but after your change those will no longer match
> up,
Silly question: What does the above “641” mean?
> at least for the "%qs ..." case constructed. I think a better way
> (for i8n) would be
> to pass down a flag whether it is "may" or "is" and have the full translatable
> strings literally passed to warning_at at the expense of some code
> duplication.
> Actually the extra flag is unnecessary, the OPT_W... we pass is already fully
> specifying that.
The only issue with the above change is: among the 4 calls to “warn_uninit”
as following:
if (use_stmt)
warn_uninit (OPT_Wuninitialized, def, SSA_NAME_VAR (def),
- "%qD is used uninitialized", use_stmt);
+ "is used uninitialized", use_stmt);
}
}
@@ -932,10 +1015,10 @@ warn_uninitialized_vars (bool wmaybe_uninit)
tree use = USE_FROM_PTR (use_p);
if (wlims.always_executed)
warn_uninit (OPT_Wuninitialized, use, SSA_NAME_VAR (use),
- "%qD is used uninitialized", stmt);
+ "is used uninitialized", stmt);
else if (wmaybe_uninit)
warn_uninit (OPT_Wmaybe_uninitialized, use, SSA_NAME_VAR (use),
- "%qD may be used uninitialized", stmt);
+ "may be used uninitialized", stmt);
}
/* For limiting the alias walk below we count all
@@ -1182,7 +1265,7 @@ warn_uninitialized_phi (gphi *phi, vec<gphi *> *worklist,
warn_uninit (OPT_Wmaybe_uninitialized, uninit_op,
SSA_NAME_VAR (uninit_op),
- "%qD may be used uninitialized in this function",
+ "may be used uninitialized in this function",
uninit_use_stmt, loc);
All the strings passed map well with the OPT_W… except the last one, since the
last one has an additional string “in this function” at the end.
However, since the last call has the last argument “loc” been NULL, maybe we
can use this to distinguish.
>
> Instead of doing
>
> + if (gimple_call_internal_p (var_def_stmt, IFN_DEFERRED_INIT))
> + {
> + /* Ignore the call to .DEFERRED_INIT that define the original
> + var itself as the following case:
> + temp = .DEFERRED_INIT (4, 2, â~@~\alt_reloc");
> + alt_reloc = temp;
> + In order to avoid generating warning for the fake usage
> + at alt_reloc = temp.
> ...
>
> I thought of many options, none really very appealing so I guess we have to
> go with this for now.
I agree with you, this is really ugly, I am not very comfortable myself either.
But looks like no better way to resolve it….
>
> So OK with the i8n thing sorted out - please post one hopefully last update
> for the patch.
Will do it.
>
> Thanks for your patience,
Thank you!
Qing
> Richard.
>
>> thanks.
>>
>> Qing.
>>
>> ========================
>> Enable -Wuninitialized + -ftrivial-auto-var-init for address
>> taken variables.
>>
>> With -ftrivial-auto-var-init, the address taken auto variable is replaced
>> with
>> a temporary variable during gimplification, and the original auto variable
>> might
>> be eliminated by compiler optimization completely. As a result, the current
>> uninitialized warning analysis cannot get enough information from the IR,
>> therefore the uninitialized warnings for address taken variable cannot be
>> issued based on the current implemenation of -ftrival-auto-var-init.
>>
>> For more info please refer to:
>> https://gcc.gnu.org/pipermail/gcc-patches/2021-August/577431.html
>>
>> In order to improve this situation, we can improve uninitialized analysis
>> for address taken auto variables with -ftrivial-auto-var-init as following:
>>
>> for the following stmt:
>>
>> _1 = .DEFERRED_INIT (4, 2, &"alt_reloc"[0]);
>> if (_1 != 0)
>>
>> The original variable DECL has been eliminated from the IR, all the necessary
>> information that is needed for reporting the warnings for DECL can be
>> acquired
>> from the call to .DEFERRED_INIT.
>>
>> A. the name string of DECL from the 3rd parameter of the call;
>> B. the location of the DECL from the location of the call;
>> C. the call can also be used to hold the information on whether the
>> warning
>> has been issued or not to suppress warning messages when needed;
>>
>> The current testing cases for uninitialized warnings +
>> -ftrivial-auto-var-init
>> are adjusted to reflect the fact that we can issue warnings for address taken
>> variables.
>>
>> gcc/ChangeLog:
>>
>> 2022-01-12 qing zhao <qing.z...@oracle.com>
>>
>> * tree-ssa-uninit.c (warn_uninit): Handle .DEFERRED_INIT call with an
>> anonymous SSA_NAME specially.
>> (check_defs): Likewise.
>> (warn_uninit_phi_uses): Adjust the message format for warn_uninit.
>> (warn_uninitialized_vars): Likewise.
>> (warn_uninitialized_phi): Likewise
>>
>> gcc/testsuite/ChangeLog:
>>
>> 2022-01-12 qing zhao <qing.z...@oracle.com>
>>
>> * gcc.dg/auto-init-uninit-16.c (testfunc): Delete xfail to reflect
>> the fact that address taken variable can be warned.
>> * gcc.dg/auto-init-uninit-34.c (warn_scalar_1): Likewise.
>> (warn_scalar_2): Likewise.
>> * gcc.dg/auto-init-uninit-37.c (T1): Likewise.
>> (T2): Likewise.
>> * gcc.dg/auto-init-uninit-B.c (baz): Likewise.
>>
>> The complete patch is attached:
