Re: [patch 1/9]#2 Rename `enum target_signal' to target_signal_t

[Pedro Alves <alves dot ped at gmail dot com>]

On Thursday 02 September 2010 11:31:22, Jan Kratochvil wrote:
> On Thu, 02 Sep 2010 03:54:47 +0200, Pedro Alves wrote:
> > A target_signal_o array to hold
> > all the possible gdb signals, and a target_signal becomes:
> > 
> > typedef const struct target_signal_o * target_signal;
> 
> This means there is abandoned the possibility to associate dynamic information
> with a signal (such as to associate siginfo_t with it).  Target signal (SIG*)
> without its siginfo_t is an incomplete information.  Therefore I was
> automatically trying to fix this problem before.

I don't think it is a good idea to associate the siginfo with a signal
and think of it as a single entity.  For many cases, you don't need the
siginfo.  Carrying it around would be extra burden, especially for the remote
target.  For example, consider a signal set to "pass nostop".  GDB does not
need to read the whole siginfo to decide whether the signal should be passed
down straight to the inferior.  IMO, it is better to leave
a "target_signal" being a tag into which signal triggered, and keep the
siginfo a separate object, which may or not be available, even.

> and TARGET_SIGNAL_SIGTRAP should be called
> TARGET_SIGNAL_BREAKPOINT_HIT etc.  

I'm not sure that would be a good idea either.  That's not a
signal in the "unix sense".  A breakpoint hit is a higher
level event than a SIGTRAP.
If the target can determine itself that a breakpoint triggered, and
we want to carry that info to infrun, we can either add a new
waitkind for it (and associate any necessary info along) if it doesn't
make sense to associate the event with a signal, or have the core
ask the target if it can explain the TARGET_SIGNAL_TRAP (much like
watchpoints are handled).  In any case, for a new breakpoint
hit waitkind (TARGET_WAITKIND_BREAKPOINT_HIT or something.
Think of TARGET_WAITKIND_FORK|VFORK|EXEC), you
wouldn't need to give infrun the whole siginfo either.  At the
remote protocol level, you'd probably have the remote stub transfer
something like "T05 bkpt", rather than a stop reply with the
whole siginfo.  Consider the remote protocol and software
single-step targets.  You don't want to have to transfer
the whole siginfo blob back and forth for each single-step.
I don't think I mentioned it yet, but I'm presently working on a
stub for an OS where "signal info" size is _not_ a hard constant,
and can be 2048 bytes, if not more.

> But a full signals abstraction is a too
> expensive/naive/unreal clean-up project for now.)

We can consider TARGET_WAITKIND_STOPPED to be an event
associated with a boring old unix-like signal, and add
other high level waitkinds for other things.  Or reuse
the waitking and add more flags to it.  In any case,
even if we got rid of the gdb-signal <-> host-signals
mapping completely (which would be underirable as it would mean
we'd need to teach gdb about any random OS we want to use
remote debugging with), embedding more info _within_ a simple
signal id appears wrong to me.  And that's what the whole
series was about, and I think we're in agreement on that now.

> typedef struct
>   {
>     enum target_signal_number sig;  /* TARGET_SIGNAL_* 30 possibilities */
>     int host_signal;  /* untranslated SIG* number for TARGET_SIGNAL_UNKNOWN.  */
>   } target_signal;
>  - Naming is kept compatible with current GDB naming.  The reality is:
>    s/target_signal/gdb_signal/
>    s/host_signal/target_signal/
> 
> Target could provide a number-name table for the `handle' command covering
> just the specific 64 signals available at the Linux target (instead of the 150
> ones from the TARGET_SIGNAL_* list).

It could.  For remote targets, there would need to be a fallback for
a table of the current whole gdb signal numbers, and a way to select
the correct table for the remote os/target --- based on osabi, or fully
described in the target description.  If you get rid of the gdb generic
signals from infrun.c completely, some things might become
weird if you don't have such a mechanism, like "signal FOO" sending
one signal when native debugging, and another when remote debugging.

-- 
Pedro Alves