This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: MIPS Linux signals
On 05/21/2012 11:48 PM, Maciej W. Rozycki wrote:
> On Mon, 21 May 2012, Michael Eager wrote:
>
>>> BTW, I wouldn't bother with gdbarch_target_signal_to_host. Nothing ever
>>> calls it.
>>
>> I hadn't noticed that. I thought that it was called to translate
>> the signal number when sent to the target. Instead, target_signal_to_host()
>> is called.
>
> Shall we drop the unused gdbarch API so as to avoid further confusion
> then? Shouldn't target_signal_from_host be renamed to something closer to what
> it really does, e.g. signal_from_target?
I agree that the current naming is confusing, but I'll point out why I think
target_signal_from_host is actually correct, and then conclude with renaming
suggestions to avoid the confusion, and fix the naming of the gdbarch hook, which
I think is not correct. We have
enum target_signal
{
...
TARGET_SIGNAL_FOO,
TARGET_SIGNAL_BAR,
...
};
which represents GDB's internal signal numbers. AFAIK, the "target_" prefix
naming in this case is just a natural choice, given that that's how we name
everything behind the target_ops abstraction -- target_read_memory,
target_resume, etc., etc. (target.h) -- the mechanism that is mostly about
abstracting the details of handling the target's debug interface API.
So this explains the "target_signal_from_" part of the function's signature.
The "from_host" part is really correct: this really is a host function -- think in
terms of autoconf's notion of build vs host vs target. It only makes sense to call
it in native code (native == host). And in fact, that's what happens. Grepping
for calls in GDB we see:
=====================
arch-utils.c
arch-utils.h
This is default_target_signal_from_host . Which is the gdbarch fallback,
which only works on native cores.
=====================
corelow.c
/* NOTE: target_signal_from_host() converts a target signal
value into gdb's internal signal value. Unfortunately gdb's
internal value is called ``target_signal'' and this function
got the name ..._from_host(). */
enum target_signal sig = (core_gdbarch != NULL
? gdbarch_target_signal_from_host (core_gdbarch,
siggy)
: target_signal_from_host (siggy));
printf_filtered (_("Program terminated with signal %d, %s.\n"),
siggy, target_signal_to_string (sig));
This is the only place the gdbarch method is called, which is handling
the cross-core scenario. If we don't have a gdbarch hook to do the
translation for us, then we fallback to assuming we're debugging a native
core, one that was generated on the host (or a similar machine).
=====================
darwin-nat.c
Native code.
=====================
gdbarch.c
gdbarch.h
gdbarch.sh
The gdbarch hook.
=====================
gnu-nat.c
inf-ttrace.c
linux-nat.c
linux-thread-db.c
nto-procfs.c
procfs.c
Native code.
=====================
remote-mips.c
Just a comment, but actually interesting in the MIPS context.
/* Return the signal corresponding to SIG, where SIG is the number which
the MIPS protocol uses for the signal. */
static enum target_signal
mips_signal_from_protocol (int sig)
{
/* We allow a few more signals than the IDT board actually returns, on
the theory that there is at least *some* hope that perhaps the numbering
for these signals is widely agreed upon. */
if (sig <= 0
|| sig > 31)
return TARGET_SIGNAL_UNKNOWN;
/* Don't want to use target_signal_from_host because we are converting
from MIPS signal numbers, not host ones. Our internal numbers
match the MIPS numbers for the signals the board can return, which
are: SIGINT, SIGSEGV, SIGBUS, SIGILL, SIGFPE, SIGTRAP. */
return (enum target_signal) sig;
}
=====================
target.c
Actually native code. I think this could move to inf-child.c.
/* Helper function for child_wait and the derivatives of child_wait.
HOSTSTATUS is the waitstatus from wait() or the equivalent; store our
translation of that in OURSTATUS. */
void
store_waitstatus (struct target_waitstatus *ourstatus, int hoststatus)
{
if (WIFEXITED (hoststatus))
{
ourstatus->kind = TARGET_WAITKIND_EXITED;
ourstatus->value.integer = WEXITSTATUS (hoststatus);
}
else if (!WIFSTOPPED (hoststatus))
{
ourstatus->kind = TARGET_WAITKIND_SIGNALLED;
ourstatus->value.sig = target_signal_from_host (WTERMSIG (hoststatus));
}
else
{
ourstatus->kind = TARGET_WAITKIND_STOPPED;
ourstatus->value.sig = target_signal_from_host (WSTOPSIG (hoststatus));
}
}
=====================
windows-nat.c
Native code.
=====================
gdbserver is always native, so always host.
Now, it's the mix between somewhat different etymologies (enum target_signal,
vs build/host/target) that leads to confusion. Renaming enum target_signal
to enum gdb_signal would fix that side of the problem.
= The gdbarch hook =
Another source of confusion is that the gdbarch hook is naturally not a host
function, it concerns with converting a foreign target's signal numbers (for cross
debugging, not host numbers, unless in the special case of host == target) to
gdb's internal numbers. So in that sense,
gdbarch_target_signal_from_host
is the wrong name. It should be called gdbarch_target_signal_from_target (!)
with the default, as today, being target_signal_from_host.
Obviously, gdbarch_target_signal_from_target is even more confusing. :-) The two
"targets" in the name refer to somewhat different ideas.
So I think that to sort all of this out, we should:
enum target_signal => enum gdb_signal
target_signal_from_host => gdb_signal_from_host (or gdb_signal_from_host_signal)
target_signal_to_host => gdb_signal_to_host (or gdb_signal_to_host_signal)
gdbarch_target_signal_from_host => gdbarch_gdb_signal_from_target (or gdbarch_gdb_signal_from_target_signal)
gdbarch_target_signal_to_host => gdbarch_gdb_signal_to_target (or gdbarch_gdb_signal_to_target_signal)
--
Pedro Alves