Patchwork fix broken -finstrument-functions implementation

login
register
mail settings
Submitter Nathan Froyd
Date March 4, 2011, 3 p.m.
Message ID <20110304150014.GA20915@nightcrawler>
Download mbox | patch
Permalink /patch/85422/
State New
Headers show

Comments

Nathan Froyd - March 4, 2011, 3 p.m.
Paul Wörgerer noticed that -finstrument-functions does do what it says
on the tin; it's been broken for the last three months due to a silly
thinko while gimplifying.  Expanding __builtin_return_address doesn't
complain, as builtins.c thinks appropriate warnings would have been
issued earlier, and simplify generates 0, which gives bogus results in
this case.

He provided the below patch to gimplify.c, which I think qualifies as
obvious (and does not require copyright assignment).  I tweaked the
existing testcase so that we catch mistakes like this in the future.

In the absence of comments, I plan to check this in in 24 hours.

-Nathan

gcc/
2011-03-04  Paul Wörgerer  <paul.woegerer@mentor.com>

	* gimplify.c (gimplify_function_tree): Fix building calls
	to __builtin_return_address.

gcc/testsuite/
2011-03-04  Paul Wörgerer  <paul.woegerer@mentor.com>
	    Nathan Froyd  <froydnj@codesourcery.com>

	* gcc.dg/20001117-1.c: Abort on NULL call_sites.
Nathan Froyd - March 4, 2011, 3:02 p.m.
On Fri, Mar 04, 2011 at 10:00:15AM -0500, Nathan Froyd wrote:
> Paul Wörgerer noticed that -finstrument-functions does do what it says
> on the tin

That's Paul *Wögerer*, fixed in the ChangeLog entries as well; sorry about
that!

-Nathan

Patch

Index: gcc/testsuite/gcc.dg/20001117-1.c
===================================================================
--- gcc/testsuite/gcc.dg/20001117-1.c	(revision 170675)
+++ gcc/testsuite/gcc.dg/20001117-1.c	(working copy)
@@ -24,5 +24,19 @@  int main ()
   exit (0);
 }
 
-void __attribute__((no_instrument_function)) __cyg_profile_func_enter(void *this_fn, void *call_site) { }
-void __attribute__((no_instrument_function)) __cyg_profile_func_exit(void *this_fn, void *call_site) { } 
+/* Abort on non-NULL CALL_SITE to ensure that __builtin_return_address
+   was expanded properly.  */
+void __attribute__((no_instrument_function))
+__cyg_profile_func_enter(void *this_fn, void *call_site)
+{
+  if (call_site == (void *)0)
+    abort ();
+}
+
+void __attribute__((no_instrument_function))
+__cyg_profile_func_exit(void *this_fn, void *call_site)
+{
+  if (call_site == (void *)0)
+    abort ();
+}
+
Index: gcc/gimplify.c
===================================================================
--- gcc/gimplify.c	(revision 170675)
+++ gcc/gimplify.c	(working copy)
@@ -7868,7 +7868,7 @@  gimplify_function_tree (tree fndecl)
       gimple call;
 
       x = implicit_built_in_decls[BUILT_IN_RETURN_ADDRESS];
-      call = gimple_build_call (x, 0);
+      call = gimple_build_call (x, 1, integer_zero_node);
       tmp_var = create_tmp_var (ptr_type_node, "return_addr");
       gimple_call_set_lhs (call, tmp_var);
       gimplify_seq_add_stmt (&cleanup, call);
@@ -7880,7 +7880,7 @@  gimplify_function_tree (tree fndecl)
       tf = gimple_build_try (seq, cleanup, GIMPLE_TRY_FINALLY);
 
       x = implicit_built_in_decls[BUILT_IN_RETURN_ADDRESS];
-      call = gimple_build_call (x, 0);
+      call = gimple_build_call (x, 1, integer_zero_node);
       tmp_var = create_tmp_var (ptr_type_node, "return_addr");
       gimple_call_set_lhs (call, tmp_var);
       gimplify_seq_add_stmt (&body, call);