* [Patch][V4][Patch 2/2]Enable -Wuninitialized + -ftrivial-auto-var-init for address taken variables.
@ 2022-01-13 1:45 Qing Zhao
2022-01-14 12:45 ` Richard Biener
0 siblings, 1 reply; 7+ messages in thread
From: Qing Zhao @ 2022-01-13 1:45 UTC (permalink / raw)
To: Richard Biener; +Cc: Martin Sebor, gcc Patches
[-- Attachment #1: Type: text/plain, Size: 3517 bytes --]
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?
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.zhao@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.zhao@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:
[-- Attachment #2: 0001-Enable-Wuninitialized-ftrivial-auto-var-init-for-add.patch --]
[-- Type: application/octet-stream, Size: 18643 bytes --]
From 2357b6a5224131169fcc7ee54cdc9243ebd3d231 Mon Sep 17 00:00:00 2001
From: Qing Zhao <qing.zhao@oracle.com>
Date: Wed, 12 Jan 2022 22:40:40 +0000
Subject: [PATCH] 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.zhao@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.zhao@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.
---
gcc/testsuite/gcc.dg/auto-init-uninit-16.c | 4 +-
gcc/testsuite/gcc.dg/auto-init-uninit-34.c | 8 +-
gcc/testsuite/gcc.dg/auto-init-uninit-37.c | 44 ++++----
gcc/testsuite/gcc.dg/auto-init-uninit-B.c | 4 +-
gcc/tree-ssa-uninit.c | 125 +++++++++++++++++----
5 files changed, 130 insertions(+), 55 deletions(-)
diff --git a/gcc/testsuite/gcc.dg/auto-init-uninit-16.c b/gcc/testsuite/gcc.dg/auto-init-uninit-16.c
index 38e19502a679..f14864be901f 100644
--- a/gcc/testsuite/gcc.dg/auto-init-uninit-16.c
+++ b/gcc/testsuite/gcc.dg/auto-init-uninit-16.c
@@ -1,7 +1,5 @@
/* { dg-do compile } */
/* { dg-options "-O2 -Wuninitialized -ftrivial-auto-var-init=zero" } */
-/* -ftrivial-auto-var-init will make the uninitialized warning for address
- taken auto var going away, FIXME later. */
int foo, bar;
@@ -20,6 +18,6 @@ void testfunc()
decode_reloc(foo, &alt_reloc);
- if (alt_reloc) /* { dg-warning "may be used uninitialized" "" { xfail *-*-* } } */
+ if (alt_reloc) /* { dg-warning "may be used uninitialized" "" } */
bar = 42;
}
diff --git a/gcc/testsuite/gcc.dg/auto-init-uninit-34.c b/gcc/testsuite/gcc.dg/auto-init-uninit-34.c
index 1a687654ebb7..d6e7ed3e8600 100644
--- a/gcc/testsuite/gcc.dg/auto-init-uninit-34.c
+++ b/gcc/testsuite/gcc.dg/auto-init-uninit-34.c
@@ -4,8 +4,6 @@
to functions declared with attribute access is diagnosed where expected.
{ dg-do compile }
{ dg-options "-O -Wall -ftrivial-auto-var-init=zero" } */
-/* -ftrivial-auto-var-init will make the uninitialized warning for address
- taken auto var going away, FIXME later. */
#define RW(...) __attribute__ ((access (read_write, __VA_ARGS__)))
@@ -21,10 +19,10 @@ void nowarn_scalar (void)
void warn_scalar_1 (void)
{
- int i1; // { dg-message "declared here" "" { xfail *-*-* } }
+ int i1; // { dg-message "declared here" "" }
int i2, i3 = 1, i4;
- f4pi (&i1, &i2, &i3, &i4); // { dg-warning "'i1' may be used uninitialized" "" { xfail *-*-* } }
+ f4pi (&i1, &i2, &i3, &i4); // { dg-warning "'i1' may be used uninitialized" "" }
}
void warn_scalar_2 (void)
@@ -32,7 +30,7 @@ void warn_scalar_2 (void)
int j1 = 0, j2, j4;
int j3;
- f4pi (&j1, &j2, &j3, &j4); // { dg-warning "'j3' may be used uninitialized" "" { xfail *-*-* } }
+ f4pi (&j1, &j2, &j3, &j4); // { dg-warning "'j3' may be used uninitialized" "" }
}
diff --git a/gcc/testsuite/gcc.dg/auto-init-uninit-37.c b/gcc/testsuite/gcc.dg/auto-init-uninit-37.c
index 2791b37ca009..aea554262f8e 100644
--- a/gcc/testsuite/gcc.dg/auto-init-uninit-37.c
+++ b/gcc/testsuite/gcc.dg/auto-init-uninit-37.c
@@ -5,8 +5,6 @@
arguments of array, VLA, or pointer types.
{ dg-do compile }
{ dg-options "-O2 -Wall -ftrack-macro-expansion=0 -ftrivial-auto-var-init=zero" } */
-/* -ftrivial-auto-var-init will make the uninitialized warning for address
- taken auto var going away, FIXME later. */
#define NONE /* none */
#define RO(...) __attribute__ ((access (read_only, __VA_ARGS__)))
@@ -42,9 +40,9 @@ typedef int IA_[];
typedef const int CIA_[];
T1 (NONE, fia_, IA_);
-T1 (NONE, fcia_, CIA_); // { dg-warning "\\\[-Wmaybe-uninitialized" "" { xfail *-*-* } }
-T1 (RO (1), froia_, IA_); // { dg-warning "\\\[-Wuninitialized" "" { xfail *-*-* } }
-T1 (RW (1), frwia_, IA_); // { dg-warning "\\\[-Wmaybe-uninitialized" "" { xfail *-*-* } }
+T1 (NONE, fcia_, CIA_); // { dg-warning "\\\[-Wmaybe-uninitialized" "" }
+T1 (RO (1), froia_, IA_); // { dg-warning "\\\[-Wuninitialized" "" }
+T1 (RW (1), frwia_, IA_); // { dg-warning "\\\[-Wmaybe-uninitialized" "" }
T1 (WO (1), fwoia_, IA_);
T1 (X (1), fxia_, IA_);
@@ -53,9 +51,9 @@ typedef int IA1[1];
typedef const int CIA1[1];
T1 (NONE, fia1, IA1);
-T1 (NONE, fcia1, CIA1); // { dg-warning "\\\[-Wmaybe-uninitialized" "" { xfail *-*-* } }
-T1 (RO (1), froia1, IA1); // { dg-warning "\\\[-Wuninitialized" "" { xfail *-*-* } }
-T1 (RW (1), frwia1, IA1); // { dg-warning "\\\[-Wmaybe-uninitialized" "" { xfail *-*-* } }
+T1 (NONE, fcia1, CIA1); // { dg-warning "\\\[-Wmaybe-uninitialized" "" }
+T1 (RO (1), froia1, IA1); // { dg-warning "\\\[-Wuninitialized" "" }
+T1 (RW (1), frwia1, IA1); // { dg-warning "\\\[-Wmaybe-uninitialized" "" }
T1 (WO (1), fwoia1, IA1);
T1 (X (1), fxia1, IA1);
@@ -64,9 +62,9 @@ T1 (X (1), fxia1, IA1);
#define CIARS1 const int[restrict static 1]
T1 (NONE, fiars1, IARS1);
-T1 (NONE, fciars1, CIARS1);// { dg-warning "\\\[-Wmaybe-uninitialized" "" { xfail *-*-* } }
-T1 (RO (1), froiars1, IARS1); // { dg-warning "\\\[-Wuninitialized" "" { xfail *-*-* } }
-T1 (RW (1), frwiars1, IARS1); // { dg-warning "\\\[-Wmaybe-uninitialized" "" { xfail *-*-* } }
+T1 (NONE, fciars1, CIARS1);// { dg-warning "\\\[-Wmaybe-uninitialized" "" }
+T1 (RO (1), froiars1, IARS1); // { dg-warning "\\\[-Wuninitialized" "" }
+T1 (RW (1), frwiars1, IARS1); // { dg-warning "\\\[-Wmaybe-uninitialized" "" }
T1 (WO (1), fwoiars1, IARS1);
T1 (X (1), fxiars1, IARS1);
@@ -75,9 +73,9 @@ T1 (X (1), fxiars1, IARS1);
#define CIAS1 const int[static 1]
T1 (NONE, fias1, IAS1);
-T1 (NONE, fcias1, CIAS1); // { dg-warning "\\\[-Wmaybe-uninitialized" "" { xfail *-*-* } }
-T1 (RO (1), froias1, IAS1); // { dg-warning "\\\[-Wuninitialized" "" { xfail *-*-* } }
-T1 (RW (1), frwias1, IAS1); // { dg-warning "\\\[-Wmaybe-uninitialized" "" { xfail *-*-* } }
+T1 (NONE, fcias1, CIAS1); // { dg-warning "\\\[-Wmaybe-uninitialized" "" }
+T1 (RO (1), froias1, IAS1); // { dg-warning "\\\[-Wuninitialized" "" }
+T1 (RW (1), frwias1, IAS1); // { dg-warning "\\\[-Wmaybe-uninitialized" "" }
T1 (WO (1), fwoias1, IAS1);
T1 (X (1), fxias1, IAS1);
@@ -86,9 +84,9 @@ T1 (X (1), fxias1, IAS1);
#define CIAX const int[*]
T1 (NONE, fiax, IAX);
-T1 (NONE, fciax, CIAX); // { dg-warning "\\\[-Wmaybe-uninitialized" "" { xfail *-*-* } }
-T1 (RO (1), froiax, IAX); // { dg-warning "\\\[-Wuninitialized" "" { xfail *-*-* } }
-T1 (RW (1), frwiax, IAX); // { dg-warning "\\\[-Wmaybe-uninitialized" "" { xfail *-*-* } }
+T1 (NONE, fciax, CIAX); // { dg-warning "\\\[-Wmaybe-uninitialized" "" }
+T1 (RO (1), froiax, IAX); // { dg-warning "\\\[-Wuninitialized" "" }
+T1 (RW (1), frwiax, IAX); // { dg-warning "\\\[-Wmaybe-uninitialized" "" }
T1 (WO (1), fwoiax, IAX);
T1 (X (1), fxiax, IAX);
@@ -97,9 +95,9 @@ T1 (X (1), fxiax, IAX);
#define CIAN int n, const int[n]
T2 (NONE, fian, IAN);
-T2 (NONE, fcian, CIAN); // { dg-warning "\\\[-Wmaybe-uninitialized" "" { xfail *-*-* } }
-T2 (RO (2, 1), froian, IAN); // { dg-warning "\\\[-Wuninitialized" "" { xfail *-*-* } }
-T2 (RW (2, 1), frwian, IAN); // { dg-warning "\\\[-Wmaybe-uninitialized" "" { xfail *-*-* } }
+T2 (NONE, fcian, CIAN); // { dg-warning "\\\[-Wmaybe-uninitialized" "" }
+T2 (RO (2, 1), froian, IAN); // { dg-warning "\\\[-Wuninitialized" "" }
+T2 (RW (2, 1), frwian, IAN); // { dg-warning "\\\[-Wmaybe-uninitialized" "" }
T2 (WO (2, 1), fwoian, IAN);
T2 (X (2, 1), fxian, IAN);
@@ -108,9 +106,9 @@ typedef int* IP;
typedef const int* CIP;
T1 (NONE, fip, IP);
-T1 (NONE, fcip, CIP); // { dg-warning "\\\[-Wmaybe-uninitialized" "" { xfail *-*-* } }
-T1 (RO (1), froip, IP); // { dg-warning "\\\[-Wuninitialized" "" { xfail *-*-* } }
-T1 (RW (1), frwip, IP); // { dg-warning "\\\[-Wmaybe-uninitialized" "" { xfail *-*-* } }
+T1 (NONE, fcip, CIP); // { dg-warning "\\\[-Wmaybe-uninitialized" "" }
+T1 (RO (1), froip, IP); // { dg-warning "\\\[-Wuninitialized" "" }
+T1 (RW (1), frwip, IP); // { dg-warning "\\\[-Wmaybe-uninitialized" "" }
T1 (WO (1), fwoip, IP);
T1 (X (1), fxip, IP);
diff --git a/gcc/testsuite/gcc.dg/auto-init-uninit-B.c b/gcc/testsuite/gcc.dg/auto-init-uninit-B.c
index b6d3efdfb880..40d3196ea47f 100644
--- a/gcc/testsuite/gcc.dg/auto-init-uninit-B.c
+++ b/gcc/testsuite/gcc.dg/auto-init-uninit-B.c
@@ -1,7 +1,5 @@
/* Origin: PR c/179 from Gray Watson <gray@256.com>, adapted as a testcase
by Joseph Myers <jsm28@cam.ac.uk>. */
-/* -ftrivial-auto-var-init will make the uninitialized warning for address
- taken auto var going away, FIXME later. */
/* { dg-do compile } */
/* { dg-options "-O2 -Wuninitialized -ftrivial-auto-var-init=zero" } */
extern void foo (int *);
@@ -11,7 +9,7 @@ void
baz (void)
{
int i;
- if (i) /* { dg-warning "is used uninitialized" "uninit i warning" { xfail *-*-* } } */
+ if (i) /* { dg-warning "is used uninitialized" "uninit i warning" } */
bar (i);
foo (&i);
}
diff --git a/gcc/tree-ssa-uninit.c b/gcc/tree-ssa-uninit.c
index 7a6df30c19e8..48bf3216b583 100644
--- a/gcc/tree-ssa-uninit.c
+++ b/gcc/tree-ssa-uninit.c
@@ -182,24 +182,70 @@ warn_uninit (opt_code opt, tree t, tree var, const char *gmsgid,
}
/* Anonymous SSA_NAMEs shouldn't be uninitialized, but ssa_undefined_value_p
- can return true if the def stmt of an anonymous SSA_NAME is COMPLEX_EXPR
- created for conversion from scalar to complex. Use the underlying var of
- the COMPLEX_EXPRs real part in that case. See PR71581. */
+ can return true if the def stmt of an anonymous SSA_NAME is
+ 1. A COMPLEX_EXPR created for conversion from scalar to complex. Use the
+ underlying var of the COMPLEX_EXPRs real part in that case. See PR71581.
+
+ Or
+
+ 2. A call to .DEFERRED_INIT internal function. Since the original variable
+ has been eliminated by optimziation, we need to get the variable name,
+ and variable declaration location from this call. We recorded variable
+ name into VAR_NAME_STR, and will get location info and record warning
+ suppressed info to VAR_DEF_STMT, which is the .DEFERRED_INIT call. */
+
+ const char *var_name_str = NULL;
+ gimple *var_def_stmt = NULL;
+
if (!var && !SSA_NAME_VAR (t))
{
- gimple *def_stmt = SSA_NAME_DEF_STMT (t);
- if (is_gimple_assign (def_stmt)
- && gimple_assign_rhs_code (def_stmt) == COMPLEX_EXPR)
+ var_def_stmt = SSA_NAME_DEF_STMT (t);
+
+ if (is_gimple_assign (var_def_stmt)
+ && gimple_assign_rhs_code (var_def_stmt) == COMPLEX_EXPR)
{
- tree v = gimple_assign_rhs1 (def_stmt);
+ tree v = gimple_assign_rhs1 (var_def_stmt);
if (TREE_CODE (v) == SSA_NAME
&& has_undefined_value_p (v)
- && zerop (gimple_assign_rhs2 (def_stmt)))
+ && zerop (gimple_assign_rhs2 (var_def_stmt)))
var = SSA_NAME_VAR (v);
}
+
+ 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.
+ */
+ tree lhs_var = NULL_TREE;
+ tree lhs_var_name = NULL_TREE;
+ const char *lhs_var_name_str = NULL;
+
+ /* Get the variable name from the 3rd argument of call. */
+ tree var_name = gimple_call_arg (var_def_stmt, 2);
+ var_name = TREE_OPERAND (TREE_OPERAND (var_name, 0), 0);
+ var_name_str = TREE_STRING_POINTER (var_name);
+
+ 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;
+ gcc_assert (var_name_str && var_def_stmt);
+ }
}
- if (var == NULL_TREE)
+ if (var == NULL_TREE && var_name_str == NULL)
return;
/* Avoid warning if we've already done so or if the warning has been
@@ -207,36 +253,59 @@ warn_uninit (opt_code opt, tree t, tree var, const char *gmsgid,
if (((warning_suppressed_p (context, OPT_Wuninitialized)
|| (gimple_assign_single_p (context)
&& get_no_uninit_warning (gimple_assign_rhs1 (context)))))
- || get_no_uninit_warning (var))
+ || (var && get_no_uninit_warning (var))
+ || (var_name_str
+ && warning_suppressed_p (var_def_stmt, OPT_Wuninitialized)))
return;
/* Use either the location of the read statement or that of the PHI
argument, or that of the uninitialized variable, in that order,
whichever is valid. */
- location_t location;
+ location_t location = UNKNOWN_LOCATION;
if (gimple_has_location (context))
location = gimple_location (context);
else if (phi_arg_loc != UNKNOWN_LOCATION)
location = phi_arg_loc;
- else
+ else if (var)
location = DECL_SOURCE_LOCATION (var);
+ else if (var_name_str)
+ location = gimple_location (var_def_stmt);
+
location = linemap_resolve_location (line_table, location,
LRK_SPELLING_LOCATION, NULL);
auto_diagnostic_group d;
- if (!warning_at (location, opt, gmsgid, var))
- return;
+ char *gmsgid_final = XNEWVEC (char, strlen (gmsgid) + 5);
+ gmsgid_final[0] = 0;
+ strcat (gmsgid_final, var ? "%qD " : "%qs ");
+ strcat (gmsgid_final, gmsgid);
+
+ if ((var && !warning_at (location, opt, gmsgid_final, var))
+ || (var_name_str
+ && !warning_at (location, opt, gmsgid_final, var_name_str)))
+ {
+ XDELETE (gmsgid_final);
+ return;
+ }
+ XDELETE (gmsgid_final);
/* Avoid subsequent warnings for reads of the same variable again. */
- suppress_warning (var, opt);
+ if (var)
+ suppress_warning (var, opt);
+ else if (var_name_str)
+ suppress_warning (var_def_stmt, opt);
/* Issue a note pointing to the read variable unless the warning
is at the same location. */
- location_t var_loc = DECL_SOURCE_LOCATION (var);
+ location_t var_loc = var ? DECL_SOURCE_LOCATION (var)
+ : gimple_location (var_def_stmt);
if (location == var_loc)
return;
- inform (var_loc, "%qD was declared here", var);
+ if (var)
+ inform (var_loc, "%qD was declared here", var);
+ else if (var_name_str)
+ inform (var_loc, "%qs was declared here", var_name_str);
}
struct check_defs_data
@@ -380,6 +449,20 @@ check_defs (ao_ref *ref, tree vdef, void *data_)
if (gimple_call_internal_p (def_stmt, IFN_DEFERRED_INIT))
return false;
+ /* For address taken variable, a temporary variable is added between
+ the variable and the call to .DEFERRED_INIT function as:
+ _1 = .DEFERRED_INIT (4, 2, &"i1"[0]);
+ i1 = _1;
+ Ignore this vdef as well. */
+ if (is_gimple_assign (def_stmt)
+ && gimple_assign_rhs_code (def_stmt) == SSA_NAME)
+ {
+ tree tmp_var = gimple_assign_rhs1 (def_stmt);
+ if (gimple_call_internal_p (SSA_NAME_DEF_STMT (tmp_var),
+ IFN_DEFERRED_INIT))
+ return false;
+ }
+
/* The ASAN_MARK intrinsic doesn't modify the variable. */
if (is_gimple_call (def_stmt))
{
@@ -878,7 +961,7 @@ warn_uninit_phi_uses (basic_block bb)
}
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);
}
--
2.27.0
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Patch][V4][Patch 2/2]Enable -Wuninitialized + -ftrivial-auto-var-init for address taken variables.
2022-01-13 1:45 [Patch][V4][Patch 2/2]Enable -Wuninitialized + -ftrivial-auto-var-init for address taken variables Qing Zhao
@ 2022-01-14 12:45 ` Richard Biener
2022-01-14 16:30 ` Qing Zhao
0 siblings, 1 reply; 7+ messages in thread
From: Richard Biener @ 2022-01-14 12:45 UTC (permalink / raw)
To: Qing Zhao; +Cc: Martin Sebor, gcc Patches
On Thu, Jan 13, 2022 at 2:45 AM Qing Zhao <qing.zhao@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,
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.
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.
So OK with the i8n thing sorted out - please post one hopefully last update
for the patch.
Thanks for your patience,
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.zhao@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.zhao@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:
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Patch][V4][Patch 2/2]Enable -Wuninitialized + -ftrivial-auto-var-init for address taken variables.
2022-01-14 12:45 ` Richard Biener
@ 2022-01-14 16:30 ` Qing Zhao
2022-01-14 18:11 ` Martin Sebor
0 siblings, 1 reply; 7+ messages in thread
From: Qing Zhao @ 2022-01-14 16:30 UTC (permalink / raw)
To: Richard Biener; +Cc: Martin Sebor, gcc Patches
> On Jan 14, 2022, at 6:45 AM, Richard Biener <richard.guenther@gmail.com> wrote:
>
> On Thu, Jan 13, 2022 at 2:45 AM Qing Zhao <qing.zhao@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.zhao@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.zhao@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:
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Patch][V4][Patch 2/2]Enable -Wuninitialized + -ftrivial-auto-var-init for address taken variables.
2022-01-14 16:30 ` Qing Zhao
@ 2022-01-14 18:11 ` Martin Sebor
2022-01-14 18:29 ` Qing Zhao
0 siblings, 1 reply; 7+ messages in thread
From: Martin Sebor @ 2022-01-14 18:11 UTC (permalink / raw)
To: Qing Zhao, Richard Biener; +Cc: gcc Patches
On 1/14/22 09:30, Qing Zhao wrote:
>
>
>> On Jan 14, 2022, at 6:45 AM, Richard Biener <richard.guenther@gmail.com> wrote:
>>
>> On Thu, Jan 13, 2022 at 2:45 AM Qing Zhao <qing.zhao@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.zhao@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.zhao@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:
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Patch][V4][Patch 2/2]Enable -Wuninitialized + -ftrivial-auto-var-init for address taken variables.
2022-01-14 18:11 ` Martin Sebor
@ 2022-01-14 18:29 ` Qing Zhao
2022-01-14 18:58 ` Martin Sebor
0 siblings, 1 reply; 7+ messages in thread
From: Qing Zhao @ 2022-01-14 18:29 UTC (permalink / raw)
To: Martin Sebor; +Cc: Richard Biener, gcc Patches
> On Jan 14, 2022, at 12:11 PM, Martin Sebor <msebor@gmail.com> wrote:
>
> On 1/14/22 09:30, Qing Zhao wrote:
>>> On Jan 14, 2022, at 6:45 AM, Richard Biener <richard.guenther@gmail.com> wrote:
>>>
>>> On Thu, Jan 13, 2022 at 2:45 AM Qing Zhao <qing.zhao@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 :)
I thought of this, but I didn’t find a meaningful statement around line 641 in tree-ssa-uninit.c…
636 found_alloc = true;
637 break;
638 }
639
640 if (!is_gimple_assign (def_stmt))
641 break;
642
643 tree_code code = gimple_assign_rhs_code (def_stmt);
644 if (code != ADDR_EXPR && code != POINTER_PLUS_EXPR)
645 break;
>
>>> 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.
Okay, so, you mean that it’s safe to just remove “in this function” from the last callsite?
>
> 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.
You mean the following in the routine warn_uninit:
/* Use either the location of the read statement or that of the PHI
argument, or that of the uninitialized variable, in that order,
whichever is valid. */
location_t location = UNKNOWN_LOCATION;
if (gimple_has_location (context))
location = gimple_location (context);
else if (phi_arg_loc != UNKNOWN_LOCATION)
location = phi_arg_loc;
else if (var)
location = DECL_SOURCE_LOCATION (var);
else if (var_name_str)
location = gimple_location (var_def_stmt);
When all the above failed, we could add the “location of the curly brace closing the function definition”?
Qing
>
> 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.zhao@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.zhao@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:
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Patch][V4][Patch 2/2]Enable -Wuninitialized + -ftrivial-auto-var-init for address taken variables.
2022-01-14 18:29 ` Qing Zhao
@ 2022-01-14 18:58 ` Martin Sebor
2022-01-14 20:20 ` Qing Zhao
0 siblings, 1 reply; 7+ messages in thread
From: Martin Sebor @ 2022-01-14 18:58 UTC (permalink / raw)
To: Qing Zhao; +Cc: Richard Biener, gcc Patches
On 1/14/22 11:29, Qing Zhao wrote:
>
>
>> On Jan 14, 2022, at 12:11 PM, Martin Sebor <msebor@gmail.com> wrote:
>>
>> On 1/14/22 09:30, Qing Zhao wrote:
>>>> On Jan 14, 2022, at 6:45 AM, Richard Biener <richard.guenther@gmail.com> wrote:
>>>>
>>>> On Thu, Jan 13, 2022 at 2:45 AM Qing Zhao <qing.zhao@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 :)
>
> I thought of this, but I didn’t find a meaningful statement around line 641 in tree-ssa-uninit.c…
>
> 636 found_alloc = true;
> 637 break;
> 638 }
> 639
> 640 if (!is_gimple_assign (def_stmt))
> 641 break;
> 642
> 643 tree_code code = gimple_assign_rhs_code (def_stmt);
> 644 if (code != ADDR_EXPR && code != POINTER_PLUS_EXPR)
> 645 break;
>
>>
>>>> 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.
>
> Okay, so, you mean that it’s safe to just remove “in this function” from the last callsite?
Yes, I would remove it. It might require some cleanup in the test
suite but I wouldn't expect it to be too bad.
>
>>
>> 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.
>
> You mean the following in the routine warn_uninit:
>
> /* Use either the location of the read statement or that of the PHI
> argument, or that of the uninitialized variable, in that order,
> whichever is valid. */
> location_t location = UNKNOWN_LOCATION;
> if (gimple_has_location (context))
> location = gimple_location (context);
> else if (phi_arg_loc != UNKNOWN_LOCATION)
> location = phi_arg_loc;
> else if (var)
> location = DECL_SOURCE_LOCATION (var);
> else if (var_name_str)
> location = gimple_location (var_def_stmt);
>
> When all the above failed, we could add the “location of the curly brace closing the function definition”?
Yes, that's what I meant. But to be clear, I didn't mean to suggest
you make the change as part of this change It was just a side note.
Sorry for not being clearer.
Martin
> Qing
>
>>
>> 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.zhao@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.zhao@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:
>>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Patch][V4][Patch 2/2]Enable -Wuninitialized + -ftrivial-auto-var-init for address taken variables.
2022-01-14 18:58 ` Martin Sebor
@ 2022-01-14 20:20 ` Qing Zhao
0 siblings, 0 replies; 7+ messages in thread
From: Qing Zhao @ 2022-01-14 20:20 UTC (permalink / raw)
To: Martin Sebor; +Cc: Richard Biener, gcc Patches
> On Jan 14, 2022, at 12:58 PM, Martin Sebor <msebor@gmail.com> wrote:
>
> On 1/14/22 11:29, Qing Zhao wrote:
>>> On Jan 14, 2022, at 12:11 PM, Martin Sebor <msebor@gmail.com> wrote:
>>>
>>> On 1/14/22 09:30, Qing Zhao wrote:
>>>>> On Jan 14, 2022, at 6:45 AM, Richard Biener <richard.guenther@gmail.com> wrote:
>>>>>
>>>>> On Thu, Jan 13, 2022 at 2:45 AM Qing Zhao <qing.zhao@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 :)
>> I thought of this, but I didn’t find a meaningful statement around line 641 in tree-ssa-uninit.c…
>> 636 found_alloc = true;
>> 637 break;
>> 638 }
>> 639
>> 640 if (!is_gimple_assign (def_stmt))
>> 641 break;
>> 642
>> 643 tree_code code = gimple_assign_rhs_code (def_stmt);
>> 644 if (code != ADDR_EXPR && code != POINTER_PLUS_EXPR)
>> 645 break;
>>
>>>
>>>>> 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.
>> Okay, so, you mean that it’s safe to just remove “in this function” from the last callsite?
>
> Yes, I would remove it. It might require some cleanup in the test
> suite but I wouldn't expect it to be too bad.
Okay, I see.
I didn’t see any test case regression due to this change.
>
>>>
>>> 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.
>> You mean the following in the routine warn_uninit:
>> /* Use either the location of the read statement or that of the PHI
>> argument, or that of the uninitialized variable, in that order,
>> whichever is valid. */
>> location_t location = UNKNOWN_LOCATION;
>> if (gimple_has_location (context))
>> location = gimple_location (context);
>> else if (phi_arg_loc != UNKNOWN_LOCATION)
>> location = phi_arg_loc;
>> else if (var)
>> location = DECL_SOURCE_LOCATION (var);
>> else if (var_name_str)
>> location = gimple_location (var_def_stmt);
>> When all the above failed, we could add the “location of the curly brace closing the function definition”?
>
> Yes, that's what I meant. But to be clear, I didn't mean to suggest
> you make the change as part of this change It was just a side note.
> Sorry for not being clearer.
Okay.
Thanks a lot.
Qing
>
> Martin
>
>
>> Qing
>>>
>>> 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.zhao@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.zhao@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:
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-01-14 20:20 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-13 1:45 [Patch][V4][Patch 2/2]Enable -Wuninitialized + -ftrivial-auto-var-init for address taken variables Qing Zhao
2022-01-14 12:45 ` Richard Biener
2022-01-14 16:30 ` Qing Zhao
2022-01-14 18:11 ` Martin Sebor
2022-01-14 18:29 ` Qing Zhao
2022-01-14 18:58 ` Martin Sebor
2022-01-14 20:20 ` Qing Zhao
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).