sanopt: Avoid crash on anonymous parameter [PR93436]

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.
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
-- 
Marek Polacek • Red Hat, Inc. • 300 A St, Boston, MA

Comments

Jakub Jelinek Jan. 26, 2020, 11:09 a.m. | #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. | #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
-- 
Marek Polacek • Red Hat, Inc. • 300 A St, Boston, MA
Jakub Jelinek Jan. 26, 2020, 9:19 p.m. | #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. | #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 --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));