[PATCH] Fix removing of df problem in df_finish_pass

[PATCH] Fix removing of df problem in df_finish_pass

[Thomas Preud'homme]

Hi,

In df_finish_pass, optional problems are removed manually making non null
entries in df->problems_in_order non contiguous. This may lead to null pointer
dereference when accessing all problems from df->problems_in_order[0] to
df->problems_in_order[df->num_problems_defined - 1] and miss some other
problems. Such a scenario was actually encountered when working on a patch.
This patch use the existing function df_remove_problem to do the deletion,
which require iterating on problems via the df->problems_by_index[] array
since each call mess up with df->num_problems_defined and order of
problems in df->problems_in_order[].

ChangeLog entry is as follows:

2015-02-12  Thomas Preud'homme  <thomas.preudhomme@arm.com>

        * df-core.c (df_finish_pass): Iterate over df->problems_by_index[] and
        use df_remove_problem rather than manually removing problems, living
        holes in df->problems_in_order[].

diff --git a/gcc/df-core.c b/gcc/df-core.c
index 82f1364..67040a1 100644
--- a/gcc/df-core.c
+++ b/gcc/df-core.c
@@ -642,7 +642,6 @@ void
 df_finish_pass (bool verify ATTRIBUTE_UNUSED)
 {
   int i;
-  int removed = 0;
 
 #ifdef ENABLE_DF_CHECKING
   int saved_flags;
@@ -658,21 +657,15 @@ df_finish_pass (bool verify ATTRIBUTE_UNUSED)
   saved_flags = df->changeable_flags;
 #endif
 
-  for (i = 0; i < df->num_problems_defined; i++)
+  /* We iterate over problems by index as each problem removed will
+     lead to problems_in_order to be reordered.  */
+  for (i = 0; i < DF_LAST_PROBLEM_PLUS1; i++)
     {
-      struct dataflow *dflow = df->problems_in_order[i];
-      struct df_problem *problem = dflow->problem;
+      struct dataflow *dflow = df->problems_by_index[i];
 
-      if (dflow->optional_p)
-	{
-	  gcc_assert (problem->remove_problem_fun);
-	  (problem->remove_problem_fun) ();
-	  df->problems_in_order[i] = NULL;
-	  df->problems_by_index[problem->id] = NULL;
-	  removed++;
-	}
+      if (dflow && dflow->optional_p)
+	df_remove_problem (dflow);
     }
-  df->num_problems_defined -= removed;
 
   /* Clear all of the flags.  */
   df->changeable_flags = 0;


Testsuite was run with a bootstrapped x86_64 native compiler and an
arm-none-eabi GCC cross-compiler targetting Cortex-M3 without any
regression.

Although the problem is real, it doesn't seem that GCC hits it now
(I stumbled upon it while working on a patch). Therefore I'm not sure
if this should go in stage4 or not. Please advise me on this.

Ok for trunk/stage1?

Best regards,

Thomas

Re: [PATCH] Fix removing of df problem in df_finish_pass

[Bernhard Reutner-Fischer]

On February 27, 2015 12:42:43 PM GMT+01:00, Thomas Preud'homme <thomas.preudhomme@arm.com> wrote:
>Hi,
>
>In df_finish_pass, optional problems are removed manually making non
>null
>entries in df->problems_in_order non contiguous. This may lead to null
>pointer
>dereference when accessing all problems from df->problems_in_order[0]
>to
>df->problems_in_order[df->num_problems_defined - 1] and miss some other
>problems. Such a scenario was actually encountered when working on a
>patch.
>This patch use the existing function df_remove_problem to do the
>deletion,
>which require iterating on problems via the df->problems_by_index[]
>array
>since each call mess up with df->num_problems_defined and order of
>problems in df->problems_in_order[].
>
>ChangeLog entry is as follows:
>
>2015-02-12  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>
> * df-core.c (df_finish_pass): Iterate over df->problems_by_index[] and
>   use df_remove_problem rather than manually removing problems, living

leaving

Thanks,

>        holes in df->problems_in_order[].
>
>diff --git a/gcc/df-core.c b/gcc/df-core.c
>index 82f1364..67040a1 100644
>--- a/gcc/df-core.c
>+++ b/gcc/df-core.c
>@@ -642,7 +642,6 @@ void
> df_finish_pass (bool verify ATTRIBUTE_UNUSED)
> {
>   int i;
>-  int removed = 0;
> 
> #ifdef ENABLE_DF_CHECKING
>   int saved_flags;
>@@ -658,21 +657,15 @@ df_finish_pass (bool verify ATTRIBUTE_UNUSED)
>   saved_flags = df->changeable_flags;
> #endif
> 
>-  for (i = 0; i < df->num_problems_defined; i++)
>+  /* We iterate over problems by index as each problem removed will
>+     lead to problems_in_order to be reordered.  */
>+  for (i = 0; i < DF_LAST_PROBLEM_PLUS1; i++)
>     {
>-      struct dataflow *dflow = df->problems_in_order[i];
>-      struct df_problem *problem = dflow->problem;
>+      struct dataflow *dflow = df->problems_by_index[i];
> 
>-      if (dflow->optional_p)
>-	{
>-	  gcc_assert (problem->remove_problem_fun);
>-	  (problem->remove_problem_fun) ();
>-	  df->problems_in_order[i] = NULL;
>-	  df->problems_by_index[problem->id] = NULL;
>-	  removed++;
>-	}
>+      if (dflow && dflow->optional_p)
>+	df_remove_problem (dflow);
>     }
>-  df->num_problems_defined -= removed;
> 
>   /* Clear all of the flags.  */
>   df->changeable_flags = 0;
>
>
>Testsuite was run with a bootstrapped x86_64 native compiler and an
>arm-none-eabi GCC cross-compiler targetting Cortex-M3 without any
>regression.
>
>Although the problem is real, it doesn't seem that GCC hits it now
>(I stumbled upon it while working on a patch). Therefore I'm not sure
>if this should go in stage4 or not. Please advise me on this.
>
>Ok for trunk/stage1?
>
>Best regards,
>
>Thomas

RE: [PATCH] Fix removing of df problem in df_finish_pass

[Thomas Preud'homme]

> From: Bernhard Reutner-Fischer [mailto:rep.dot.nop@gmail.com]
> Sent: Saturday, February 28, 2015 4:00 AM
> >   use df_remove_problem rather than manually removing problems,
> living
> 
> leaving

Indeed. Please find updated changelog below:

2015-03-03  Thomas Preud'homme  <thomas.preudhomme@arm.com>

	* df-core.c (df_finish_pass): Iterate over df->problems_by_index[] and
	use df_remove_problem rather than manually removing problems, leaving
	holes in df->problems_in_order[].

Best regards,

Thomas