public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [V2][PATCH] Fixing PR107411
@ 2023-02-21 14:46 Qing Zhao
  2023-02-27 14:30 ` Fwd: " Qing Zhao
  0 siblings, 1 reply; 3+ messages in thread
From: Qing Zhao @ 2023-02-21 14:46 UTC (permalink / raw)
  To: jakub, rguenther; +Cc: gcc-patches, Qing Zhao

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.
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 (gimple_add_init_for_auto_var): Use sprintf to replace
	xasprintf.
	* 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                 |  4 ++--
 gcc/testsuite/g++.dg/pr107411.C | 10 ++++++++++
 gcc/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 b/gcc/gimplify.cc
index 96845154a92..35d1ea22623 100644
--- a/gcc/gimplify.cc
+++ b/gcc/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 b/gcc/tree-ssa-uninit.cc
index c555cf5cd50..9f720ae1f4f 100644
--- a/gcc/tree-ssa-uninit.cc
+++ b/gcc/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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Fwd: [V2][PATCH] Fixing PR107411
  2023-02-21 14:46 [V2][PATCH] Fixing PR107411 Qing Zhao
@ 2023-02-27 14:30 ` Qing Zhao
  2023-02-28 10:02   ` Richard Biener
  0 siblings, 1 reply; 3+ messages in thread
From: Qing Zhao @ 2023-02-27 14:30 UTC (permalink / raw)
  To: Jakub Jelinek, Richard Biener; +Cc: gcc Patches

[-- Attachment #1: Type: text/plain, Size: 5218 bytes --]

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

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



^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: Fwd: [V2][PATCH] Fixing PR107411
  2023-02-27 14:30 ` Fwd: " Qing Zhao
@ 2023-02-28 10:02   ` Richard Biener
  0 siblings, 0 replies; 3+ messages in thread
From: Richard Biener @ 2023-02-28 10:02 UTC (permalink / raw)
  To: Qing Zhao; +Cc: Jakub Jelinek, gcc Patches

[-- Attachment #1: Type: text/plain, Size: 5647 bytes --]

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
> 
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg,
Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman;
HRB 36809 (AG Nuernberg)

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2023-02-28 10:02 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-21 14:46 [V2][PATCH] Fixing PR107411 Qing Zhao
2023-02-27 14:30 ` Fwd: " Qing Zhao
2023-02-28 10:02   ` Richard Biener

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).