Fix -fcompare-debug issue in cp_maybe_instrument_return (PR sanitizer/86406)

Message ID 20180710073404.GD7166@tucnak
State New
Headers show
Series
  • Fix -fcompare-debug issue in cp_maybe_instrument_return (PR sanitizer/86406)
Related show

Commit Message

Jakub Jelinek July 10, 2018, 7:34 a.m.
Hi!

cp_maybe_instrument_return is looking for a return stmt at the end of
function to decide whether to omit -fsanitize=return instrumentation or
__builtin_unreachable addition.  If a STATEMENT_LIST has a return followed
by DEBUG_BEGIN_STMT (or multiple of them), it doesn't find the return
though.

Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
trunk?

2018-07-10  Jakub Jelinek  <jakub@redhat.com>

	PR sanitizer/86406
	* cp-gimplify.c (cp_maybe_instrument_return): Skip trailing
	DEBUG_BEGIN_STMTs.

	* g++.dg/ubsan/pr86406.C: New test.


	Jakub

Comments

Richard Biener July 10, 2018, 8:01 a.m. | #1
On Tue, 10 Jul 2018, Jakub Jelinek wrote:

> Hi!
> 
> cp_maybe_instrument_return is looking for a return stmt at the end of
> function to decide whether to omit -fsanitize=return instrumentation or
> __builtin_unreachable addition.  If a STATEMENT_LIST has a return followed
> by DEBUG_BEGIN_STMT (or multiple of them), it doesn't find the return
> though.
> 
> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
> trunk?

OK.  This also affects the branch?

Thanks,
Richard.

> 2018-07-10  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR sanitizer/86406
> 	* cp-gimplify.c (cp_maybe_instrument_return): Skip trailing
> 	DEBUG_BEGIN_STMTs.
> 
> 	* g++.dg/ubsan/pr86406.C: New test.
> 
> --- gcc/cp/cp-gimplify.c.jj	2018-07-05 11:41:51.687718588 +0200
> +++ gcc/cp/cp-gimplify.c	2018-07-09 09:57:16.368775004 +0200
> @@ -1621,6 +1621,13 @@ cp_maybe_instrument_return (tree fndecl)
>  	case STATEMENT_LIST:
>  	  {
>  	    tree_stmt_iterator i = tsi_last (t);
> +	    while (!tsi_end_p (i))
> +	      {
> +		tree p = tsi_stmt (i);
> +		if (TREE_CODE (p) != DEBUG_BEGIN_STMT)
> +		  break;
> +		tsi_prev (&i);
> +	      }
>  	    if (!tsi_end_p (i))
>  	      {
>  		t = tsi_stmt (i);
> --- gcc/testsuite/g++.dg/ubsan/pr86406.C.jj	2018-07-09 09:58:57.362878125 +0200
> +++ gcc/testsuite/g++.dg/ubsan/pr86406.C	2018-07-09 09:58:37.716858063 +0200
> @@ -0,0 +1,33 @@
> +// PR sanitizer/86406
> +// { dg-do compile }
> +// { dg-options "-fcompare-debug -fsanitize=undefined -g -O1" }
> +
> +typedef enum { } cmd_status;
> +class ECell;
> +class ECell_const_ptr { };
> +class ECell_ptr
> +{
> +  ECell *mp_element;
> +  ECell *getPointer () const { return mp_element; }
> +public:
> +  operator  ECell_const_ptr () const { return ECell_const_ptr(); }
> +};
> +
> +extern ECell_ptr NULL_CELL;
> +class VwUI_2DCellLayerView;
> +class view_cell_layoutImpl
> +{
> +  cmd_status handleChangeFlags (VwUI_2DCellLayerView *
> +                                      p_ui_celllayerview,
> +                                      ECell_const_ptr p_peekCell);
> +  cmd_status openCellLayoutView ();
> +};
> +
> +cmd_status
> +view_cell_layoutImpl::openCellLayoutView ()
> +{
> +  ECell_const_ptr pcell = NULL_CELL;
> +  VwUI_2DCellLayerView *p_user_interface;
> +  return handleChangeFlags (p_user_interface, pcell);
> +  ;
> +}
> 
> 	Jakub
> 
>
Jakub Jelinek July 10, 2018, 8:03 a.m. | #2
On Tue, Jul 10, 2018 at 10:01:10AM +0200, Richard Biener wrote:
> On Tue, 10 Jul 2018, Jakub Jelinek wrote:
> > cp_maybe_instrument_return is looking for a return stmt at the end of
> > function to decide whether to omit -fsanitize=return instrumentation or
> > __builtin_unreachable addition.  If a STATEMENT_LIST has a return followed
> > by DEBUG_BEGIN_STMT (or multiple of them), it doesn't find the return
> > though.
> > 
> > Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
> > trunk?
> 
> OK.  This also affects the branch?

8.x only, will commit it there too.

> > 2018-07-10  Jakub Jelinek  <jakub@redhat.com>
> > 
> > 	PR sanitizer/86406
> > 	* cp-gimplify.c (cp_maybe_instrument_return): Skip trailing
> > 	DEBUG_BEGIN_STMTs.
> > 
> > 	* g++.dg/ubsan/pr86406.C: New test.

	Jakub

Patch

--- gcc/cp/cp-gimplify.c.jj	2018-07-05 11:41:51.687718588 +0200
+++ gcc/cp/cp-gimplify.c	2018-07-09 09:57:16.368775004 +0200
@@ -1621,6 +1621,13 @@  cp_maybe_instrument_return (tree fndecl)
 	case STATEMENT_LIST:
 	  {
 	    tree_stmt_iterator i = tsi_last (t);
+	    while (!tsi_end_p (i))
+	      {
+		tree p = tsi_stmt (i);
+		if (TREE_CODE (p) != DEBUG_BEGIN_STMT)
+		  break;
+		tsi_prev (&i);
+	      }
 	    if (!tsi_end_p (i))
 	      {
 		t = tsi_stmt (i);
--- gcc/testsuite/g++.dg/ubsan/pr86406.C.jj	2018-07-09 09:58:57.362878125 +0200
+++ gcc/testsuite/g++.dg/ubsan/pr86406.C	2018-07-09 09:58:37.716858063 +0200
@@ -0,0 +1,33 @@ 
+// PR sanitizer/86406
+// { dg-do compile }
+// { dg-options "-fcompare-debug -fsanitize=undefined -g -O1" }
+
+typedef enum { } cmd_status;
+class ECell;
+class ECell_const_ptr { };
+class ECell_ptr
+{
+  ECell *mp_element;
+  ECell *getPointer () const { return mp_element; }
+public:
+  operator  ECell_const_ptr () const { return ECell_const_ptr(); }
+};
+
+extern ECell_ptr NULL_CELL;
+class VwUI_2DCellLayerView;
+class view_cell_layoutImpl
+{
+  cmd_status handleChangeFlags (VwUI_2DCellLayerView *
+                                      p_ui_celllayerview,
+                                      ECell_const_ptr p_peekCell);
+  cmd_status openCellLayoutView ();
+};
+
+cmd_status
+view_cell_layoutImpl::openCellLayoutView ()
+{
+  ECell_const_ptr pcell = NULL_CELL;
+  VwUI_2DCellLayerView *p_user_interface;
+  return handleChangeFlags (p_user_interface, pcell);
+  ;
+}