On 1/14/22 09:30, Qing Zhao wrote:
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 around line 641 :)
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.
Now that diagnostics are prefixed by something like "In function 'foo':
the "in this function" part is superfluous and could be removed from
all warning messages.
When there's no location (i.e., loc is UNKNOWN_LOCATION) the called
code tries to derive a location from the context. When it can't, it
won't point anywhere useful, so if that case ever comes up here it
should probably be handled by using the location of the curly brace
closing the function definition.
Martin
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:
