This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 3/3] Use signal information to determine SIGTRAP type for FreeBSD.
- From: Pedro Alves <palves at redhat dot com>
- To: John Baldwin <jhb at FreeBSD dot org>, gdb-patches at sourceware dot org
- Date: Fri, 2 Mar 2018 20:19:48 +0000
- Subject: Re: [PATCH 3/3] Use signal information to determine SIGTRAP type for FreeBSD.
- Authentication-results: sourceware.org; auth=none
- References: <20180228014656.32372-1-jhb@FreeBSD.org> <20180228014656.32372-4-jhb@FreeBSD.org>
Hi John,
LGTM, with nits below. Please go ahead and push with
these fixed.
On 02/28/2018 01:46 AM, John Baldwin wrote:
> +#ifdef TRAP_BRKPT
> +/*
> + * MIPS does not set si_code for SIGTRAP. sparc64 reports
> + * non-standard values in si_code for SIGTRAP.
> + */
GNU formatting, please.
> +#if !defined(__mips__) && !defined(__sparc64__)
> +#define USE_SIGTRAP_SIGINFO
^^^
tab vs space?
> +#endif
> +#endif
> +
I'd suggest indenting the directives, like:
#ifdef TRAP_BRKPT
...
# if !defined(__mips__) && !defined(__sparc64__)
# define USE_SIGTRAP_SIGINFO
# endif
#endif
> /* Wait for the child specified by PTID to do something. Return the
> process ID of the child, or MINUS_ONE_PTID in case of error; store
> the status in *OURSTATUS. */
> @@ -1372,6 +1433,13 @@ fbsd_wait (struct target_ops *ops,
> }
> #endif
>
> +#ifdef USE_SIGTRAP_SIGINFO
> + if (fbsd_handle_debug_trap (ops, wptid, pl))
> + {
> + return wptid;
> + }
Remove redundant braces.
> +#endif
> +
> /* Note that PL_FLAG_SCE is set for any event reported while
> a thread is executing a system call in the kernel. In
> particular, signals that interrupt a sleep in a system
> @@ -1410,6 +1478,41 @@ fbsd_wait (struct target_ops *ops,
> }
> }
>
> +#ifdef USE_SIGTRAP_SIGINFO
> +/* Implement the "to_stopped_by_sw_breakpoint" target_ops method. */
> +
> +static int
> +fbsd_stopped_by_sw_breakpoint (struct target_ops *ops)
> +{
> + struct ptrace_lwpinfo pl;
> +
> + if (ptrace (PT_LWPINFO, get_ptrace_pid (inferior_ptid), (caddr_t) &pl,
> + sizeof pl) == -1)
> + return 0;
> +
> + return (pl.pl_flags & PL_FLAG_SI) && pl.pl_siginfo.si_signo == SIGTRAP
> + && pl.pl_siginfo.si_code == TRAP_BRKPT;
For multi-line expressions, the convention is to wrap in ()s so
that the && aligns under the (. Like:
return ((pl.pl_flags & PL_FLAG_SI)
&& pl.pl_siginfo.si_signo == SIGTRAP
&& pl.pl_siginfo.si_code == TRAP_BRKPT);
> +#ifdef USE_SIGTRAP_SIGINFO
> + t->to_stopped_by_sw_breakpoint = fbsd_stopped_by_sw_breakpoint;
> + t->to_supports_stopped_by_sw_breakpoint =
> + fbsd_supports_stopped_by_sw_breakpoint;
> + t->to_supports_stopped_by_hw_breakpoint =
> + fbsd_supports_stopped_by_hw_breakpoint;
'=' goes on the next line, like:
t->to_supports_stopped_by_sw_breakpoint
= fbsd_supports_stopped_by_sw_breakpoint;
Thanks,
Pedro Alves