* [Patch][V2]Enable -Wuninitialized + -ftrivial-auto-var-init for address taken variables
@ 2021-12-16 15:59 Qing Zhao
2022-01-04 16:11 ` Patch Ping : " Qing Zhao
2022-01-05 10:33 ` Richard Biener
0 siblings, 2 replies; 9+ messages in thread
From: Qing Zhao @ 2021-12-16 15:59 UTC (permalink / raw)
To: Richard Biener, Martin Sebor; +Cc: gcc Patches
[-- Attachment #1: Type: text/plain, Size: 5212 bytes --]
Hi,
This is the 2nd version of the patch.
The original patch is at:
https://gcc.gnu.org/pipermail/gcc-patches/2021-December/586341.html
In addition to resolve the two issues mentioned in the original patch,
This patch also can be used as a very good workaround for the issue in PR103720
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103720
And as I checked, the patch can fix all the bogus uninitialized warnings when
building kernel with -O2 + -ftrivial-auto-var-init=zero + -Wuninitialized.
So, this is a very important patch that need to be included into gcc12.
Compared to the 1st patch, the major changes are to resolve Martin’s comments on
tree-ssa-uninit.c
1. Add some meaningful temporaries to break the long expression to make it
Readable. And also add comments to explain the purpose of the statement;
2. Resolve the memory leakage of the dynamically created string.
The patch has been bootstrapped and regressing tested on both x86 and aarch64, no issues.
Okay for commit?
thanks.
Qing
=================================================
******Compared to the 1st version, the code change is:
--- a/gcc/tree-ssa-uninit.c
+++ b/gcc/tree-ssa-uninit.c
@@ -182,9 +182,22 @@ warn_uninit (opt_code opt, tree t, tree var, const char *gmsgid,
@@ -798,26 +798,35 @@
if (!var && !SSA_NAME_VAR (t))
{
gimple *def_stmt = SSA_NAME_DEF_STMT (t);
-@@ -197,9 +210,34 @@ warn_uninit (opt_code opt, tree t, tree var, const char *gmsgid,
+@@ -197,9 +210,43 @@ warn_uninit (opt_code opt, tree t, tree var, const char *gmsgid,
&& zerop (gimple_assign_rhs2 (def_stmt)))
var = SSA_NAME_VAR (v);
}
+
+ if (gimple_call_internal_p (def_stmt, IFN_DEFERRED_INIT))
+ {
++ 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. */
+ var_name = gimple_call_arg (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)
-+ && TREE_CODE (gimple_assign_lhs (context)) == VAR_DECL
-+ && DECL_NAME (gimple_assign_lhs (context))
-+ && IDENTIFIER_POINTER (DECL_NAME (gimple_assign_lhs (context))))
-+ if (strcmp
-+ (IDENTIFIER_POINTER (DECL_NAME (gimple_assign_lhs (context))),
-+ var_name_str) == 0)
-+ return;
++ /* 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;
+
+ /* Get the variable declaration location from the def_stmt. */
+ var_decl_loc = gimple_location (def_stmt);
@@ -834,7 +843,7 @@
return;
/* Avoid warning if we've already done so or if the warning has been
-@@ -207,36 +245,56 @@ warn_uninit (opt_code opt, tree t, tree var, const char *gmsgid,
+@@ -207,36 +254,54 @@ 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)))))
@@ -863,25 +872,24 @@
auto_diagnostic_group d;
- if (!warning_at (location, opt, gmsgid, var))
+- return;
+ char *gmsgid_final = XNEWVEC (char, strlen (gmsgid) + 5);
+ gmsgid_final[0] = 0;
-+ if (var)
-+ strcat (gmsgid_final, "%qD ");
-+ else if (var_name)
-+ strcat (gmsgid_final, "%qs ");
++ strcat (gmsgid_final, var ? "%qD " : "%qs ");
+ strcat (gmsgid_final, gmsgid);
+
-+ if (var && !warning_at (location, opt, gmsgid_final, var))
-+ return;
-+ else if (var_name && !warning_at (location, opt, gmsgid_final, var_name_str))
- return;
++ if ((var && !warning_at (location, opt, gmsgid_final, var))
++ || (var_name && !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 (repl_var)
-+ suppress_warning (repl_var, opt);
++ if (var || repl_var)
++ suppress_warning (var ? var : repl_var, opt);
/* Issue a note pointing to the read variable unless the warning
is at the same location. */
@@ -898,7 +906,7 @@
}
******The complete patch is:
[-- Attachment #2: 0001-Enable-Wuninitialized-ftrivial-auto-var-init-for-add.patch --]
[-- Type: application/octet-stream, Size: 44684 bytes --]
From d4b62903b227b483a3634978ff690606e6d30731 Mon Sep 17 00:00:00 2001
From: Qing Zhao <qing.zhao@oracle.com>
Date: Wed, 15 Dec 2021 23:17:59 +0000
Subject: [PATCH] Enable -Wuninitialized + -ftrivial-auto-var-init for address
taken variables.
This patch is to resolve the following two issues in the current implemenation
of -ftrivial-auto-var-init:
1. The 3rd parameter of function .DEFERRED_INIT is IS_VLA currently, which is
not needed at all;
2. The address taken 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 report warnings for these variables.
To solve the above problems, we change the 3rd paramter of function
.DEFERRED_INIT from IS_VLA to NAME of the DECL. During uninitialized warning
analysis phase, for the following stmt:
_1 = .DEFERRED_INIT (4, 2, &"alt_reloc"[0]);
The original user variable has been eliminated from the IR, the LHS is the
temporary variable that was created to replace it. we will get the necessary
information from this stmt for reportinng the warning message:
A. the name of the DECL from the 3rd parameter of the call;
B. the location of the DECL from the location of the call;
C. the LHS is used to hold the information on whether the warning
has been issued or not to suppress warning messages when needed;
the current testing cases are updated to reflect these changes.
gcc/ChangeLog:
2021-12-15 qing zhao <qing.zhao@oracle.com>
* gimplify.c (gimple_add_init_for_auto_var): Delete the 3rd argument.
Change the 3rd argument of function .DEFERRED_INIT to the name of the
decl.
(gimplify_decl_expr): Delete the 3rd argument when call
gimple_add_init_for_auto_var.
* internal-fn.c (expand_DEFERRED_INIT): Update comments to reflect
the 3rd argument change of function .DEFERRED_INIT.
* tree-cfg.c (verify_gimple_call): Update comments and verification
to reflect the 3rd argument change of function .DEFERRED_INIT.
* tree-sra.c (generate_subtree_deferred_init): Delete the 3rd argument.
(sra_modify_deferred_init): Change the 3rd argument of function
.DEFERRED_INIT to the name of the decl.
* 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:
2021-12-15 qing zhao <qing.zhao@oracle.com>
* c-c++-common/auto-init-1.c: Adjust testcase to reflect the 3rd
argument change of function .DEFERRED_INIT.
* c-c++-common/auto-init-10.c: Likewise.
* c-c++-common/auto-init-11.c: Likewise.
* c-c++-common/auto-init-12.c: Likewise.
* c-c++-common/auto-init-13.c: Likewise.
* c-c++-common/auto-init-14.c: Likewise.
* c-c++-common/auto-init-15.c: Likewise.
* c-c++-common/auto-init-16.c: Likewise.
* c-c++-common/auto-init-2.c: Likewise.
* c-c++-common/auto-init-3.c: Likewise.
* c-c++-common/auto-init-4.c: Likewise.
* c-c++-common/auto-init-5.c: Likewise.
* c-c++-common/auto-init-6.c: Likewise.
* c-c++-common/auto-init-7.c: Likewise.
* c-c++-common/auto-init-8.c: Likewise.
* c-c++-common/auto-init-9.c: Likewise.
* c-c++-common/auto-init-esra.c: Likewise.
* c-c++-common/auto-init-padding-1.c: Likewise.
* gcc.dg/auto-init-uninit-16.c (testfunc): Adjust testcase 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.target/aarch64/auto-init-2.c: Adjust.
---
gcc/gimplify.c | 17 ++-
gcc/internal-fn.c | 10 +-
gcc/testsuite/c-c++-common/auto-init-1.c | 20 ++--
gcc/testsuite/c-c++-common/auto-init-10.c | 2 +-
gcc/testsuite/c-c++-common/auto-init-11.c | 2 +-
gcc/testsuite/c-c++-common/auto-init-12.c | 2 +-
gcc/testsuite/c-c++-common/auto-init-13.c | 4 +-
gcc/testsuite/c-c++-common/auto-init-14.c | 4 +-
gcc/testsuite/c-c++-common/auto-init-15.c | 2 +-
gcc/testsuite/c-c++-common/auto-init-16.c | 2 +-
gcc/testsuite/c-c++-common/auto-init-2.c | 20 ++--
gcc/testsuite/c-c++-common/auto-init-3.c | 6 +-
gcc/testsuite/c-c++-common/auto-init-4.c | 6 +-
gcc/testsuite/c-c++-common/auto-init-5.c | 6 +-
gcc/testsuite/c-c++-common/auto-init-6.c | 6 +-
gcc/testsuite/c-c++-common/auto-init-7.c | 8 +-
gcc/testsuite/c-c++-common/auto-init-8.c | 8 +-
gcc/testsuite/c-c++-common/auto-init-9.c | 4 +-
gcc/testsuite/c-c++-common/auto-init-esra.c | 6 +-
.../c-c++-common/auto-init-padding-1.c | 2 +-
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.target/aarch64/auto-init-2.c | 2 +-
gcc/tree-cfg.c | 39 ++----
gcc/tree-sra.c | 12 +-
gcc/tree-ssa-uninit.c | 111 +++++++++++++++---
28 files changed, 203 insertions(+), 158 deletions(-)
diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index b118c72f62c..44cfe3441a2 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -1748,16 +1748,13 @@ force_labels_r (tree *tp, int *walk_subtrees, void *data ATTRIBUTE_UNUSED)
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;
+ 3rd argument: NAME of the DECL;
+
+ as LHS = DEFERRED_INIT (SIZE of the DECL, INIT_TYPE, NAME of the DECL). */
- 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 a MEM_REF whose address is the pointer
- to this DECL. */
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 (auto_var_p (decl));
@@ -1767,13 +1764,14 @@ gimple_add_init_for_auto_var (tree 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 decl_name
+ = build_string_literal (IDENTIFIER_LENGTH (DECL_NAME (decl)) + 1,
+ IDENTIFIER_POINTER (DECL_NAME (decl)));
tree call = build_call_expr_internal_loc (loc, IFN_DEFERRED_INIT,
TREE_TYPE (decl), 3,
decl_size, init_type_node,
- is_vla_node);
+ decl_name);
gimplify_assign (decl, call, seq_p);
}
@@ -1947,7 +1945,6 @@ gimplify_decl_expr (tree *stmt_p, gimple_seq *seq_p)
{
gimple_add_init_for_auto_var (decl,
flag_auto_var_init,
- is_vla,
seq_p);
/* The expanding of a call to the above .DEFERRED_INIT will apply
block initialization to the whole space covered by this variable.
diff --git a/gcc/internal-fn.c b/gcc/internal-fn.c
index 08f94b7a17a..8b0a2fa369c 100644
--- a/gcc/internal-fn.c
+++ b/gcc/internal-fn.c
@@ -3011,11 +3011,7 @@ expand_UNIQUE (internal_fn, gcall *stmt)
}
/* Expand the IFN_DEFERRED_INIT function:
- 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 a MEM_REF whose address is the pointer
- to this DECL.
+ LHS = DEFERRED_INIT (SIZE of the DECL, INIT_TYPE, NAME of the DECL);
Initialize the LHS with zero/pattern according to its second argument
INIT_TYPE:
@@ -3071,8 +3067,8 @@ expand_DEFERRED_INIT (internal_fn, gcall *stmt)
if (!reg_lhs)
{
- /* If this is a VLA or the variable is not in register,
- expand to a memset to initialize it. */
+ /* If the variable is not in register, expand to a memset
+ to initialize it. */
mark_addressable (lhs);
tree var_addr = build_fold_addr_expr (lhs);
diff --git a/gcc/testsuite/c-c++-common/auto-init-1.c b/gcc/testsuite/c-c++-common/auto-init-1.c
index 84ba0a90d45..df04358728b 100644
--- a/gcc/testsuite/c-c++-common/auto-init-1.c
+++ b/gcc/testsuite/c-c++-common/auto-init-1.c
@@ -29,13 +29,13 @@ void foo()
return;
}
-/* { dg-final { scan-tree-dump "temp1 = .DEFERRED_INIT \\(1, 2, 0\\)" "gimple" } } */
-/* { dg-final { scan-tree-dump "temp2 = .DEFERRED_INIT \\(2, 2, 0\\)" "gimple" } } */
-/* { dg-final { scan-tree-dump "temp3 = .DEFERRED_INIT \\(4, 2, 0\\)" "gimple" } } */
-/* { dg-final { scan-tree-dump "temp4 = .DEFERRED_INIT \\(4, 2, 0\\)" "gimple" } } */
-/* { dg-final { scan-tree-dump "temp5 = .DEFERRED_INIT \\(4, 2, 0\\)" "gimple" { target ilp32 } } } */
-/* { dg-final { scan-tree-dump "temp5 = .DEFERRED_INIT \\(8, 2, 0\\)" "gimple" { target lp64 } } } */
-/* { dg-final { scan-tree-dump "temp6 = .DEFERRED_INIT \\(8, 2, 0\\)" "gimple" } } */
-/* { dg-final { scan-tree-dump "temp7 = .DEFERRED_INIT \\(4, 2, 0\\)" "gimple" { target ilp32 } } } */
-/* { dg-final { scan-tree-dump "temp7 = .DEFERRED_INIT \\(8, 2, 0\\)" "gimple" { target lp64 } } } */
-/* { dg-final { scan-tree-dump "temp8 = .DEFERRED_INIT \\(1, 2, 0\\)" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp1 = .DEFERRED_INIT \\(1, 2, \&\"temp1\"" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp2 = .DEFERRED_INIT \\(2, 2, \&\"temp2\"" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp3 = .DEFERRED_INIT \\(4, 2, \&\"temp3\"" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp4 = .DEFERRED_INIT \\(4, 2, \&\"temp4\"" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp5 = .DEFERRED_INIT \\(4, 2, \&\"temp5\"" "gimple" { target ilp32 } } } */
+/* { dg-final { scan-tree-dump "temp5 = .DEFERRED_INIT \\(8, 2, \&\"temp5\"" "gimple" { target lp64 } } } */
+/* { dg-final { scan-tree-dump "temp6 = .DEFERRED_INIT \\(8, 2, \&\"temp6\"" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp7 = .DEFERRED_INIT \\(4, 2, \&\"temp7\"" "gimple" { target ilp32 } } } */
+/* { dg-final { scan-tree-dump "temp7 = .DEFERRED_INIT \\(8, 2, \&\"temp7\"" "gimple" { target lp64 } } } */
+/* { dg-final { scan-tree-dump "temp8 = .DEFERRED_INIT \\(1, 2, \&\"temp8\"" "gimple" } } */
diff --git a/gcc/testsuite/c-c++-common/auto-init-10.c b/gcc/testsuite/c-c++-common/auto-init-10.c
index f35205f2a30..dda7ea1e032 100644
--- a/gcc/testsuite/c-c++-common/auto-init-10.c
+++ b/gcc/testsuite/c-c++-common/auto-init-10.c
@@ -18,5 +18,5 @@ void foo()
return;
}
-/* { dg-final { scan-tree-dump "temp1 = .DEFERRED_INIT \\(2, 1, 0\\)" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp1 = .DEFERRED_INIT \\(2, 1, \&\"temp1\"" "gimple" } } */
/* { dg-final { scan-tree-dump-not "temp2 = .DEFERRED_INIT \\(" "gimple" } } */
diff --git a/gcc/testsuite/c-c++-common/auto-init-11.c b/gcc/testsuite/c-c++-common/auto-init-11.c
index a2d66908d83..6eb468785ce 100644
--- a/gcc/testsuite/c-c++-common/auto-init-11.c
+++ b/gcc/testsuite/c-c++-common/auto-init-11.c
@@ -11,4 +11,4 @@ void foo(int n)
return;
}
-/* { dg-final { scan-tree-dump ".DEFERRED_INIT \\(D.\\d*, 2, 1\\)" "gimple" } } */
+/* { dg-final { scan-tree-dump ".DEFERRED_INIT \\(D.\\d*, 2, \&\"arr\"" "gimple" } } */
diff --git a/gcc/testsuite/c-c++-common/auto-init-12.c b/gcc/testsuite/c-c++-common/auto-init-12.c
index f05d743fda0..964291c5bd9 100644
--- a/gcc/testsuite/c-c++-common/auto-init-12.c
+++ b/gcc/testsuite/c-c++-common/auto-init-12.c
@@ -11,4 +11,4 @@ void foo(int n)
return;
}
-/* { dg-final { scan-tree-dump ".DEFERRED_INIT \\(D.\\d*, 1, 1\\)" "gimple" } } */
+/* { dg-final { scan-tree-dump ".DEFERRED_INIT \\(D.\\d*, 1, \&\"arr\"" "gimple" } } */
diff --git a/gcc/testsuite/c-c++-common/auto-init-13.c b/gcc/testsuite/c-c++-common/auto-init-13.c
index b0c0365b288..aa5883af770 100644
--- a/gcc/testsuite/c-c++-common/auto-init-13.c
+++ b/gcc/testsuite/c-c++-common/auto-init-13.c
@@ -19,5 +19,5 @@ int foo()
return d.b + var.bar.b;
}
-/* { dg-final { scan-tree-dump "d = .DEFERRED_INIT \\(4, 1, 0\\)" "gimple" } } */
-/* { dg-final { scan-tree-dump "var = .DEFERRED_INIT \\(4, 1, 0\\)" "gimple" } } */
+/* { dg-final { scan-tree-dump "d = .DEFERRED_INIT \\(4, 1, \&\"d\"" "gimple" } } */
+/* { dg-final { scan-tree-dump "var = .DEFERRED_INIT \\(4, 1, \&\"var\"" "gimple" } } */
diff --git a/gcc/testsuite/c-c++-common/auto-init-14.c b/gcc/testsuite/c-c++-common/auto-init-14.c
index 986bb19faaf..dd1ff3e339d 100644
--- a/gcc/testsuite/c-c++-common/auto-init-14.c
+++ b/gcc/testsuite/c-c++-common/auto-init-14.c
@@ -19,5 +19,5 @@ int foo()
return d.b + var.bar.b;
}
-/* { dg-final { scan-tree-dump "d = .DEFERRED_INIT \\(4, 2, 0\\)" "gimple" } } */
-/* { dg-final { scan-tree-dump "var = .DEFERRED_INIT \\(4, 2, 0\\)" "gimple" } } */
+/* { dg-final { scan-tree-dump "d = .DEFERRED_INIT \\(4, 2, \&\"d\"" "gimple" } } */
+/* { dg-final { scan-tree-dump "var = .DEFERRED_INIT \\(4, 2, \&\"var\"" "gimple" } } */
diff --git a/gcc/testsuite/c-c++-common/auto-init-15.c b/gcc/testsuite/c-c++-common/auto-init-15.c
index aa9d7fab68a..5857287ecbe 100644
--- a/gcc/testsuite/c-c++-common/auto-init-15.c
+++ b/gcc/testsuite/c-c++-common/auto-init-15.c
@@ -10,4 +10,4 @@ void foo(int a)
g(x);
}
-/* { dg-final { scan-tree-dump ".DEFERRED_INIT \\(D.\\d*, 2, 1\\)" "gimple" } } */
+/* { dg-final { scan-tree-dump ".DEFERRED_INIT \\(D.\\d*, 2, \&\"x\"" "gimple" } } */
diff --git a/gcc/testsuite/c-c++-common/auto-init-16.c b/gcc/testsuite/c-c++-common/auto-init-16.c
index 86493eef9e4..1e309959fc5 100644
--- a/gcc/testsuite/c-c++-common/auto-init-16.c
+++ b/gcc/testsuite/c-c++-common/auto-init-16.c
@@ -10,4 +10,4 @@ void foo(int a)
g(x);
}
-/* { dg-final { scan-tree-dump ".DEFERRED_INIT \\(D.\\d*, 1, 1\\)" "gimple" } } */
+/* { dg-final { scan-tree-dump ".DEFERRED_INIT \\(D.\\d*, 1, \&\"x\"" "gimple" } } */
diff --git a/gcc/testsuite/c-c++-common/auto-init-2.c b/gcc/testsuite/c-c++-common/auto-init-2.c
index 69768d6451d..6ac63bb1dda 100644
--- a/gcc/testsuite/c-c++-common/auto-init-2.c
+++ b/gcc/testsuite/c-c++-common/auto-init-2.c
@@ -29,13 +29,13 @@ void foo()
return;
}
-/* { dg-final { scan-tree-dump "temp1 = .DEFERRED_INIT \\(1, 1, 0\\)" "gimple" } } */
-/* { dg-final { scan-tree-dump "temp2 = .DEFERRED_INIT \\(2, 1, 0\\)" "gimple" } } */
-/* { dg-final { scan-tree-dump "temp3 = .DEFERRED_INIT \\(4, 1, 0\\)" "gimple" } } */
-/* { dg-final { scan-tree-dump "temp4 = .DEFERRED_INIT \\(4, 1, 0\\)" "gimple" } } */
-/* { dg-final { scan-tree-dump "temp5 = .DEFERRED_INIT \\(4, 1, 0\\)" "gimple" { target ilp32 } } } */
-/* { dg-final { scan-tree-dump "temp5 = .DEFERRED_INIT \\(8, 1, 0\\)" "gimple" { target lp64 } } } */
-/* { dg-final { scan-tree-dump "temp6 = .DEFERRED_INIT \\(8, 1, 0\\)" "gimple" } } */
-/* { dg-final { scan-tree-dump "temp7 = .DEFERRED_INIT \\(4, 1, 0\\)" "gimple" { target ilp32 } } } */
-/* { dg-final { scan-tree-dump "temp7 = .DEFERRED_INIT \\(8, 1, 0\\)" "gimple" { target lp64 } } } */
-/* { dg-final { scan-tree-dump "temp8 = .DEFERRED_INIT \\(1, 1, 0\\)" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp1 = .DEFERRED_INIT \\(1, 1, \&\"temp1\"" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp2 = .DEFERRED_INIT \\(2, 1, \&\"temp2\"" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp3 = .DEFERRED_INIT \\(4, 1, \&\"temp3\"" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp4 = .DEFERRED_INIT \\(4, 1, \&\"temp4\"" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp5 = .DEFERRED_INIT \\(4, 1, \&\"temp5\"" "gimple" { target ilp32 } } } */
+/* { dg-final { scan-tree-dump "temp5 = .DEFERRED_INIT \\(8, 1, \&\"temp5\"" "gimple" { target lp64 } } } */
+/* { dg-final { scan-tree-dump "temp6 = .DEFERRED_INIT \\(8, 1, \&\"temp6\"" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp7 = .DEFERRED_INIT \\(4, 1, \&\"temp7\"" "gimple" { target ilp32 } } } */
+/* { dg-final { scan-tree-dump "temp7 = .DEFERRED_INIT \\(8, 1, \&\"temp7\"" "gimple" { target lp64 } } } */
+/* { dg-final { scan-tree-dump "temp8 = .DEFERRED_INIT \\(1, 1, \&\"temp8\"" "gimple" } } */
diff --git a/gcc/testsuite/c-c++-common/auto-init-3.c b/gcc/testsuite/c-c++-common/auto-init-3.c
index 062d60c1631..9d9c86d8dd0 100644
--- a/gcc/testsuite/c-c++-common/auto-init-3.c
+++ b/gcc/testsuite/c-c++-common/auto-init-3.c
@@ -14,6 +14,6 @@ long double foo()
return result;
}
-/* { dg-final { scan-tree-dump "temp1 = .DEFERRED_INIT \\(4, 2, 0\\)" "gimple" } } */
-/* { dg-final { scan-tree-dump "temp2 = .DEFERRED_INIT \\(8, 2, 0\\)" "gimple" } } */
-/* { dg-final { scan-tree-dump "temp3 = .DEFERRED_INIT \\((8|12|16), 2, 0\\)" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp1 = .DEFERRED_INIT \\(4, 2, \&\"temp1\"" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp2 = .DEFERRED_INIT \\(8, 2, \&\"temp2\"" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp3 = .DEFERRED_INIT \\((8|12|16), 2, \&\"temp3\"" "gimple" } } */
diff --git a/gcc/testsuite/c-c++-common/auto-init-4.c b/gcc/testsuite/c-c++-common/auto-init-4.c
index 9d8f23ed4e3..848df2a0e26 100644
--- a/gcc/testsuite/c-c++-common/auto-init-4.c
+++ b/gcc/testsuite/c-c++-common/auto-init-4.c
@@ -14,6 +14,6 @@ long double foo()
return result;
}
-/* { dg-final { scan-tree-dump "temp1 = .DEFERRED_INIT \\(4, 1, 0\\)" "gimple" } } */
-/* { dg-final { scan-tree-dump "temp2 = .DEFERRED_INIT \\(8, 1, 0\\)" "gimple" } } */
-/* { dg-final { scan-tree-dump "temp3 = .DEFERRED_INIT \\((8|12|16), 1, 0\\)" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp1 = .DEFERRED_INIT \\(4, 1, \&\"temp1\"" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp2 = .DEFERRED_INIT \\(8, 1, \&\"temp2\"" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp3 = .DEFERRED_INIT \\((8|12|16), 1, \&\"temp3\"" "gimple" } } */
diff --git a/gcc/testsuite/c-c++-common/auto-init-5.c b/gcc/testsuite/c-c++-common/auto-init-5.c
index 9c98a6e7ab2..9c4de612182 100644
--- a/gcc/testsuite/c-c++-common/auto-init-5.c
+++ b/gcc/testsuite/c-c++-common/auto-init-5.c
@@ -15,7 +15,7 @@ _Complex long double foo()
return result;
}
-/* { dg-final { scan-tree-dump "temp1 = .DEFERRED_INIT \\(8, 2, 0\\)" "gimple" } } */
-/* { dg-final { scan-tree-dump "temp2 = .DEFERRED_INIT \\(16, 2, 0\\)" "gimple" } } */
-/* { dg-final { scan-tree-dump "temp3 = .DEFERRED_INIT \\((16|24|32), 2, 0\\)" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp1 = .DEFERRED_INIT \\(8, 2, \&\"temp1\"" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp2 = .DEFERRED_INIT \\(16, 2, \&\"temp2\"" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp3 = .DEFERRED_INIT \\((16|24|32), 2, \&\"temp3\"" "gimple" } } */
diff --git a/gcc/testsuite/c-c++-common/auto-init-6.c b/gcc/testsuite/c-c++-common/auto-init-6.c
index 3fe24562ebb..6a406447f3d 100644
--- a/gcc/testsuite/c-c++-common/auto-init-6.c
+++ b/gcc/testsuite/c-c++-common/auto-init-6.c
@@ -15,7 +15,7 @@ _Complex long double foo()
return result;
}
-/* { dg-final { scan-tree-dump "temp1 = .DEFERRED_INIT \\(8, 1, 0\\)" "gimple" } } */
-/* { dg-final { scan-tree-dump "temp2 = .DEFERRED_INIT \\(16, 1, 0\\)" "gimple" } } */
-/* { dg-final { scan-tree-dump "temp3 = .DEFERRED_INIT \\((16|24|32), 1, 0\\)" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp1 = .DEFERRED_INIT \\(8, 1, \&\"temp1\"" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp2 = .DEFERRED_INIT \\(16, 1, \&\"temp2\"" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp3 = .DEFERRED_INIT \\((16|24|32), 1, \&\"temp3\"" "gimple" } } */
diff --git a/gcc/testsuite/c-c++-common/auto-init-7.c b/gcc/testsuite/c-c++-common/auto-init-7.c
index 19986969a8f..b44dd5e68ed 100644
--- a/gcc/testsuite/c-c++-common/auto-init-7.c
+++ b/gcc/testsuite/c-c++-common/auto-init-7.c
@@ -29,7 +29,7 @@ double foo()
return result;
}
-/* { dg-final { scan-tree-dump "temp1 = .DEFERRED_INIT \\(12, 2, 0\\)" "gimple" } } */
-/* { dg-final { scan-tree-dump "temp2 = .DEFERRED_INIT \\(24, 2, 0\\)" "gimple" } } */
-/* { dg-final { scan-tree-dump "temp3 = .DEFERRED_INIT \\(28, 2, 0\\)" "gimple" } } */
-/* { dg-final { scan-tree-dump "temp4 = .DEFERRED_INIT \\(8, 2, 0\\)" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp1 = .DEFERRED_INIT \\(12, 2, \&\"temp1\"" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp2 = .DEFERRED_INIT \\(24, 2, \&\"temp2\"" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp3 = .DEFERRED_INIT \\(28, 2, \&\"temp3\"" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp4 = .DEFERRED_INIT \\(8, 2, \&\"temp4\"" "gimple" } } */
diff --git a/gcc/testsuite/c-c++-common/auto-init-8.c b/gcc/testsuite/c-c++-common/auto-init-8.c
index 9778e911e3a..739ac028931 100644
--- a/gcc/testsuite/c-c++-common/auto-init-8.c
+++ b/gcc/testsuite/c-c++-common/auto-init-8.c
@@ -29,7 +29,7 @@ double foo()
return result;
}
-/* { dg-final { scan-tree-dump "temp1 = .DEFERRED_INIT \\(12, 1, 0\\)" "gimple" } } */
-/* { dg-final { scan-tree-dump "temp2 = .DEFERRED_INIT \\(24, 1, 0\\)" "gimple" } } */
-/* { dg-final { scan-tree-dump "temp3 = .DEFERRED_INIT \\(28, 1, 0\\)" "gimple" } } */
-/* { dg-final { scan-tree-dump "temp4 = .DEFERRED_INIT \\(8, 1, 0\\)" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp1 = .DEFERRED_INIT \\(12, 1, \&\"temp1\"" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp2 = .DEFERRED_INIT \\(24, 1, \&\"temp2\"" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp3 = .DEFERRED_INIT \\(28, 1, \&\"temp3\"" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp4 = .DEFERRED_INIT \\(8, 1, \&\"temp4\"" "gimple" } } */
diff --git a/gcc/testsuite/c-c++-common/auto-init-9.c b/gcc/testsuite/c-c++-common/auto-init-9.c
index 29acb7f8669..113107ffd5c 100644
--- a/gcc/testsuite/c-c++-common/auto-init-9.c
+++ b/gcc/testsuite/c-c++-common/auto-init-9.c
@@ -16,5 +16,5 @@ void foo()
return;
}
-/* { dg-final { scan-tree-dump "temp1 = .DEFERRED_INIT \\(2, 2, 0\\)" "gimple" } } */
-/* { dg-final { scan-tree-dump-not "temp2 = .DEFERRED_INIT \\(8, 2, 0\\)" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp1 = .DEFERRED_INIT \\(2, 2, \&\"temp1\"" "gimple" } } */
+/* { dg-final { scan-tree-dump-not "temp2 = .DEFERRED_INIT \\(8, 2, \&\"temp2\"" "gimple" } } */
diff --git a/gcc/testsuite/c-c++-common/auto-init-esra.c b/gcc/testsuite/c-c++-common/auto-init-esra.c
index 77ec02355df..ce6779f20fc 100644
--- a/gcc/testsuite/c-c++-common/auto-init-esra.c
+++ b/gcc/testsuite/c-c++-common/auto-init-esra.c
@@ -1,6 +1,6 @@
/* Verify the strength reduction adjustment for -ftrivial-auto-var-init. */
/* { dg-do compile } */
-/* { dg-options "-O2 -ftrivial-auto-var-init=zero -fdump-tree-gimple -fdump-tree-esra" } */
+/* { dg-options "-O2 -ftrivial-auto-var-init=zero -fno-PIC -fdump-tree-gimple -fdump-tree-esra" } */
typedef double VECTOR[3];
@@ -31,5 +31,5 @@ void VCross(VECTOR a, const VECTOR b, const VECTOR c)
Assign_Vector(a, tmp);
}
-/* { dg-final { scan-tree-dump-times "tmp = .DEFERRED_INIT \\(24, 2, 0\\)" 1 "gimple" } } */
-/* { dg-final { scan-tree-dump-times ".DEFERRED_INIT \\(8, 2, 0\\)" 3 "esra" } } */
+/* { dg-final { scan-tree-dump-times "tmp = .DEFERRED_INIT \\(24, 2, \&\"tmp\"" 1 "gimple" } } */
+/* { dg-final { scan-tree-dump-times ".DEFERRED_INIT \\(8, 2, \&\"tmp\"" 3 "esra" } } */
diff --git a/gcc/testsuite/c-c++-common/auto-init-padding-1.c b/gcc/testsuite/c-c++-common/auto-init-padding-1.c
index 83db8dde832..d2e322717f0 100644
--- a/gcc/testsuite/c-c++-common/auto-init-padding-1.c
+++ b/gcc/testsuite/c-c++-common/auto-init-padding-1.c
@@ -19,5 +19,5 @@ void foo(int a)
g(s);
}
-/* { dg-final { scan-tree-dump ".DEFERRED_INIT \\(24, 1, 0\\)" "gimple" } } */
+/* { dg-final { scan-tree-dump ".DEFERRED_INIT \\(24, 1, \&\"s\"" "gimple" } } */
/* { dg-final { scan-tree-dump "__builtin_clear_padding" "gimple" } } */
diff --git a/gcc/testsuite/gcc.dg/auto-init-uninit-16.c b/gcc/testsuite/gcc.dg/auto-init-uninit-16.c
index 38e19502a67..f14864be901 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 1a687654ebb..d6e7ed3e860 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 2791b37ca00..aea554262f8 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 b6d3efdfb88..40d3196ea47 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/testsuite/gcc.target/aarch64/auto-init-2.c b/gcc/testsuite/gcc.target/aarch64/auto-init-2.c
index 2c54e6d6038..375befd325b 100644
--- a/gcc/testsuite/gcc.target/aarch64/auto-init-2.c
+++ b/gcc/testsuite/gcc.target/aarch64/auto-init-2.c
@@ -32,4 +32,4 @@ void foo()
/* { dg-final { scan-rtl-dump-times "0xfe\\\]" 1 "expand" } } */
/* { dg-final { scan-rtl-dump-times "0xfffffffffffffefe" 1 "expand" } } */
/* { dg-final { scan-rtl-dump-times "0xfffffffffefefefe" 2 "expand" } } */
-/* { dg-final { scan-rtl-dump-times "0xfefefefefefefefe" 2 "expand" } } */
+/* { dg-final { scan-rtl-dump-times "0xfefefefefefefefe" 3 "expand" } } */
diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
index 672e384ef09..622802c469f 100644
--- a/gcc/tree-cfg.c
+++ b/gcc/tree-cfg.c
@@ -3455,19 +3455,14 @@ verify_gimple_call (gcall *stmt)
}
/* For a call to .DEFERRED_INIT,
- LHS = DEFERRED_INIT (SIZE of the DECL, INIT_TYPE, IS_VLA)
- we should guarantee that the 1st and the 3rd arguments are consistent:
- 1st argument: SIZE of the DECL;
- 3rd argument: IS_VLA, 0 NO, 1 YES;
+ LHS = DEFERRED_INIT (SIZE of the DECL, INIT_TYPE, NAME of the DECL)
+ we should guarantee that when the 1st argument is a constant, it should
+ be the same as the size of the LHS. */
- if IS_VLA is false, the 1st argument should be a constant and the same as
- the size of the LHS. */
if (gimple_call_internal_p (stmt, IFN_DEFERRED_INIT))
{
tree size_of_arg0 = gimple_call_arg (stmt, 0);
tree size_of_lhs = TYPE_SIZE_UNIT (TREE_TYPE (lhs));
- tree is_vla_node = gimple_call_arg (stmt, 2);
- bool is_vla = (bool) TREE_INT_CST_LOW (is_vla_node);
if (TREE_CODE (lhs) == SSA_NAME)
lhs = SSA_NAME_VAR (lhs);
@@ -3477,27 +3472,13 @@ verify_gimple_call (gcall *stmt)
&size_from_arg0);
bool is_constant_size_lhs = poly_int_tree_p (size_of_lhs,
&size_from_lhs);
- if (!is_vla)
- {
- if (!is_constant_size_arg0)
- {
- error ("%<DEFFERED_INIT%> calls for non-VLA should have "
- "constant size for the first argument");
- return true;
- }
- else if (!is_constant_size_lhs)
- {
- error ("%<DEFFERED_INIT%> calls for non-VLA should have "
- "constant size for the LHS");
- return true;
- }
- else if (maybe_ne (size_from_arg0, size_from_lhs))
- {
- error ("%<DEFFERED_INIT%> calls for non-VLA should have same "
- "constant size for the first argument and LHS");
- return true;
- }
- }
+ if (is_constant_size_arg0 && is_constant_size_lhs)
+ if (maybe_ne (size_from_arg0, size_from_lhs))
+ {
+ error ("%<DEFFERED_INIT%> calls should have same "
+ "constant size for the first argument and LHS");
+ return true;
+ }
}
/* ??? The C frontend passes unpromoted arguments in case it
diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c
index 76e3aae405c..a96e725af67 100644
--- a/gcc/tree-sra.c
+++ b/gcc/tree-sra.c
@@ -4123,7 +4123,7 @@ get_repl_default_def_ssa_name (struct access *racc, tree reg_type)
static void
generate_subtree_deferred_init (struct access *access,
tree init_type,
- tree is_vla,
+ tree decl_name,
gimple_stmt_iterator *gsi,
location_t loc)
{
@@ -4135,7 +4135,7 @@ generate_subtree_deferred_init (struct access *access,
gimple *call
= gimple_build_call_internal (IFN_DEFERRED_INIT, 3,
TYPE_SIZE_UNIT (TREE_TYPE (repl)),
- init_type, is_vla);
+ init_type, decl_name);
gimple_call_set_lhs (call, repl);
gsi_insert_before (gsi, call, GSI_SAME_STMT);
update_stmt (call);
@@ -4144,7 +4144,7 @@ generate_subtree_deferred_init (struct access *access,
}
if (access->first_child)
generate_subtree_deferred_init (access->first_child, init_type,
- is_vla, gsi, loc);
+ decl_name, gsi, loc);
access = access ->next_sibling;
}
@@ -4152,7 +4152,7 @@ generate_subtree_deferred_init (struct access *access,
}
/* For a call to .DEFERRED_INIT:
- var = .DEFERRED_INIT (size_of_var, init_type, is_vla);
+ var = .DEFERRED_INIT (size_of_var, init_type, name_of_var);
examine the LHS variable VAR and replace it with a scalar replacement if
there is one, also replace the RHS call to a call to .DEFERRED_INIT of
the corresponding scalar relacement variable. Examine the subtree and
@@ -4164,7 +4164,7 @@ sra_modify_deferred_init (gimple *stmt, gimple_stmt_iterator *gsi)
{
tree lhs = gimple_call_lhs (stmt);
tree init_type = gimple_call_arg (stmt, 1);
- tree is_vla = gimple_call_arg (stmt, 2);
+ tree decl_name = gimple_call_arg (stmt, 2);
struct access *lhs_access = get_access_for_expr (lhs);
if (!lhs_access)
@@ -4185,7 +4185,7 @@ sra_modify_deferred_init (gimple *stmt, gimple_stmt_iterator *gsi)
if (lhs_access->first_child)
generate_subtree_deferred_init (lhs_access->first_child,
- init_type, is_vla, gsi, loc);
+ init_type, decl_name, gsi, loc);
if (lhs_access->grp_covered)
{
unlink_stmt_vdef (stmt);
diff --git a/gcc/tree-ssa-uninit.c b/gcc/tree-ssa-uninit.c
index 1df0bcc42c0..ac540f7b370 100644
--- a/gcc/tree-ssa-uninit.c
+++ b/gcc/tree-ssa-uninit.c
@@ -182,9 +182,22 @@ 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.
+
+ 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. At the same time, we
+ should use the temporary variable that was created to replace the original
+ variable as a placeholder to record the information on whether the warning
+ message for the variable has been issued or not. */
+
+ tree var_name = NULL_TREE;
+ const char *var_name_str = NULL;
+ location_t var_decl_loc = UNKNOWN_LOCATION;
+ tree repl_var = NULL_TREE;
+
if (!var && !SSA_NAME_VAR (t))
{
gimple *def_stmt = SSA_NAME_DEF_STMT (t);
@@ -197,9 +210,43 @@ warn_uninit (opt_code opt, tree t, tree var, const char *gmsgid,
&& zerop (gimple_assign_rhs2 (def_stmt)))
var = SSA_NAME_VAR (v);
}
+
+ if (gimple_call_internal_p (def_stmt, IFN_DEFERRED_INIT))
+ {
+ 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. */
+ var_name = gimple_call_arg (def_stmt, 2);
+ var_name = TREE_OPERAND (TREE_OPERAND (var_name, 0), 0);
+ var_name_str = TREE_STRING_POINTER (var_name);
+
+ /* 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;
+
+ /* Get the variable declaration location from the def_stmt. */
+ var_decl_loc = gimple_location (def_stmt);
+
+ /* The LHS of the call is a temporary variable, we use it as a
+ placeholder to record the information on whether the warning
+ has been issued or not. */
+ repl_var = gimple_call_lhs (def_stmt);
+ }
}
- if (var == NULL_TREE)
+ if (var == NULL_TREE && var_name == NULL_TREE)
return;
/* Avoid warning if we've already done so or if the warning has been
@@ -207,36 +254,54 @@ 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))
+ || (repl_var && get_no_uninit_warning (repl_var)))
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)
+ location = var_decl_loc;
+
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 && !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 || repl_var)
+ suppress_warning (var ? var : repl_var, 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) : var_decl_loc;
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)
+ inform (var_loc, "%qs was declared here", var_name_str);
}
struct check_defs_data
@@ -380,6 +445,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 +957,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 +1011,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 +1261,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] 9+ messages in thread
* Patch Ping : [Patch][V2]Enable -Wuninitialized + -ftrivial-auto-var-init for address taken variables
2021-12-16 15:59 [Patch][V2]Enable -Wuninitialized + -ftrivial-auto-var-init for address taken variables Qing Zhao
@ 2022-01-04 16:11 ` Qing Zhao
2022-01-05 10:33 ` Richard Biener
1 sibling, 0 replies; 9+ messages in thread
From: Qing Zhao @ 2022-01-04 16:11 UTC (permalink / raw)
To: Richard Biener, Martin Sebor; +Cc: gcc Patches
Hi,
I’d like to ping the patch:
https://gcc.gnu.org/pipermail/gcc-patches/2021-December/587014.html
Please take a look and let me know whether it’s okay for committing?
Thanks.
Qing
> On Dec 16, 2021, at 9:59 AM, Qing Zhao via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
>
> Hi,
>
> This is the 2nd version of the patch.
> The original patch is at:
>
> https://gcc.gnu.org/pipermail/gcc-patches/2021-December/586341.html
>
> In addition to resolve the two issues mentioned in the original patch,
> This patch also can be used as a very good workaround for the issue in PR103720
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103720
>
> And as I checked, the patch can fix all the bogus uninitialized warnings when
> building kernel with -O2 + -ftrivial-auto-var-init=zero + -Wuninitialized.
>
> So, this is a very important patch that need to be included into gcc12.
>
> Compared to the 1st patch, the major changes are to resolve Martin’s comments on
> tree-ssa-uninit.c
>
> 1. Add some meaningful temporaries to break the long expression to make it
> Readable. And also add comments to explain the purpose of the statement;
>
> 2. Resolve the memory leakage of the dynamically created string.
>
> The patch has been bootstrapped and regressing tested on both x86 and aarch64, no issues.
> Okay for commit?
>
> thanks.
>
> Qing
>
> =================================================
>
> ******Compared to the 1st version, the code change is:
>
> --- a/gcc/tree-ssa-uninit.c
> +++ b/gcc/tree-ssa-uninit.c
> @@ -182,9 +182,22 @@ warn_uninit (opt_code opt, tree t, tree var, const char *gmsgid,
> @@ -798,26 +798,35 @@
> if (!var && !SSA_NAME_VAR (t))
> {
> gimple *def_stmt = SSA_NAME_DEF_STMT (t);
> -@@ -197,9 +210,34 @@ warn_uninit (opt_code opt, tree t, tree var, const char *gmsgid,
> +@@ -197,9 +210,43 @@ warn_uninit (opt_code opt, tree t, tree var, const char *gmsgid,
> && zerop (gimple_assign_rhs2 (def_stmt)))
> var = SSA_NAME_VAR (v);
> }
> +
> + if (gimple_call_internal_p (def_stmt, IFN_DEFERRED_INIT))
> + {
> ++ 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. */
> + var_name = gimple_call_arg (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)
> -+ && TREE_CODE (gimple_assign_lhs (context)) == VAR_DECL
> -+ && DECL_NAME (gimple_assign_lhs (context))
> -+ && IDENTIFIER_POINTER (DECL_NAME (gimple_assign_lhs (context))))
> -+ if (strcmp
> -+ (IDENTIFIER_POINTER (DECL_NAME (gimple_assign_lhs (context))),
> -+ var_name_str) == 0)
> -+ return;
> ++ /* 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;
> +
> + /* Get the variable declaration location from the def_stmt. */
> + var_decl_loc = gimple_location (def_stmt);
> @@ -834,7 +843,7 @@
> return;
>
> /* Avoid warning if we've already done so or if the warning has been
> -@@ -207,36 +245,56 @@ warn_uninit (opt_code opt, tree t, tree var, const char *gmsgid,
> +@@ -207,36 +254,54 @@ 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)))))
> @@ -863,25 +872,24 @@
>
> auto_diagnostic_group d;
> - if (!warning_at (location, opt, gmsgid, var))
> +- return;
> + char *gmsgid_final = XNEWVEC (char, strlen (gmsgid) + 5);
> + gmsgid_final[0] = 0;
> -+ if (var)
> -+ strcat (gmsgid_final, "%qD ");
> -+ else if (var_name)
> -+ strcat (gmsgid_final, "%qs ");
> ++ strcat (gmsgid_final, var ? "%qD " : "%qs ");
> + strcat (gmsgid_final, gmsgid);
> +
> -+ if (var && !warning_at (location, opt, gmsgid_final, var))
> -+ return;
> -+ else if (var_name && !warning_at (location, opt, gmsgid_final, var_name_str))
> - return;
> ++ if ((var && !warning_at (location, opt, gmsgid_final, var))
> ++ || (var_name && !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 (repl_var)
> -+ suppress_warning (repl_var, opt);
> ++ if (var || repl_var)
> ++ suppress_warning (var ? var : repl_var, opt);
>
> /* Issue a note pointing to the read variable unless the warning
> is at the same location. */
> @@ -898,7 +906,7 @@
> }
>
> ******The complete patch is:
>
>
>
>
> <0001-Enable-Wuninitialized-ftrivial-auto-var-init-for-add.patch>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Patch][V2]Enable -Wuninitialized + -ftrivial-auto-var-init for address taken variables
2021-12-16 15:59 [Patch][V2]Enable -Wuninitialized + -ftrivial-auto-var-init for address taken variables Qing Zhao
2022-01-04 16:11 ` Patch Ping : " Qing Zhao
@ 2022-01-05 10:33 ` Richard Biener
2022-01-05 16:59 ` Qing Zhao
1 sibling, 1 reply; 9+ messages in thread
From: Richard Biener @ 2022-01-05 10:33 UTC (permalink / raw)
To: Qing Zhao; +Cc: Martin Sebor, gcc Patches
On Thu, Dec 16, 2021 at 5:00 PM Qing Zhao <qing.zhao@oracle.com> wrote:
>
> Hi,
>
> This is the 2nd version of the patch.
> The original patch is at:
>
> https://gcc.gnu.org/pipermail/gcc-patches/2021-December/586341.html
>
> In addition to resolve the two issues mentioned in the original patch,
> This patch also can be used as a very good workaround for the issue in PR103720
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103720
>
> And as I checked, the patch can fix all the bogus uninitialized warnings when
> building kernel with -O2 + -ftrivial-auto-var-init=zero + -Wuninitialized.
>
> So, this is a very important patch that need to be included into gcc12.
>
> Compared to the 1st patch, the major changes are to resolve Martin’s comments on
> tree-ssa-uninit.c
>
> 1. Add some meaningful temporaries to break the long expression to make it
> Readable. And also add comments to explain the purpose of the statement;
>
> 2. Resolve the memory leakage of the dynamically created string.
>
> The patch has been bootstrapped and regressing tested on both x86 and aarch64, no issues.
> Okay for commit?
tree decl_name
+ = build_string_literal (IDENTIFIER_LENGTH (DECL_NAME (decl)) + 1,
+ IDENTIFIER_POINTER (DECL_NAME (decl)));
you need to deal with DECL_NAME being NULL. It's also a bit awkward
to build another
copy of all decl names :/ The changes in the uninit warning are also
quite ugly, refactoring
things to always pass down a name / location pair might improve that
(but I'd like to
understand the case to fix first).
+ /* The LHS of the call is a temporary variable, we use it as a
+ placeholder to record the information on whether the warning
+ has been issued or not. */
+ repl_var = gimple_call_lhs (def_stmt);
this seems to be a change that can be done independently?
+ /* 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. 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?
The patch adjusts testcases but doesn't add new ones but each of the
above changes
would warrant one and make it easier to understand the motivation of
the changes.
Richard.
> thanks.
>
> Qing
>
> =================================================
>
> ******Compared to the 1st version, the code change is:
>
> --- a/gcc/tree-ssa-uninit.c
> +++ b/gcc/tree-ssa-uninit.c
> @@ -182,9 +182,22 @@ warn_uninit (opt_code opt, tree t, tree var, const char *gmsgid,
> @@ -798,26 +798,35 @@
> if (!var && !SSA_NAME_VAR (t))
> {
> gimple *def_stmt = SSA_NAME_DEF_STMT (t);
> -@@ -197,9 +210,34 @@ warn_uninit (opt_code opt, tree t, tree var, const char *gmsgid,
> +@@ -197,9 +210,43 @@ warn_uninit (opt_code opt, tree t, tree var, const char *gmsgid,
> && zerop (gimple_assign_rhs2 (def_stmt)))
> var = SSA_NAME_VAR (v);
> }
> +
> + if (gimple_call_internal_p (def_stmt, IFN_DEFERRED_INIT))
> + {
> ++ 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. */
> + var_name = gimple_call_arg (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)
> -+ && TREE_CODE (gimple_assign_lhs (context)) == VAR_DECL
> -+ && DECL_NAME (gimple_assign_lhs (context))
> -+ && IDENTIFIER_POINTER (DECL_NAME (gimple_assign_lhs (context))))
> -+ if (strcmp
> -+ (IDENTIFIER_POINTER (DECL_NAME (gimple_assign_lhs (context))),
> -+ var_name_str) == 0)
> -+ return;
> ++ /* 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;
> +
> + /* Get the variable declaration location from the def_stmt. */
> + var_decl_loc = gimple_location (def_stmt);
> @@ -834,7 +843,7 @@
> return;
>
> /* Avoid warning if we've already done so or if the warning has been
> -@@ -207,36 +245,56 @@ warn_uninit (opt_code opt, tree t, tree var, const char *gmsgid,
> +@@ -207,36 +254,54 @@ 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)))))
> @@ -863,25 +872,24 @@
>
> auto_diagnostic_group d;
> - if (!warning_at (location, opt, gmsgid, var))
> +- return;
> + char *gmsgid_final = XNEWVEC (char, strlen (gmsgid) + 5);
> + gmsgid_final[0] = 0;
> -+ if (var)
> -+ strcat (gmsgid_final, "%qD ");
> -+ else if (var_name)
> -+ strcat (gmsgid_final, "%qs ");
> ++ strcat (gmsgid_final, var ? "%qD " : "%qs ");
> + strcat (gmsgid_final, gmsgid);
> +
> -+ if (var && !warning_at (location, opt, gmsgid_final, var))
> -+ return;
> -+ else if (var_name && !warning_at (location, opt, gmsgid_final, var_name_str))
> - return;
> ++ if ((var && !warning_at (location, opt, gmsgid_final, var))
> ++ || (var_name && !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 (repl_var)
> -+ suppress_warning (repl_var, opt);
> ++ if (var || repl_var)
> ++ suppress_warning (var ? var : repl_var, opt);
>
> /* Issue a note pointing to the read variable unless the warning
> is at the same location. */
> @@ -898,7 +906,7 @@
> }
>
> ******The complete patch is:
>
>
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Patch][V2]Enable -Wuninitialized + -ftrivial-auto-var-init for address taken variables
2022-01-05 10:33 ` Richard Biener
@ 2022-01-05 16:59 ` Qing Zhao
2022-01-07 22:33 ` Qing Zhao
2022-01-11 13:43 ` Richard Biener
0 siblings, 2 replies; 9+ messages in thread
From: Qing Zhao @ 2022-01-05 16:59 UTC (permalink / raw)
To: Richard Biener; +Cc: Martin Sebor, gcc Patches
Hi, Richard,
Thanks a lot for the review and questions.
See my reply embedded below:
> On Jan 5, 2022, at 4:33 AM, Richard Biener <richard.guenther@gmail.com> wrote:
>
> On Thu, Dec 16, 2021 at 5:00 PM Qing Zhao <qing.zhao@oracle.com> wrote:
>>
>> Hi,
>>
>> This is the 2nd version of the patch.
>> The original patch is at:
>>
>> https://gcc.gnu.org/pipermail/gcc-patches/2021-December/586341.html
>>
>> In addition to resolve the two issues mentioned in the original patch,
>> This patch also can be used as a very good workaround for the issue in PR103720
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103720
>>
>> And as I checked, the patch can fix all the bogus uninitialized warnings when
>> building kernel with -O2 + -ftrivial-auto-var-init=zero + -Wuninitialized.
>>
>> So, this is a very important patch that need to be included into gcc12.
>>
>> Compared to the 1st patch, the major changes are to resolve Martin’s comments on
>> tree-ssa-uninit.c
>>
>> 1. Add some meaningful temporaries to break the long expression to make it
>> Readable. And also add comments to explain the purpose of the statement;
>>
>> 2. Resolve the memory leakage of the dynamically created string.
>>
>> The patch has been bootstrapped and regressing tested on both x86 and aarch64, no issues.
>> Okay for commit?
>
> tree decl_name
> + = build_string_literal (IDENTIFIER_LENGTH (DECL_NAME (decl)) + 1,
> + IDENTIFIER_POINTER (DECL_NAME (decl)));
>
> you need to deal with DECL_NAME being NULL.
Okay.
Usually under what situation, the decl_name will be NULL?
> It's also a bit awkward
> to build another
> copy of all decl names :/
Yes, this is awkward. But it might be unavoidable for address taken variables since the original variable might be completely deleted by optimizations.
See the details at:
https://gcc.gnu.org/pipermail/gcc-patches/2021-August/577431.html
We had a previous discussion on this issue, and the idea of adding this 3rd argument with the name of the variable was proposed by you at that time. -:)
And I think that’s a good solution to this problem.
> The changes in the uninit warning are also
> quite ugly, refactoring
> things to always pass down a name / location pair might improve that
> (but I'd like to
> understand the case to fix first).
I will try this idea to see whether it will work.
>
> + /* The LHS of the call is a temporary variable, we use it as a
> + placeholder to record the information on whether the warning
> + has been issued or not. */
> + repl_var = gimple_call_lhs (def_stmt);
>
> this seems to be a change that can be done independently?
The major purpose of this “repl_var” is used to record the info whether the warning has been issued for the variable or not, then avoid emitting it again later.
Since the original variable has been completely deleted by optimization, we have to use this “repl_var” for a placeholder to record such info.
>
> + /* 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.
> 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.
And this patch is to resolve this issue.
Hope this is clear.
Thanks.
Qimg
>
> The patch adjusts testcases but doesn't add new ones but each of the
> above changes
> would warrant one and make it easier to understand the motivation of
> the changes.
>
> Richard.
>
>> thanks.
>>
>> Qing
>>
>> =================================================
>>
>> ******Compared to the 1st version, the code change is:
>>
>> --- a/gcc/tree-ssa-uninit.c
>> +++ b/gcc/tree-ssa-uninit.c
>> @@ -182,9 +182,22 @@ warn_uninit (opt_code opt, tree t, tree var, const char *gmsgid,
>> @@ -798,26 +798,35 @@
>> if (!var && !SSA_NAME_VAR (t))
>> {
>> gimple *def_stmt = SSA_NAME_DEF_STMT (t);
>> -@@ -197,9 +210,34 @@ warn_uninit (opt_code opt, tree t, tree var, const char *gmsgid,
>> +@@ -197,9 +210,43 @@ warn_uninit (opt_code opt, tree t, tree var, const char *gmsgid,
>> && zerop (gimple_assign_rhs2 (def_stmt)))
>> var = SSA_NAME_VAR (v);
>> }
>> +
>> + if (gimple_call_internal_p (def_stmt, IFN_DEFERRED_INIT))
>> + {
>> ++ 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. */
>> + var_name = gimple_call_arg (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)
>> -+ && TREE_CODE (gimple_assign_lhs (context)) == VAR_DECL
>> -+ && DECL_NAME (gimple_assign_lhs (context))
>> -+ && IDENTIFIER_POINTER (DECL_NAME (gimple_assign_lhs (context))))
>> -+ if (strcmp
>> -+ (IDENTIFIER_POINTER (DECL_NAME (gimple_assign_lhs (context))),
>> -+ var_name_str) == 0)
>> -+ return;
>> ++ /* 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;
>> +
>> + /* Get the variable declaration location from the def_stmt. */
>> + var_decl_loc = gimple_location (def_stmt);
>> @@ -834,7 +843,7 @@
>> return;
>>
>> /* Avoid warning if we've already done so or if the warning has been
>> -@@ -207,36 +245,56 @@ warn_uninit (opt_code opt, tree t, tree var, const char *gmsgid,
>> +@@ -207,36 +254,54 @@ 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)))))
>> @@ -863,25 +872,24 @@
>>
>> auto_diagnostic_group d;
>> - if (!warning_at (location, opt, gmsgid, var))
>> +- return;
>> + char *gmsgid_final = XNEWVEC (char, strlen (gmsgid) + 5);
>> + gmsgid_final[0] = 0;
>> -+ if (var)
>> -+ strcat (gmsgid_final, "%qD ");
>> -+ else if (var_name)
>> -+ strcat (gmsgid_final, "%qs ");
>> ++ strcat (gmsgid_final, var ? "%qD " : "%qs ");
>> + strcat (gmsgid_final, gmsgid);
>> +
>> -+ if (var && !warning_at (location, opt, gmsgid_final, var))
>> -+ return;
>> -+ else if (var_name && !warning_at (location, opt, gmsgid_final, var_name_str))
>> - return;
>> ++ if ((var && !warning_at (location, opt, gmsgid_final, var))
>> ++ || (var_name && !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 (repl_var)
>> -+ suppress_warning (repl_var, opt);
>> ++ if (var || repl_var)
>> ++ suppress_warning (var ? var : repl_var, opt);
>>
>> /* Issue a note pointing to the read variable unless the warning
>> is at the same location. */
>> @@ -898,7 +906,7 @@
>> }
>>
>> ******The complete patch is:
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Patch][V2]Enable -Wuninitialized + -ftrivial-auto-var-init for address taken variables
2022-01-05 16:59 ` Qing Zhao
@ 2022-01-07 22:33 ` Qing Zhao
2022-01-11 13:43 ` Richard Biener
1 sibling, 0 replies; 9+ messages in thread
From: Qing Zhao @ 2022-01-07 22:33 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc Patches
> On Jan 5, 2022, at 10:59 AM, Qing Zhao via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
>
> Hi, Richard,
>
> Thanks a lot for the review and questions.
> See my reply embedded below:
>
>
>> On Jan 5, 2022, at 4:33 AM, Richard Biener <richard.guenther@gmail.com> wrote:
>>
>> On Thu, Dec 16, 2021 at 5:00 PM Qing Zhao <qing.zhao@oracle.com> wrote:
>>>
>>> Hi,
>>>
>>> This is the 2nd version of the patch.
>>> The original patch is at:
>>>
>>> https://gcc.gnu.org/pipermail/gcc-patches/2021-December/586341.html
>>>
>>> In addition to resolve the two issues mentioned in the original patch,
>>> This patch also can be used as a very good workaround for the issue in PR103720
>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103720
>>>
>>> And as I checked, the patch can fix all the bogus uninitialized warnings when
>>> building kernel with -O2 + -ftrivial-auto-var-init=zero + -Wuninitialized.
>>>
>>> So, this is a very important patch that need to be included into gcc12.
>>>
>>> Compared to the 1st patch, the major changes are to resolve Martin’s comments on
>>> tree-ssa-uninit.c
>>>
>>> 1. Add some meaningful temporaries to break the long expression to make it
>>> Readable. And also add comments to explain the purpose of the statement;
>>>
>>> 2. Resolve the memory leakage of the dynamically created string.
>>>
>>> The patch has been bootstrapped and regressing tested on both x86 and aarch64, no issues.
>>> Okay for commit?
>>
>> tree decl_name
>> + = build_string_literal (IDENTIFIER_LENGTH (DECL_NAME (decl)) + 1,
>> + IDENTIFIER_POINTER (DECL_NAME (decl)));
>>
>> you need to deal with DECL_NAME being NULL.
>
> Okay.
> Usually under what situation, the decl_name will be NULL?
Anyway, I made the following change in gimplify.c to address this concern:
[opc@qinzhao-ol8u3-x86 gcc]$ git diff
diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index cc6b643312e..a9714c9f9c4 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -1764,9 +1764,12 @@ gimple_add_init_for_auto_var (tree decl,
tree init_type_node
= build_int_cst (integer_type_node, (int) init_type);
+ const char *decl_name_str = DECL_NAME (decl)
+ ? IDENTIFIER_POINTER (DECL_NAME (decl))
+ : "<anonymous>";
tree decl_name
- = build_string_literal (IDENTIFIER_LENGTH (DECL_NAME (decl)) + 1,
- IDENTIFIER_POINTER (DECL_NAME (decl)));
+ = build_string_literal (strlen (decl_name_str) + 1,
+ decl_name_str);
Let me know if you see any issue with this solution.
>
>> It's also a bit awkward
>> to build another
>> copy of all decl names :/
>
> Yes, this is awkward. But it might be unavoidable for address taken variables since the original variable might be completely deleted by optimizations.
> See the details at:
> https://gcc.gnu.org/pipermail/gcc-patches/2021-August/577431.html
>
> We had a previous discussion on this issue, and the idea of adding this 3rd argument with the name of the variable was proposed by you at that time. -:)
> And I think that’s a good solution to this problem.
>
>
>> The changes in the uninit warning are also
>> quite ugly, refactoring
>> things to always pass down a name / location pair might improve that
>> (but I'd like to
>> understand the case to fix first).
>
> I will try this idea to see whether it will work.
Martin Sebor raised the similar concern and similar suggestions in the previous round of review, and I tried to pass the name string and use “%qs” consistently.
However, that didn’t work since there are some special cases that %qD handles specially other than %qs. There were multiple testing cases failed after the change.
In order to resolve the regression, we have to copy multiple details from the handling of “%D” to uninit warning.
Please see:
https://gcc.gnu.org/pipermail/gcc-patches/2021-December/586814.html
For more details.
>
>>
>> + /* The LHS of the call is a temporary variable, we use it as a
>> + placeholder to record the information on whether the warning
>> + has been issued or not. */
>> + repl_var = gimple_call_lhs (def_stmt);
>>
>> this seems to be a change that can be done independently?
>
> The major purpose of this “repl_var” is used to record the info whether the warning has been issued for the variable or not, then avoid emitting it again later.
> Since the original variable has been completely deleted by optimization, we have to use this “repl_var” for a placeholder to record such info.
>>
>> + /* 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.
>
>> 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.
>
> And this patch is to resolve this issue.
> Hope this is clear.
>
> Thanks.
>
> Qimg
>
>
>>
>> The patch adjusts testcases but doesn't add new ones but each of the
>> above changes
>> would warrant one and make it easier to understand the motivation of
>> the changes.
Adjusting the following testing cases is the main purpose of this patch:
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
Previously, the warnings on address taken variables were missing, I added “xfail” at the warning lines.
With this patch, all the warnings for address taken variables can be issued correctly, so, I removed all the “xfail” at the warning lines.
All the other adjustment are for the 3rd parameter of .DEFERRED_INIT.
Let me know whether it’s clear about the purpose of this patch.
If you still have questions on the purpose of this patch, please let me know.
Thanks a lot.
Qing
>>
>> Richard.
>>
>>> thanks.
>>>
>>> Qing
>>>
>>> =================================================
>>>
>>> ******Compared to the 1st version, the code change is:
>>>
>>> --- a/gcc/tree-ssa-uninit.c
>>> +++ b/gcc/tree-ssa-uninit.c
>>> @@ -182,9 +182,22 @@ warn_uninit (opt_code opt, tree t, tree var, const char *gmsgid,
>>> @@ -798,26 +798,35 @@
>>> if (!var && !SSA_NAME_VAR (t))
>>> {
>>> gimple *def_stmt = SSA_NAME_DEF_STMT (t);
>>> -@@ -197,9 +210,34 @@ warn_uninit (opt_code opt, tree t, tree var, const char *gmsgid,
>>> +@@ -197,9 +210,43 @@ warn_uninit (opt_code opt, tree t, tree var, const char *gmsgid,
>>> && zerop (gimple_assign_rhs2 (def_stmt)))
>>> var = SSA_NAME_VAR (v);
>>> }
>>> +
>>> + if (gimple_call_internal_p (def_stmt, IFN_DEFERRED_INIT))
>>> + {
>>> ++ 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. */
>>> + var_name = gimple_call_arg (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)
>>> -+ && TREE_CODE (gimple_assign_lhs (context)) == VAR_DECL
>>> -+ && DECL_NAME (gimple_assign_lhs (context))
>>> -+ && IDENTIFIER_POINTER (DECL_NAME (gimple_assign_lhs (context))))
>>> -+ if (strcmp
>>> -+ (IDENTIFIER_POINTER (DECL_NAME (gimple_assign_lhs (context))),
>>> -+ var_name_str) == 0)
>>> -+ return;
>>> ++ /* 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;
>>> +
>>> + /* Get the variable declaration location from the def_stmt. */
>>> + var_decl_loc = gimple_location (def_stmt);
>>> @@ -834,7 +843,7 @@
>>> return;
>>>
>>> /* Avoid warning if we've already done so or if the warning has been
>>> -@@ -207,36 +245,56 @@ warn_uninit (opt_code opt, tree t, tree var, const char *gmsgid,
>>> +@@ -207,36 +254,54 @@ 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)))))
>>> @@ -863,25 +872,24 @@
>>>
>>> auto_diagnostic_group d;
>>> - if (!warning_at (location, opt, gmsgid, var))
>>> +- return;
>>> + char *gmsgid_final = XNEWVEC (char, strlen (gmsgid) + 5);
>>> + gmsgid_final[0] = 0;
>>> -+ if (var)
>>> -+ strcat (gmsgid_final, "%qD ");
>>> -+ else if (var_name)
>>> -+ strcat (gmsgid_final, "%qs ");
>>> ++ strcat (gmsgid_final, var ? "%qD " : "%qs ");
>>> + strcat (gmsgid_final, gmsgid);
>>> +
>>> -+ if (var && !warning_at (location, opt, gmsgid_final, var))
>>> -+ return;
>>> -+ else if (var_name && !warning_at (location, opt, gmsgid_final, var_name_str))
>>> - return;
>>> ++ if ((var && !warning_at (location, opt, gmsgid_final, var))
>>> ++ || (var_name && !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 (repl_var)
>>> -+ suppress_warning (repl_var, opt);
>>> ++ if (var || repl_var)
>>> ++ suppress_warning (var ? var : repl_var, opt);
>>>
>>> /* Issue a note pointing to the read variable unless the warning
>>> is at the same location. */
>>> @@ -898,7 +906,7 @@
>>> }
>>>
>>> ******The complete patch is:
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Patch][V2]Enable -Wuninitialized + -ftrivial-auto-var-init for address taken variables
2022-01-05 16:59 ` Qing Zhao
2022-01-07 22:33 ` Qing Zhao
@ 2022-01-11 13:43 ` Richard Biener
2022-01-11 16:32 ` Qing Zhao
1 sibling, 1 reply; 9+ messages in thread
From: Richard Biener @ 2022-01-11 13:43 UTC (permalink / raw)
To: Qing Zhao; +Cc: Martin Sebor, gcc Patches
On Wed, Jan 5, 2022 at 5:59 PM Qing Zhao <qing.zhao@oracle.com> wrote:
>
> Hi, Richard,
>
> Thanks a lot for the review and questions.
> See my reply embedded below:
>
>
> > On Jan 5, 2022, at 4:33 AM, Richard Biener <richard.guenther@gmail.com> wrote:
> >
> > On Thu, Dec 16, 2021 at 5:00 PM Qing Zhao <qing.zhao@oracle.com> wrote:
> >>
> >> Hi,
> >>
> >> This is the 2nd version of the patch.
> >> The original patch is at:
> >>
> >> https://gcc.gnu.org/pipermail/gcc-patches/2021-December/586341.html
> >>
> >> In addition to resolve the two issues mentioned in the original patch,
> >> This patch also can be used as a very good workaround for the issue in PR103720
> >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103720
> >>
> >> And as I checked, the patch can fix all the bogus uninitialized warnings when
> >> building kernel with -O2 + -ftrivial-auto-var-init=zero + -Wuninitialized.
> >>
> >> So, this is a very important patch that need to be included into gcc12.
> >>
> >> Compared to the 1st patch, the major changes are to resolve Martin’s comments on
> >> tree-ssa-uninit.c
> >>
> >> 1. Add some meaningful temporaries to break the long expression to make it
> >> Readable. And also add comments to explain the purpose of the statement;
> >>
> >> 2. Resolve the memory leakage of the dynamically created string.
> >>
> >> The patch has been bootstrapped and regressing tested on both x86 and aarch64, no issues.
> >> Okay for commit?
> >
> > tree decl_name
> > + = build_string_literal (IDENTIFIER_LENGTH (DECL_NAME (decl)) + 1,
> > + IDENTIFIER_POINTER (DECL_NAME (decl)));
> >
> > you need to deal with DECL_NAME being NULL.
>
> Okay.
> Usually under what situation, the decl_name will be NULL?
I don't know but it definitely happens.
> > It's also a bit awkward
> > to build another
> > copy of all decl names :/
>
> Yes, this is awkward. But it might be unavoidable for address taken variables since the original variable might be completely deleted by optimizations.
> See the details at:
> https://gcc.gnu.org/pipermail/gcc-patches/2021-August/577431.html
>
> We had a previous discussion on this issue, and the idea of adding this 3rd argument with the name of the variable was proposed by you at that time. -:)
I know ... I didn't have a better idea.
> And I think that’s a good solution to this problem.
>
> > The changes in the uninit warning are also
> > quite ugly, refactoring
> > things to always pass down a name / location pair might improve that
> > (but I'd like to
> > understand the case to fix first).
>
> I will try this idea to see whether it will work.
>
> >
> > + /* The LHS of the call is a temporary variable, we use it as a
> > + placeholder to record the information on whether the warning
> > + has been issued or not. */
> > + repl_var = gimple_call_lhs (def_stmt);
> >
> > this seems to be a change that can be done independently?
>
> The major purpose of this “repl_var” is used to record the info whether the warning has been issued for the variable or not, then avoid emitting it again later.
> Since the original variable has been completely deleted by optimization, we have to use this “repl_var” for a placeholder to record such info.
But the ... = .DEFERRED_INIT stmt itself could be used to record this
since its location is
already used to indicate the original location, like with
suppress_warning/warning_suppressed_p?
> >
> > + /* 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?
>
> > 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.
Sorry for the delay btw,
Richard.
>
> And this patch is to resolve this issue.
> Hope this is clear.
>
> Thanks.
>
> Qimg
>
>
> >
> > The patch adjusts testcases but doesn't add new ones but each of the
> > above changes
> > would warrant one and make it easier to understand the motivation of
> > the changes.
> >
> > Richard.
> >
> >> thanks.
> >>
> >> Qing
> >>
> >> =================================================
> >>
> >> ******Compared to the 1st version, the code change is:
> >>
> >> --- a/gcc/tree-ssa-uninit.c
> >> +++ b/gcc/tree-ssa-uninit.c
> >> @@ -182,9 +182,22 @@ warn_uninit (opt_code opt, tree t, tree var, const char *gmsgid,
> >> @@ -798,26 +798,35 @@
> >> if (!var && !SSA_NAME_VAR (t))
> >> {
> >> gimple *def_stmt = SSA_NAME_DEF_STMT (t);
> >> -@@ -197,9 +210,34 @@ warn_uninit (opt_code opt, tree t, tree var, const char *gmsgid,
> >> +@@ -197,9 +210,43 @@ warn_uninit (opt_code opt, tree t, tree var, const char *gmsgid,
> >> && zerop (gimple_assign_rhs2 (def_stmt)))
> >> var = SSA_NAME_VAR (v);
> >> }
> >> +
> >> + if (gimple_call_internal_p (def_stmt, IFN_DEFERRED_INIT))
> >> + {
> >> ++ 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. */
> >> + var_name = gimple_call_arg (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)
> >> -+ && TREE_CODE (gimple_assign_lhs (context)) == VAR_DECL
> >> -+ && DECL_NAME (gimple_assign_lhs (context))
> >> -+ && IDENTIFIER_POINTER (DECL_NAME (gimple_assign_lhs (context))))
> >> -+ if (strcmp
> >> -+ (IDENTIFIER_POINTER (DECL_NAME (gimple_assign_lhs (context))),
> >> -+ var_name_str) == 0)
> >> -+ return;
> >> ++ /* 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;
> >> +
> >> + /* Get the variable declaration location from the def_stmt. */
> >> + var_decl_loc = gimple_location (def_stmt);
> >> @@ -834,7 +843,7 @@
> >> return;
> >>
> >> /* Avoid warning if we've already done so or if the warning has been
> >> -@@ -207,36 +245,56 @@ warn_uninit (opt_code opt, tree t, tree var, const char *gmsgid,
> >> +@@ -207,36 +254,54 @@ 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)))))
> >> @@ -863,25 +872,24 @@
> >>
> >> auto_diagnostic_group d;
> >> - if (!warning_at (location, opt, gmsgid, var))
> >> +- return;
> >> + char *gmsgid_final = XNEWVEC (char, strlen (gmsgid) + 5);
> >> + gmsgid_final[0] = 0;
> >> -+ if (var)
> >> -+ strcat (gmsgid_final, "%qD ");
> >> -+ else if (var_name)
> >> -+ strcat (gmsgid_final, "%qs ");
> >> ++ strcat (gmsgid_final, var ? "%qD " : "%qs ");
> >> + strcat (gmsgid_final, gmsgid);
> >> +
> >> -+ if (var && !warning_at (location, opt, gmsgid_final, var))
> >> -+ return;
> >> -+ else if (var_name && !warning_at (location, opt, gmsgid_final, var_name_str))
> >> - return;
> >> ++ if ((var && !warning_at (location, opt, gmsgid_final, var))
> >> ++ || (var_name && !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 (repl_var)
> >> -+ suppress_warning (repl_var, opt);
> >> ++ if (var || repl_var)
> >> ++ suppress_warning (var ? var : repl_var, opt);
> >>
> >> /* Issue a note pointing to the read variable unless the warning
> >> is at the same location. */
> >> @@ -898,7 +906,7 @@
> >> }
> >>
> >> ******The complete patch is:
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Patch][V2]Enable -Wuninitialized + -ftrivial-auto-var-init for address taken variables
2022-01-11 13:43 ` Richard Biener
@ 2022-01-11 16:32 ` Qing Zhao
2022-01-12 10:46 ` Richard Biener
0 siblings, 1 reply; 9+ messages in thread
From: Qing Zhao @ 2022-01-11 16:32 UTC (permalink / raw)
To: Richard Biener; +Cc: Martin Sebor, gcc Patches
Hi, Richard,
> On Jan 11, 2022, at 7:43 AM, Richard Biener <richard.guenther@gmail.com> wrote:
>
>>>>
>>>>
>>>> 1. Add some meaningful temporaries to break the long expression to make it
>>>> Readable. And also add comments to explain the purpose of the statement;
>>>>
>>>> 2. Resolve the memory leakage of the dynamically created string.
>>>>
>>>> The patch has been bootstrapped and regressing tested on both x86 and aarch64, no issues.
>>>> Okay for commit?
>>>
>>> tree decl_name
>>> + = build_string_literal (IDENTIFIER_LENGTH (DECL_NAME (decl)) + 1,
>>> + IDENTIFIER_POINTER (DECL_NAME (decl)));
>>>
>>> you need to deal with DECL_NAME being NULL.
>>
>> Okay.
>> Usually under what situation, the decl_name will be NULL?
>
> I don't know but it definitely happens.
>
>>> It's also a bit awkward
>>> to build another
>>> copy of all decl names :/
>>
>> Yes, this is awkward. But it might be unavoidable for address taken variables since the original variable might be completely deleted by optimizations.
>> See the details at:
>> https://gcc.gnu.org/pipermail/gcc-patches/2021-August/577431.html
>>
>> We had a previous discussion on this issue, and the idea of adding this 3rd argument with the name of the variable was proposed by you at that time. -:)
>
> I know ... I didn't have a better idea.
I think that adding the name string of the auto variable as one parameter to the function .DEFERRED_INIT might be the only solution to this issue? (In the very beginning of the implementation, we added the VAR itself as one parameter to the function .DEFERRED_INIT, but that design didn’t work out)
>
>>
>>
>>>
>>> + /* The LHS of the call is a temporary variable, we use it as a
>>> + placeholder to record the information on whether the warning
>>> + has been issued or not. */
>>> + repl_var = gimple_call_lhs (def_stmt);
>>>
>>> this seems to be a change that can be done independently?
>>
>> The major purpose of this “repl_var” is used to record the info whether the warning has been issued for the variable or not, then avoid emitting it again later.
>> Since the original variable has been completely deleted by optimization, we have to use this “repl_var” for a placeholder to record such info.
>
> But the ... = .DEFERRED_INIT stmt itself could be used to record this
> since its location is
> already used to indicate the original location, like with
> suppress_warning/warning_suppressed_p?
Ah, I will check on this. Thanks a lot.
>
>>>
>>> + /* 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
/* { dg-do compile } */
/* { dg-options "-O2 -Wuninitialized -ftrivial-auto-var-init=zero" } */
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;
}
****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.
>
>>
>>> 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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Patch][V2]Enable -Wuninitialized + -ftrivial-auto-var-init for address taken variables
2022-01-11 16:32 ` Qing Zhao
@ 2022-01-12 10:46 ` Richard Biener
2022-01-12 16:33 ` Qing Zhao
0 siblings, 1 reply; 9+ messages in thread
From: Richard Biener @ 2022-01-12 10:46 UTC (permalink / raw)
To: Qing Zhao; +Cc: Martin Sebor, gcc Patches
On Tue, Jan 11, 2022 at 5:32 PM Qing Zhao <qing.zhao@oracle.com> wrote:
>
> Hi, Richard,
>
> > On Jan 11, 2022, at 7:43 AM, Richard Biener <richard.guenther@gmail.com> wrote:
> >
> >>>>
> >>>>
> >>>> 1. Add some meaningful temporaries to break the long expression to make it
> >>>> Readable. And also add comments to explain the purpose of the statement;
> >>>>
> >>>> 2. Resolve the memory leakage of the dynamically created string.
> >>>>
> >>>> The patch has been bootstrapped and regressing tested on both x86 and aarch64, no issues.
> >>>> Okay for commit?
> >>>
> >>> tree decl_name
> >>> + = build_string_literal (IDENTIFIER_LENGTH (DECL_NAME (decl)) + 1,
> >>> + IDENTIFIER_POINTER (DECL_NAME (decl)));
> >>>
> >>> you need to deal with DECL_NAME being NULL.
> >>
> >> Okay.
> >> Usually under what situation, the decl_name will be NULL?
> >
> > I don't know but it definitely happens.
> >
> >>> It's also a bit awkward
> >>> to build another
> >>> copy of all decl names :/
> >>
> >> Yes, this is awkward. But it might be unavoidable for address taken variables since the original variable might be completely deleted by optimizations.
> >> See the details at:
> >> https://gcc.gnu.org/pipermail/gcc-patches/2021-August/577431.html
> >>
> >> We had a previous discussion on this issue, and the idea of adding this 3rd argument with the name of the variable was proposed by you at that time. -:)
> >
> > I know ... I didn't have a better idea.
>
> I think that adding the name string of the auto variable as one parameter to the function .DEFERRED_INIT might be the only solution to this issue? (In the very beginning of the implementation, we added the VAR itself as one parameter to the function .DEFERRED_INIT, but that design didn’t work out)
> >
> >>
> >>
> >>>
> >>> + /* The LHS of the call is a temporary variable, we use it as a
> >>> + placeholder to record the information on whether the warning
> >>> + has been issued or not. */
> >>> + repl_var = gimple_call_lhs (def_stmt);
> >>>
> >>> this seems to be a change that can be done independently?
> >>
> >> The major purpose of this “repl_var” is used to record the info whether the warning has been issued for the variable or not, then avoid emitting it again later.
> >> Since the original variable has been completely deleted by optimization, we have to use this “repl_var” for a placeholder to record such info.
> >
> > But the ... = .DEFERRED_INIT stmt itself could be used to record this
> > since its location is
> > already used to indicate the original location, like with
> > suppress_warning/warning_suppressed_p?
>
> Ah, I will check on this. Thanks a lot.
> >
> >>>
> >>> + /* 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
> /* { dg-do compile } */
> /* { dg-options "-O2 -Wuninitialized -ftrivial-auto-var-init=zero" } */
>
> 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;
> }
>
> ****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"? I was
expecting the first patch to fix this.
I'll wait for an update to the second patch.
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
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Patch][V2]Enable -Wuninitialized + -ftrivial-auto-var-init for address taken variables
2022-01-12 10:46 ` Richard Biener
@ 2022-01-12 16:33 ` Qing Zhao
0 siblings, 0 replies; 9+ messages in thread
From: Qing Zhao @ 2022-01-12 16:33 UTC (permalink / raw)
To: Richard Biener; +Cc: Martin Sebor, gcc Patches
> On Jan 12, 2022, at 4:46 AM, Richard Biener <richard.guenther@gmail.com> wrote:
>
> On Tue, Jan 11, 2022 at 5:32 PM Qing Zhao <qing.zhao@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
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-01-12 16:33 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-16 15:59 [Patch][V2]Enable -Wuninitialized + -ftrivial-auto-var-init for address taken variables Qing Zhao
2022-01-04 16:11 ` Patch Ping : " Qing Zhao
2022-01-05 10:33 ` Richard Biener
2022-01-05 16:59 ` Qing Zhao
2022-01-07 22:33 ` Qing Zhao
2022-01-11 13:43 ` Richard Biener
2022-01-11 16:32 ` Qing Zhao
2022-01-12 10:46 ` Richard Biener
2022-01-12 16:33 ` 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).