diff mbox series

[V2] Fixing PR107411

Message ID 4794643D-1A2B-428C-BA10-F5B426D262B1@oracle.com
State New
Headers show
Series [V2] Fixing PR107411 | expand

Commit Message

Qing Zhao Feb. 27, 2023, 2:30 p.m. UTC 
Ping.

Qing

Begin forwarded message:

From: Qing Zhao <qing.zhao@oracle.com<mailto:qing.zhao@oracle.com>>
Subject: [V2][PATCH] Fixing PR107411
Date: February 21, 2023 at 9:46:04 AM EST
To: jakub@redhat.com<mailto:jakub@redhat.com>, rguenther@suse.de<mailto:rguenther@suse.de>
Cc: gcc-patches@gcc.gnu.org<mailto:gcc-patches@gcc.gnu.org>, Qing Zhao <qing.zhao@oracle.com<mailto:qing.zhao@oracle.com>>

This is the 2nd version of the patch.
compared to the first version, the major change is:

use sprintf to replace xasprintf per Jacub's suggestion.

bootstrapped and regression tested on both x86 and aarch64.

Okay for committing?

thanks.

Qing

=======================


This is a bug in tree-ssa-uninit.cc<http://tree-ssa-uninit.cc>.
When doing the following:

 /* Ignore the call to .DEFERRED_INIT that define the original
    var itself as the following case:
      temp = .DEFERRED_INIT (4, 2, “alt_reloc");
      alt_reloc = temp;
    In order to avoid generating warning for the fake usage
    at alt_reloc = temp.
 */

We need to compare the var name inside the .DEFERRED_INIT call
(the 3rd argument) and the name for the LHS variable. if they are the same,
we will NOT report the warning.

There is one issue when we get the name for the LHS variable. when the
variable doesn't have a DECL_NAME (it's not a user declared variable,
which is the case for this bug):

 _1 = .DEFERRED_INIT (4, 2, &"D.2389"[0]);
 D.2389 = _1;

The current checking just ignores this case, and still report the warning.

The fix is very simple, when getting the name for the LHS variable, we should
consider this case and come up with the name the same way as we construct the
3rd argument for the call to .DEFERRED_INIT (please refer to the routine
"gimple_add_init_for_auto_var")

PR middle-end/107411

gcc/ChangeLog:

PR middle-end/107411
* gimplify.cc<http://gimplify.cc> (gimple_add_init_for_auto_var): Use sprintf to replace
xasprintf.
* tree-ssa-uninit.cc<http://tree-ssa-uninit.cc> (warn_uninit): Handle the case when the
LHS varaible of a .DEFERRED_INIT call doesn't have a DECL_NAME.

gcc/testsuite/ChangeLog:

PR middle-end/107411
* g++.dg/pr107411.C: New test.
---
gcc/gimplify.cc<http://gimplify.cc>                 |  4 ++--
gcc/testsuite/g++.dg/pr107411.C | 10 ++++++++++
gcc/tree-ssa-uninit.cc<http://tree-ssa-uninit.cc>          | 23 ++++++++++++++++-------
3 files changed, 28 insertions(+), 9 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/pr107411.C

--
2.31.1

Comments

Richard Biener Feb. 28, 2023, 10:02 a.m. UTC | #1 
On Mon, 27 Feb 2023, Qing Zhao wrote:

> Ping.

OK.

Thanks,
Richard.

