sanopt: Avoid crash on anonymous parameter [PR93436]
diff mbox series

Message ID 20200126001320.1283996-1-polacek@redhat.com
State New
Headers show
Series
  • sanopt: Avoid crash on anonymous parameter [PR93436]
Related show

Commit Message

Marek Polacek Jan. 26, 2020, 12:13 a.m. UTC
Here we crash when using -fsanitize=address -fdump-tree-sanopt because
the dumping code uses IDENTIFIER_POINTER on a null DECL_NAME.  Instead,
we can print "<anonymous>" in such a case.  Or we could avoid printing
that diagnostic altogether.

I don't think this warrants a testcase.

Tested x86_64-linux, ok for trunk and 9?

2020-01-25  Marek Polacek  <polacek@redhat.com>

	PR tree-optimization/93436
	* sanopt.c (sanitize_rewrite_addressable_params): Avoid crash on
	null DECL_NAME.
---
 gcc/sanopt.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)


base-commit: 05107d4e4ccd11ecc8712d6e271fcb4b5f17060f

Comments

Jakub Jelinek Jan. 26, 2020, 11:09 a.m. UTC | #1
On Sat, Jan 25, 2020 at 07:13:20PM -0500, Marek Polacek wrote:
> Here we crash when using -fsanitize=address -fdump-tree-sanopt because
> the dumping code uses IDENTIFIER_POINTER on a null DECL_NAME.  Instead,
> we can print "<anonymous>" in such a case.  Or we could avoid printing
> that diagnostic altogether.
> 
> I don't think this warrants a testcase.
> 
> Tested x86_64-linux, ok for trunk and 9?

Wouldn't it be better to:
	  if (dump_file)
	    {
	      fprintf (dump_file,
		       "Rewriting parameter whose address is taken: ");
	      print_generic_expr (dump_file, arg, dump_flags);
	      fputc ('\n', dump_file);
	    }
or so?  That way, we can print D.1234 for those, but user has a way
to override it to D.xxxx etc.

> 2020-01-25  Marek Polacek  <polacek@redhat.com>
> 
> 	PR tree-optimization/93436
> 	* sanopt.c (sanitize_rewrite_addressable_params): Avoid crash on
> 	null DECL_NAME.

	Jakub
Marek Polacek Jan. 26, 2020, 9:18 p.m. UTC | #2
On Sun, Jan 26, 2020 at 12:09:09PM +0100, Jakub Jelinek wrote:
> On Sat, Jan 25, 2020 at 07:13:20PM -0500, Marek Polacek wrote:
> > Here we crash when using -fsanitize=address -fdump-tree-sanopt because
> > the dumping code uses IDENTIFIER_POINTER on a null DECL_NAME.  Instead,
> > we can print "<anonymous>" in such a case.  Or we could avoid printing
> > that diagnostic altogether.
> > 
> > I don't think this warrants a testcase.
> > 
> > Tested x86_64-linux, ok for trunk and 9?
> 
> Wouldn't it be better to:
> 	  if (dump_file)
> 	    {
> 	      fprintf (dump_file,
> 		       "Rewriting parameter whose address is taken: ");
> 	      print_generic_expr (dump_file, arg, dump_flags);
> 	      fputc ('\n', dump_file);
> 	    }
> or so?  That way, we can print D.1234 for those, but user has a way
> to override it to D.xxxx etc.

Sure, that's better, thanks.

Here we crash when using -fsanitize=address -fdump-tree-sanopt because
the dumping code uses IDENTIFIER_POINTER on a null DECL_NAME.  Instead,
we can use print_generic_expr.

Tested x86_64-linux, ok for trunk and 9?

2020-01-26  Marek Polacek  <polacek@redhat.com>

	PR tree-optimization/93436
	* sanopt.c (sanitize_rewrite_addressable_params): Avoid crash on
	null DECL_NAME.
---
 gcc/sanopt.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/gcc/sanopt.c b/gcc/sanopt.c
index 619aae45a15..0788eef597e 100644
--- a/gcc/sanopt.c
+++ b/gcc/sanopt.c
@@ -1174,9 +1174,12 @@ sanitize_rewrite_addressable_params (function *fun)
 	    continue;
 
 	  if (dump_file)
-	    fprintf (dump_file,
-		     "Rewriting parameter whose address is taken: %s\n",
-		     IDENTIFIER_POINTER (DECL_NAME (arg)));
+	    {
+	      fprintf (dump_file,
+		       "Rewriting parameter whose address is taken: ");
+	      print_generic_expr (dump_file, arg, dump_flags);
+	      fputc ('\n', dump_file);
+	    }
 
 	  SET_DECL_PT_UID (var, DECL_PT_UID (arg));
 

base-commit: 9664b52aeb3db9ae35bba1766d677790c8a461ef
Jakub Jelinek Jan. 26, 2020, 9:19 p.m. UTC | #3
On Sun, Jan 26, 2020 at 04:18:33PM -0500, Marek Polacek wrote:
> 2020-01-26  Marek Polacek  <polacek@redhat.com>
> 
> 	PR tree-optimization/93436
> 	* sanopt.c (sanitize_rewrite_addressable_params): Avoid crash on
> 	null DECL_NAME.

LGTM, thanks.

> ---
>  gcc/sanopt.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/gcc/sanopt.c b/gcc/sanopt.c
> index 619aae45a15..0788eef597e 100644
> --- a/gcc/sanopt.c
> +++ b/gcc/sanopt.c
> @@ -1174,9 +1174,12 @@ sanitize_rewrite_addressable_params (function *fun)
>  	    continue;
>  
>  	  if (dump_file)
> -	    fprintf (dump_file,
> -		     "Rewriting parameter whose address is taken: %s\n",
> -		     IDENTIFIER_POINTER (DECL_NAME (arg)));
> +	    {
> +	      fprintf (dump_file,
> +		       "Rewriting parameter whose address is taken: ");
> +	      print_generic_expr (dump_file, arg, dump_flags);
> +	      fputc ('\n', dump_file);
> +	    }
>  
>  	  SET_DECL_PT_UID (var, DECL_PT_UID (arg));
>  
> 
> base-commit: 9664b52aeb3db9ae35bba1766d677790c8a461ef

	Jakub
Martin Liška Jan. 27, 2020, 11:05 a.m. UTC | #4
On 1/26/20 10:18 PM, Marek Polacek wrote:
> Sure, that's better, thanks.

Thank you for the fix Marek.

Martin

Patch
diff mbox series

diff --git a/gcc/sanopt.c b/gcc/sanopt.c
index 619aae45a15..63fd68d4ad1 100644
--- a/gcc/sanopt.c
+++ b/gcc/sanopt.c
@@ -1176,7 +1176,9 @@  sanitize_rewrite_addressable_params (function *fun)
 	  if (dump_file)
 	    fprintf (dump_file,
 		     "Rewriting parameter whose address is taken: %s\n",
-		     IDENTIFIER_POINTER (DECL_NAME (arg)));
+		     (DECL_NAME (arg)
+		      ? IDENTIFIER_POINTER (DECL_NAME (arg))
+		      : "<anonymous>"));
 
 	  SET_DECL_PT_UID (var, DECL_PT_UID (arg));