gOlogy: do not change code in isolate-paths for warnings only

Message ID orefcjpvv5.fsf@lxoliva.fsfla.org
State New
Headers show
Series
  • gOlogy: do not change code in isolate-paths for warnings only
Related show

Commit Message

Alexandre Oliva Oct. 21, 2018, 8:06 a.m.
The isolate-paths pass is activated by various -f flags, but also by
-Wnull-dereference.  Most of its codegen changes are conditioned on at
least one of the -f flags, but those that detect, warn about and
isolate paths that return the address of local variables are enabled
even if the pass is activated only by -Wnull-dereference.

-W flags should not cause codegen changes, so this patch makes the
codegen changes conditional on the presence of any of the -f flags
that activate the pass.  Should we have a separate option to activate
only this kind of transformation?

Regstrapped on x86_64- and i686-linux-gnu.  Ok to install?


for  gcc/ChangeLog

	* gimple-ssa-isolate-paths.c
	(find_implicit_erroneous_behavior): Do not change code if the
	pass is running for warnings only.
	(find_explicit_erroneous_behavior): Likewise.
---
 gcc/gimple-ssa-isolate-paths.c |   17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)



-- 
Alexandre Oliva, freedom fighter   https://FSFLA.org/blogs/lxo
Be the change, be Free!         FSF Latin America board member
GNU Toolchain Engineer                Free Software Evangelist
Hay que enGNUrecerse, pero sin perder la terGNUra jamás-GNUChe

Comments

Richard Biener Oct. 21, 2018, 5:31 p.m. | #1
On October 21, 2018 10:06:06 AM GMT+02:00, Alexandre Oliva <aoliva@redhat.com> wrote:
>The isolate-paths pass is activated by various -f flags, but also by

>-Wnull-dereference.  Most of its codegen changes are conditioned on at

>least one of the -f flags, but those that detect, warn about and

>isolate paths that return the address of local variables are enabled

>even if the pass is activated only by -Wnull-dereference.

>

>-W flags should not cause codegen changes, so this patch makes the

>codegen changes conditional on the presence of any of the -f flags

>that activate the pass.  Should we have a separate option to activate

>only this kind of transformation?


I don't think this is necessary. 

>Regstrapped on x86_64- and i686-linux-gnu.  Ok to install?


OK. 

Richard. 

>

>for  gcc/ChangeLog

>

>	* gimple-ssa-isolate-paths.c

>	(find_implicit_erroneous_behavior): Do not change code if the

>	pass is running for warnings only.

>	(find_explicit_erroneous_behavior): Likewise.

>---

> gcc/gimple-ssa-isolate-paths.c |   17 +++++++++++++----

> 1 file changed, 13 insertions(+), 4 deletions(-)

>

>diff --git a/gcc/gimple-ssa-isolate-paths.c

>b/gcc/gimple-ssa-isolate-paths.c

>index e1fab61bedab..880836c21aa7 100644

>--- a/gcc/gimple-ssa-isolate-paths.c

>+++ b/gcc/gimple-ssa-isolate-paths.c

>@@ -431,7 +431,9 @@ find_implicit_erroneous_behavior (void)

> 					"declared here");

> 			  }

> 

>-			  if (gimple_bb (use_stmt) == bb)

>+			  if ((flag_isolate_erroneous_paths_dereference

>+			       || flag_isolate_erroneous_paths_attribute)

>+			      && gimple_bb (use_stmt) == bb)

> 			    {

> 			      duplicate = isolate_path (bb, duplicate, e,

> 							use_stmt, lhs, true);

>@@ -553,9 +555,16 @@ find_explicit_erroneous_behavior (void)

> 			  inform (DECL_SOURCE_LOCATION(valbase),

> 				  "declared here");

> 		      }

>-		      tree zero = build_zero_cst (TREE_TYPE (val));

>-		      gimple_return_set_retval (return_stmt, zero);

>-		      update_stmt (stmt);

>+

>+		      /* Do not modify code if the user only asked for

>+			 warnings.  */

>+		      if (flag_isolate_erroneous_paths_dereference

>+			  || flag_isolate_erroneous_paths_attribute)

>+			{

>+			  tree zero = build_zero_cst (TREE_TYPE (val));

>+			  gimple_return_set_retval (return_stmt, zero);

>+			  update_stmt (stmt);

>+			}

> 		    }

> 		}

> 	    }

Patch

diff --git a/gcc/gimple-ssa-isolate-paths.c b/gcc/gimple-ssa-isolate-paths.c
index e1fab61bedab..880836c21aa7 100644
--- a/gcc/gimple-ssa-isolate-paths.c
+++ b/gcc/gimple-ssa-isolate-paths.c
@@ -431,7 +431,9 @@  find_implicit_erroneous_behavior (void)
 					"declared here");
 			  }
 
-			  if (gimple_bb (use_stmt) == bb)
+			  if ((flag_isolate_erroneous_paths_dereference
+			       || flag_isolate_erroneous_paths_attribute)
+			      && gimple_bb (use_stmt) == bb)
 			    {
 			      duplicate = isolate_path (bb, duplicate, e,
 							use_stmt, lhs, true);
@@ -553,9 +555,16 @@  find_explicit_erroneous_behavior (void)
 			  inform (DECL_SOURCE_LOCATION(valbase),
 				  "declared here");
 		      }
-		      tree zero = build_zero_cst (TREE_TYPE (val));
-		      gimple_return_set_retval (return_stmt, zero);
-		      update_stmt (stmt);
+
+		      /* Do not modify code if the user only asked for
+			 warnings.  */
+		      if (flag_isolate_erroneous_paths_dereference
+			  || flag_isolate_erroneous_paths_attribute)
+			{
+			  tree zero = build_zero_cst (TREE_TYPE (val));
+			  gimple_return_set_retval (return_stmt, zero);
+			  update_stmt (stmt);
+			}
 		    }
 		}
 	    }