Re: [PATCH] Handle MIPS Linux SIGTRAP siginfo.si_code values

[Pedro Alves <palves@redhat.com>]

On 02/24/2016 08:39 PM, Maciej W. Rozycki wrote:

>  Whereas for real breakpoint hits if we report TRAP_BRKPT for software 
> breakpoints and TRAP_HWBKPT for hardware data or instruction breakpoints 
> (the latters once implemented), then the:
> 
> 	  if (GDB_ARCH_IS_TRAP_BRKPT (siginfo.si_code)
> 	      && GDB_ARCH_IS_TRAP_HWBKPT (siginfo.si_code))
> 
> condition will never be true for legitimate SIGTRAP events and the slow 
> path won't trigger.  But then the MIPS variant will need:
> 
> # define GDB_ARCH_IS_TRAP_BRKPT(X) ((X) == SI_KERNEL || (X) == TRAP_BRKPT)
> # define GDB_ARCH_IS_TRAP_HWBKPT(X) ((X) == SI_KERNEL || (X) == TRAP_HWBKPT)

Exactly.

> 
> to actually recognise these events at all in the first place.  So we 
> better have it right away or updated kernels will break GDB for a change.  

OK, if you're still willing to change the kernel, let's do it.

I had somehow imagined (and reading back, I have no idea why) that you
_didn't_ want to change the si_code, if possible, and was going by that.

(There's always risk associated with such a change, as it's effectively
an ABI break and some tool out there may be relying on SI_KERNEL and may
thus stop working correctly.  Usually ABI stability trumps "cleanliness",
in kernel circles.)


> But you haven't put it in your proposal despite my earlier suggestion, 
> hence my question whether you want this kernel API correction and the 
> resulting optimisation or not (I do).

I was just keeping matters separate, by keeping the patch strictly
about fixing the regression with existing kernels.

>  Given my observations above it looks to me that we only really need the 
> two macros updated with my proposal on the GDB side and a corresponding 
> rather trivial kernel update to have the codes set in the first place, and 
> we can get away without complicating code with an extra run-time probe.
> 
>  Have I missed anything?

I think not.

I've just finished testing it on s390 -- no regressions.  I've
pushed it in now, as is.  I'll follow up with a new patch that makes
gdb accept the anticipated new si_codes too.

Thanks,
Pedro Alves