Bug 446251 - TARGET_SIGNAL_THR added to enum target_signal
Summary: TARGET_SIGNAL_THR added to enum target_signal
Status: RESOLVED FIXED
Alias: None
Product: valgrind
Classification: Developer tools
Component: general (other bugs)
Version First Reported In: unspecified
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Paul Floyd
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-11-29 18:51 UTC by Mark Wielaard
Modified: 2021-11-30 11:48 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed/Implemented In:
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Wielaard 2021-11-29 18:51:44 UTC
commit 68bb7c063f71631a4f207adca2235eb0f8d00d33
Author: Paul Floyd <pjfloyd@wanadoo.fr>
Date:   Sat Oct 9 15:01:08 2021 +0200

    FreeBSD support, patch 12
    
    coregrind modified files

Added TARGET_SIGNAL_THR = 77 to enum target_signal in coregrind/m_gdbserver/gdb/signals.h

But didn't update the static struct signals in coregrind/m_gdbserver/signals.c

/* This table must match in order and size the signals in enum target_signal
   in gdb/signals.h. */

Now the names after TARGET_SIGNAL_CANCEL are off by one.
(which could in theory make target_signal_to_name access one after the signal array)

Also coregrind/m_gdbserver/gdb/signals.h says:

   The numbering of these signals is chosen to match traditional unix
   signals (insofar as various unices use the same numbers, anyway).
   It is also the numbering of the GDB remote protocol.  Other remote
   protocols, if they use a different numbering, should make sure to
   translate appropriately.

   Since these numbers have actually made it out into other software
   (stubs, etc.), you mustn't disturb the assigned numbering. [...]

TARGET_SIGNAL_THR also isn't in gdb.

I believe we should simply remove TARGET_SIGNAL_THR from enum target_signal. Or if it really is necessary coordinate with gdb to assign it a new number.
Comment 1 Paul Floyd 2021-11-29 21:20:48 UTC
I don't remember what prompted to me to add that. My git commit message was just "Add SIGTHR".

Looking at the gdb code, fbsd-tdep.c there is

    /* SIGTHR is the same as SIGLWP on FreeBSD. */
    case FREEBSD_SIGTHR:
      return GDB_SIGNAL_LWP;

I'll change it to use that.
Comment 2 Paul Floyd 2021-11-30 11:48:41 UTC
Fixed with
commit 521e4690091e36ac8e1fbf388f8e30615a50f0d0