public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] fix broken -finstrument-functions implementation
@ 2011-03-04 15:00 Nathan Froyd
  2011-03-04 15:02 ` Nathan Froyd
  0 siblings, 1 reply; 2+ messages in thread
From: Nathan Froyd @ 2011-03-04 15:00 UTC (permalink / raw)
  To: gcc-patches; +Cc: paul.woergerer

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.


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

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] fix broken -finstrument-functions implementation
  2011-03-04 15:00 [PATCH] fix broken -finstrument-functions implementation Nathan Froyd
@ 2011-03-04 15:02 ` Nathan Froyd
  0 siblings, 0 replies; 2+ messages in thread
From: Nathan Froyd @ 2011-03-04 15:02 UTC (permalink / raw)
  To: gcc-patches; +Cc: paul.woegerer

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

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2011-03-04 15:02 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-03-04 15:00 [PATCH] fix broken -finstrument-functions implementation Nathan Froyd
2011-03-04 15:02 ` Nathan Froyd

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).