diff mbox

Patch: add systemtap-style marker to _Unwind_DebugHook

Message ID m3k4i7p9l3.fsf@fleche.redhat.com
State New
Headers show

Commit Message

Tom Tromey Jan. 14, 2011, 7:30 p.m. UTC 
This patch adds a systemtap-style static marker to the existing
_Unwind_DebugHook function in the unwinder.  The benefit of doing this
is that it will enable GDB's exception-handling code to work even in the
absence of debuginfo.

The cost of this probe is very small.  I believe that with the "v3"
probes it expands to a nop in the code, plus an extra note section in
the .o.

Some more info on static markers is here:

http://sourceware.org/systemtap/wiki/AddingUserSpaceProbingToApps

I tested this by building it two ways (once with sdt.h installed and
once without) on x86 F13, and then examining the resulting libgcc.so
using stap and readelf.

Tom

2011-01-14  Tom Tromey  <tromey@redhat.com>

	* unwind-dw2.c: Include sys/sdt.h if it exists.
	(_Unwind_DebugHook): Use STAP_PROBE2.
	* config.in, configure: Rebuild.
	* configure.ac: Check for sys/sdt.h.

Comments

Rainer Orth Jan. 14, 2011, 7:37 p.m. UTC | #1 
Tom Tromey <tromey@redhat.com> writes:

