Re: [PATCH] Implement stap probe on ARM's unwinder

[Ramana Radhakrishnan <ramana dot radhakrishnan at linaro dot org>]

Sergio: Other than a few minor tweaks to the Changelog it largely
looks obvious to me.

Bernd, could you take another look at this since this is now shared
with the c6x backend ?

> Thanks,
>
> Sergio.
>
> diff --git a/libgcc/ChangeLog b/libgcc/ChangeLog
> index 305e8ad..f6e9dec 100644
> --- a/libgcc/ChangeLog
> +++ b/libgcc/ChangeLog
> @@ -1,3 +1,15 @@
> +2011-11-22 ?Sergio Durigan Junior ?<sergiodj@redhat.com>
> +
> + ? ? ? Implement ARM Unwinder SystemTap probe.

This line is not required.

> + ? ? ? * unwind-arm-common.inc: Include `tconfig.h', `tsystem.h' and
> + ? ? ? `sys/sdt.h'.
> + ? ? ? (_Unwind_DebugHook): New function.
> + ? ? ? (uw_restore_core_regs): New define.
> + ? ? ? (unwind_phase2): Use `uw_restore_core_regs' instead of
> + ? ? ? `restore_core_regs'.

You don't need the `' quoting of the function names in the ChangeLog.

> + ? ? ? (unwind_phase2_forced): Likewise.
> + ? ? ? (__gnu_Unwind_Resume): Likewise.
> +
> ?2011-11-22 ?Iain Sandoe ?<iains@gcc.gnu.org>
>
> ? ? ? ?* config/darwin-crt-tm.c: New file.
> diff --git a/libgcc/unwind-arm-common.inc b/libgcc/unwind-arm-common.inc
> index 0713056..bf16902 100644
> --- a/libgcc/unwind-arm-common.inc
> +++ b/libgcc/unwind-arm-common.inc
> @@ -21,8 +21,15 @@
> ? ?see the files COPYING3 and COPYING.RUNTIME respectively. ?If not, see
> ? ?<http://www.gnu.org/licenses/>. ?*/
>
> +#include "tconfig.h"
> +#include "tsystem.h"
> ?#include "unwind.h"
>
> +/* Used for SystemTap unwinder probe. ?*/
> +#ifdef HAVE_SYS_SDT_H
> +#include <sys/sdt.h>
> +#endif
> +
> ?/* We add a prototype for abort here to avoid creating a dependency on
> ? ?target headers. ?*/
> ?extern void abort (void);
> @@ -105,6 +112,44 @@ static inline _uw selfrel_offset31 (const _uw *p);
>
> ?static _uw __gnu_unwind_get_pr_addr (int idx);
>
> +static void _Unwind_DebugHook (void *, void *)
> + ?__attribute__ ((__noinline__, __used__, __noclone__));
> +
> +/* This function is called during unwinding. ?It is intended as a hook
> + ? for a debugger to intercept exceptions. ?CFA is the CFA of the
> + ? target frame. ?HANDLER is the PC to which control will be
> + ? transferred. ?*/
> +
> +static void
> +_Unwind_DebugHook (void *cfa __attribute__ ((__unused__)),
> + ? ? ? ? ? ? ? ? ?void *handler __attribute__ ((__unused__)))
> +{
> + ?/* We only want to use stap probes starting with v3. ?Earlier
> + ? ? versions added too much startup cost. ?*/
> +#if defined (HAVE_SYS_SDT_H) && defined (STAP_PROBE2) && _SDT_NOTE_TYPE >= 3
> + ?STAP_PROBE2 (libgcc, unwind, cfa, handler);
> +#else
> + ?asm ("");
> +#endif
> +}
> +
> +/* This is a wrapper to be called when we need to restore core registers.
> + ? It will call `_Unwind_DebugHook' before restoring the registers, thus
> + ? making it possible to intercept and debug exceptions.
> +
> + ? When calling `_Unwind_DebugHook', the first argument (the CFA) is zero
> + ? because we are not interested in it. ?However, it must be there (even
> + ? being zero) because GDB expects to find it when using the probe. ?*/
> +
> +#define uw_restore_core_regs(TARGET, CORE) ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
> + ?do ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
> + ? ?{ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
> + ? ? ?void *handler = __builtin_frob_return_addr ((void *) VRS_PC (TARGET)); ?\
> + ? ? ?_Unwind_DebugHook (0, handler); ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
> + ? ? ?restore_core_regs (CORE); ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
> + ? ?} ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
> + ?while (0)
> +
> ?/* Perform a binary search for RETURN_ADDRESS in TABLE. ?The table contains
> ? ?NREC entries. ?*/
>
> @@ -253,8 +298,8 @@ unwind_phase2 (_Unwind_Control_Block * ucbp, phase2_vrs * vrs)
>
> ? if (pr_result != _URC_INSTALL_CONTEXT)
> ? ? abort();
> -
> - ?restore_core_regs (&vrs->core);
> +
> + ?uw_restore_core_regs (vrs, &vrs->core);
> ?}
>
> ?/* Perform phase2 forced unwinding. ?*/
> @@ -339,7 +384,7 @@ unwind_phase2_forced (_Unwind_Control_Block *ucbp, phase2_vrs *entry_vrs,
> ? ? ? return _URC_FAILURE;
> ? ? }
>
> - ?restore_core_regs (&saved_vrs.core);
> + ?uw_restore_core_regs (&saved_vrs, &saved_vrs.core);
> ?}
>
> ?/* This is a very limited implementation of _Unwind_GetCFA. ?It returns
> @@ -450,7 +495,7 @@ __gnu_Unwind_Resume (_Unwind_Control_Block * ucbp, phase2_vrs * entry_vrs)
> ? ? {
> ? ? case _URC_INSTALL_CONTEXT:
> ? ? ? /* Upload the registers to enter the landing pad. ?*/
> - ? ? ?restore_core_regs (&entry_vrs->core);
> + ? ? ?uw_restore_core_regs (entry_vrs, &entry_vrs->core);
>
> ? ? case _URC_CONTINUE_UNWIND:
> ? ? ? /* Continue unwinding the next frame. ?*/

Otherwise looks ok to me .

Ramana