> Qing
> 
> Begin forwarded message:
> 
> From: Qing Zhao <qing.zhao@oracle.com<mailto:qing.zhao@oracle.com>>
> Subject: [V2][PATCH] Fixing PR107411
> Date: February 21, 2023 at 9:46:04 AM EST
> To: jakub@redhat.com<mailto:jakub@redhat.com>, rguenther@suse.de<mailto:rguenther@suse.de>
> Cc: gcc-patches@gcc.gnu.org<mailto:gcc-patches@gcc.gnu.org>, Qing Zhao <qing.zhao@oracle.com<mailto:qing.zhao@oracle.com>>
> 
> This is the 2nd version of the patch.
> compared to the first version, the major change is:
> 
> use sprintf to replace xasprintf per Jacub's suggestion.
> 
> bootstrapped and regression tested on both x86 and aarch64.
> 
> Okay for committing?
> 
> thanks.
> 
> Qing
> 
> =======================
> 
> 
> This is a bug in tree-ssa-uninit.cc<http://tree-ssa-uninit.cc>.
> When doing the following:
> 
>  /* Ignore the call to .DEFERRED_INIT that define the original
>     var itself as the following case:
>       temp = .DEFERRED_INIT (4, 2, “alt_reloc");
>       alt_reloc = temp;
>     In order to avoid generating warning for the fake usage
>     at alt_reloc = temp.
>  */
> 
> We need to compare the var name inside the .DEFERRED_INIT call
> (the 3rd argument) and the name for the LHS variable. if they are the same,
> we will NOT report the warning.
> 
> There is one issue when we get the name for the LHS variable. when the
> variable doesn't have a DECL_NAME (it's not a user declared variable,
> which is the case for this bug):
> 
>  _1 = .DEFERRED_INIT (4, 2, &"D.2389"[0]);
>  D.2389 = _1;
> 
> The current checking just ignores this case, and still report the warning.
> 
> The fix is very simple, when getting the name for the LHS variable, we should
> consider this case and come up with the name the same way as we construct the
> 3rd argument for the call to .DEFERRED_INIT (please refer to the routine
> "gimple_add_init_for_auto_var")
> 
> PR middle-end/107411
> 
> gcc/ChangeLog:
> 
> PR middle-end/107411
> * gimplify.cc<http://gimplify.cc> (gimple_add_init_for_auto_var): Use sprintf to replace
> xasprintf.
> * tree-ssa-uninit.cc<http://tree-ssa-uninit.cc> (warn_uninit): Handle the case when the
> LHS varaible of a .DEFERRED_INIT call doesn't have a DECL_NAME.
> 
> gcc/testsuite/ChangeLog:
> 
> PR middle-end/107411
> * g++.dg/pr107411.C: New test.
> ---
> gcc/gimplify.cc<http://gimplify.cc>                 |  4 ++--
> gcc/testsuite/g++.dg/pr107411.C | 10 ++++++++++
> gcc/tree-ssa-uninit.cc<http://tree-ssa-uninit.cc>          | 23 ++++++++++++++++-------
> 3 files changed, 28 insertions(+), 9 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/pr107411.C
> 
> diff --git a/gcc/gimplify.cc<http://gimplify.cc> b/gcc/gimplify.cc<http://gimplify.cc>
> index 96845154a92..35d1ea22623 100644
> --- a/gcc/gimplify.cc<http://gimplify.cc>
> +++ b/gcc/gimplify.cc<http://gimplify.cc>
> @@ -1775,9 +1775,9 @@ gimple_add_init_for_auto_var (tree decl,
> 
>   else
>     {
> -      char *decl_name_anonymous = xasprintf ("D.%u", DECL_UID (decl));
> +      char decl_name_anonymous[3 + (HOST_BITS_PER_INT + 2) / 3];
> +      sprintf (decl_name_anonymous, "D.%u", DECL_UID (decl));
>       decl_name = build_string_literal (decl_name_anonymous);
> -      free (decl_name_anonymous);
>     }
> 
>   tree call = build_call_expr_internal_loc (loc, IFN_DEFERRED_INIT,
> diff --git a/gcc/testsuite/g++.dg/pr107411.C b/gcc/testsuite/g++.dg/pr107411.C
> new file mode 100644
> index 00000000000..7eefecae4f3
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/pr107411.C
> @@ -0,0 +1,10 @@
> +/* { dg-do compile } */
> +/* { dg-options "-Werror=uninitialized -ftrivial-auto-var-init=zero"  } */
> +int t();
> +void f(int);
> +
> +void j()
> +{
> +  const int& e = t();
> +  f(e);
> +}
> diff --git a/gcc/tree-ssa-uninit.cc<http://tree-ssa-uninit.cc> b/gcc/tree-ssa-uninit.cc<http://tree-ssa-uninit.cc>
> index c555cf5cd50..9f720ae1f4f 100644
> --- a/gcc/tree-ssa-uninit.cc<http://tree-ssa-uninit.cc>
> +++ b/gcc/tree-ssa-uninit.cc<http://tree-ssa-uninit.cc>
> @@ -224,8 +224,6 @@ warn_uninit (opt_code opt, tree t, tree var, gimple *context,
>     at alt_reloc = temp.
>  */
>  tree lhs_var = NULL_TREE;
> -  tree lhs_var_name = NULL_TREE;
> -  const char *lhs_var_name_str = NULL;
> 
>  /* Get the variable name from the 3rd argument of call.  */
>  tree var_name = gimple_call_arg (var_def_stmt, 2);
> @@ -239,11 +237,22 @@ warn_uninit (opt_code opt, tree t, tree var, gimple *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;
> +  if (lhs_var)
> +    {
> +      /* Get the name string for the LHS_VAR.
> + Refer to routine gimple_add_init_for_auto_var.  */
> +      if (DECL_NAME (lhs_var)
> +  && (strcmp (IDENTIFIER_POINTER (DECL_NAME (lhs_var)),
> +      var_name_str) == 0))
> + return;
> +      else if (!DECL_NAME (lhs_var))
> + {
> +  char lhs_var_name_str_buf[3 + (HOST_BITS_PER_INT + 2) / 3];
> +  sprintf (lhs_var_name_str_buf, "D.%u", DECL_UID (lhs_var));
> +  if (strcmp (lhs_var_name_str_buf, var_name_str) == 0)
> +    return;
> + }
> +    }
>  gcc_assert (var_name_str && var_def_stmt);
> }
>     }
> --
> 2.31.1
> 
> 
>
diff mbox series

Patch

diff --git a/gcc/gimplify.cc<http://gimplify.cc> b/gcc/gimplify.cc<http://gimplify.cc>
index 96845154a92..35d1ea22623 100644
--- a/gcc/gimplify.cc<http://gimplify.cc>
+++ b/gcc/gimplify.cc<http://gimplify.cc>
@@ -1775,9 +1775,9 @@  gimple_add_init_for_auto_var (tree decl,

  else
    {
-      char *decl_name_anonymous = xasprintf ("D.%u", DECL_UID (decl));
+      char decl_name_anonymous[3 + (HOST_BITS_PER_INT + 2) / 3];
+      sprintf (decl_name_anonymous, "D.%u", DECL_UID (decl));
      decl_name = build_string_literal (decl_name_anonymous);
-      free (decl_name_anonymous);
    }

  tree call = build_call_expr_internal_loc (loc, IFN_DEFERRED_INIT,
diff --git a/gcc/testsuite/g++.dg/pr107411.C b/gcc/testsuite/g++.dg/pr107411.C
new file mode 100644
index 00000000000..7eefecae4f3
--- /dev/null
+++ b/gcc/testsuite/g++.dg/pr107411.C
@@ -0,0 +1,10 @@ 
+/* { dg-do compile } */
+/* { dg-options "-Werror=uninitialized -ftrivial-auto-var-init=zero"  } */
+int t();
+void f(int);
+
+void j()
+{
+  const int& e = t();
+  f(e);
+}
diff --git a/gcc/tree-ssa-uninit.cc<http://tree-ssa-uninit.cc> b/gcc/tree-ssa-uninit.cc<http://tree-ssa-uninit.cc>
index c555cf5cd50..9f720ae1f4f 100644
--- a/gcc/tree-ssa-uninit.cc<http://tree-ssa-uninit.cc>
+++ b/gcc/tree-ssa-uninit.cc<http://tree-ssa-uninit.cc>
@@ -224,8 +224,6 @@  warn_uninit (opt_code opt, tree t, tree var, gimple *context,
    at alt_reloc = temp.
 */
 tree lhs_var = NULL_TREE;
-  tree lhs_var_name = NULL_TREE;
-  const char *lhs_var_name_str = NULL;

 /* Get the variable name from the 3rd argument of call.  */
 tree var_name = gimple_call_arg (var_def_stmt, 2);
@@ -239,11 +237,22 @@  warn_uninit (opt_code opt, tree t, tree var, gimple *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;
+  if (lhs_var)
+    {
+      /* Get the name string for the LHS_VAR.
+ Refer to routine gimple_add_init_for_auto_var.  */
+      if (DECL_NAME (lhs_var)
+  && (strcmp (IDENTIFIER_POINTER (DECL_NAME (lhs_var)),
+      var_name_str) == 0))
+ return;
+      else if (!DECL_NAME (lhs_var))
+ {
+  char lhs_var_name_str_buf[3 + (HOST_BITS_PER_INT + 2) / 3];
+  sprintf (lhs_var_name_str_buf, "D.%u", DECL_UID (lhs_var));
+  if (strcmp (lhs_var_name_str_buf, var_name_str) == 0)
+    return;
+ }
+    }
 gcc_assert (var_name_str && var_def_stmt);
}
    }