> @@ -1493,7 +1497,11 @@ static void
>  _Unwind_DebugHook (void *cfa __attribute__ ((__unused__)),
>  		   void *handler __attribute__ ((__unused__)))
>  {
> +#ifdef HAVE_SYS_SDT_H
> +  STAP_PROBE2 (libgcc, unwind, cfa, handler);
> +#else
>    asm ("");
> +#endif
>  }
>  
>  /* Install TARGET into CURRENT so that we can return to it.  This is a

This is wrong: Solaris 10+ has <sys/sdt.h> for DTrace, but only defines
DTRACE_* there.

	Rainer
Tom Tromey Jan. 14, 2011, 7:41 p.m. UTC | #2 
>>>>> "Rainer" == Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> writes:

Rainer> This is wrong: Solaris 10+ has <sys/sdt.h> for DTrace, but only defines
Rainer> DTRACE_* there.

Oops, I forgot that this was a compatibility header.

I think I will make it check for the existence of STAP_PROBE2, unless
there is some better plan.  I don't know anything about dtrace and have
no way to test it, so if we want to use the DTRACE_* macro, somebody
will have to help.

Tom
Josh Stone Jan. 14, 2011, 7:45 p.m. UTC | #3 
On 01/14/2011 11:37 AM, Rainer Orth wrote:
> Tom Tromey <tromey@redhat.com> writes:
> 
>> @@ -1493,7 +1497,11 @@ static void
>>  _Unwind_DebugHook (void *cfa __attribute__ ((__unused__)),
>>  		   void *handler __attribute__ ((__unused__)))
>>  {
>> +#ifdef HAVE_SYS_SDT_H
>> +  STAP_PROBE2 (libgcc, unwind, cfa, handler);
>> +#else
>>    asm ("");
>> +#endif
>>  }
>>  
>>  /* Install TARGET into CURRENT so that we can return to it.  This is a
> 
> This is wrong: Solaris 10+ has <sys/sdt.h> for DTrace, but only defines
> DTRACE_* there.

SystemTap's header also defines DTRACE_PROBE2 for compatibility, so you
could use that instead.  The parsing for GDB to use it on Solaris will
be very different though.

Josh
Rainer Orth Jan. 14, 2011, 7:46 p.m. UTC | #4 
Tom Tromey <tromey@redhat.com> writes:

>>>>>> "Rainer" == Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> writes:
>
> Rainer> This is wrong: Solaris 10+ has <sys/sdt.h> for DTrace, but only defines
> Rainer> DTRACE_* there.
>
> Oops, I forgot that this was a compatibility header.
>
> I think I will make it check for the existence of STAP_PROBE2, unless
> there is some better plan.  I don't know anything about dtrace and have

For the moment, that should do.

> no way to test it, so if we want to use the DTRACE_* macro, somebody
> will have to help.

Adding DTrace probes to the unwinder sounds like an excellent idea: I
recently had to debug lots of unwinder issues on Solaris, and such
probes would certainly have helped.  I'll add this to my agenda.

	Rainer
Rainer Orth Jan. 14, 2011, 7:47 p.m. UTC | #5 
Josh Stone <jistone@redhat.com> writes:

>> This is wrong: Solaris 10+ has <sys/sdt.h> for DTrace, but only defines
>> DTRACE_* there.
>
> SystemTap's header also defines DTRACE_PROBE2 for compatibility, so you
> could use that instead.  The parsing for GDB to use it on Solaris will
> be very different though.

Sounds like a plan: I can give it a try.

Thanks.
	Rainer
Roland McGrath Jan. 16, 2011, 8:08 p.m. UTC | #6 
As was mentioned, systemtap's sys/sdt.h has wrapper macros that are
source-compatible with dtrace's sys/sdt.h that you can use without any
material difference.  Systemtap also provides a dtrace wrapper script
intended to be compatible with the build-time uses of dtrace, though using
it adds nothing you need for the systemtap probes to be built right.

In versions of systemtap prior to the forthcoming 1.4, the probes produced
some undesireable overhead in PIC code, in the form of some extra dynamic
relocation records in the resultant DSO, adding to the startup cost in the
dynamic linker.  This added startup cost might be judged unacceptable for
the shared libgcc.so, since in practice it's paid by every process in the
system in its initialization.  So you might want either to make compiling
in the probe conditional on a configure option, or to use an autoconf test
that would only accept the most recent versions of sys/sdt.h (and perhaps
also accept dtrace's versions, if you are going to support dtrace too).


Thanks,
Roland
Tom Tromey Jan. 17, 2011, 2:34 p.m. UTC | #7 
>>>>> "Roland" == Roland McGrath <roland@redhat.com> writes:

Roland> As was mentioned, systemtap's sys/sdt.h has wrapper macros that
Roland> are source-compatible with dtrace's sys/sdt.h that you can use
Roland> without any material difference.

stap's are better because you can use them with a .d file and a new .o
file to define a semaphore variable; what I have read on the web
suggests that Solaris (but not some other DTrace ports) requires this
stuff.  This matters to GCC because it lets us avoid dealing with cross
compilation stuff.

Roland> In versions of systemtap prior to the forthcoming 1.4, the
Roland> probes produced some undesireable overhead in PIC code, in the
Roland> form of some extra dynamic relocation records in the resultant
Roland> DSO, adding to the startup cost in the dynamic linker.  This
Roland> added startup cost might be judged unacceptable for the shared
Roland> libgcc.so, since in practice it's paid by every process in the
Roland> system in its initialization.  So you might want either to make
Roland> compiling in the probe conditional on a configure option, or to
Roland> use an autoconf test that would only accept the most recent
Roland> versions of sys/sdt.h (and perhaps also accept dtrace's
Roland> versions, if you are going to support dtrace too).

I think gcc's configure has too many options already.

What if I check `_SDT_NOTE_TYPE >= 3'?
I'd rather sdt.h advertise its version than try to write a configure
check.  Testing target headers in gcc is a pain.

Tom
Roland McGrath Jan. 17, 2011, 5:06 p.m. UTC | #8 
> What if I check `_SDT_NOTE_TYPE >= 3'?
> I'd rather sdt.h advertise its version than try to write a configure
> check.  Testing target headers in gcc is a pain.

That test would work.  (That macro is not defined at all in earlier versions.)
diff mbox

Patch

diff --git a/gcc/configure.ac b/gcc/configure.ac
index 776c71f..44a963d 100644
--- a/gcc/configure.ac
+++ b/gcc/configure.ac
@@ -4313,6 +4313,16 @@  if test x$gcc_cv_libc_provides_ssp = xyes; then
 	    [Define if your target C library provides stack protector support])
 fi
 
+# Test for <sys/sdt.h> on the target.
+GCC_TARGET_TEMPLATE([HAVE_SYS_SDT_H])
+AC_MSG_CHECKING(sys/sdt.h in the target C library)
+have_sys_sdt_h=no
+if test -f $target_header_dir/sys/sdt.h; then
+  AC_DEFINE(HAVE_SYS_SDT_H, 1,
+            [Define if your target C library provides sys/sdt.h])
+fi
+AC_MSG_RESULT($have_sys_sdt_h)
+
 # Check if TFmode long double should be used by default or not.
 # Some glibc targets used DFmode long double, but with glibc 2.4
 # and later they can use TFmode.
diff --git a/gcc/unwind-dw2.c b/gcc/unwind-dw2.c
index 2ea9adb..ca5e95c 100644
--- a/gcc/unwind-dw2.c
+++ b/gcc/unwind-dw2.c
@@ -1,6 +1,6 @@ 
 /* DWARF2 exception handling and frame unwind runtime interface routines.
    Copyright (C) 1997, 1998, 1999, 2000, 2001, 2002, 2003, 2004, 2005, 2006,
-   2008, 2009, 2010  Free Software Foundation, Inc.
+   2008, 2009, 2010, 2011  Free Software Foundation, Inc.
 
    This file is part of GCC.
 
@@ -37,6 +37,10 @@ 
 #include "gthr.h"
 #include "unwind-dw2.h"
 
+#ifdef HAVE_SYS_SDT_H
+#include <sys/sdt.h>
+#endif
+
 #ifndef __USING_SJLJ_EXCEPTIONS__
 
 #ifndef STACK_GROWS_DOWNWARD
@@ -1493,7 +1497,11 @@  static void
 _Unwind_DebugHook (void *cfa __attribute__ ((__unused__)),
 		   void *handler __attribute__ ((__unused__)))
 {
+#ifdef HAVE_SYS_SDT_H
+  STAP_PROBE2 (libgcc, unwind, cfa, handler);
+#else
   asm ("");
+#endif
 }
 
 /* Install TARGET into CURRENT so that we can return to it.  This is a