[PATCH 3/3] Use signal information to determine SIGTRAP type for FreeBSD.

[Pedro Alves]

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