fix broken -finstrument-functions implementation

Submitted by Nathan Froyd on March 4, 2011, 3 p.m.

Details

Message ID 20110304150014.GA20915@nightcrawler
State New
Headers show

Commit Message

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.

Comments

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 hide | download patch | download mbox

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);