From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 18263 invoked by alias); 22 May 2012 10:17:43 -0000 Received: (qmail 18249 invoked by uid 22791); 22 May 2012 10:17:41 -0000 X-SWARE-Spam-Status: No, hits=-7.4 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_THREADED,RCVD_IN_DNSWL_HI,RCVD_IN_HOSTKARMA_W,SPF_HELO_PASS,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 22 May 2012 10:17:24 +0000 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q4MAHKfM026423 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 22 May 2012 06:17:20 -0400 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id q4MAHIb3030422; Tue, 22 May 2012 06:17:19 -0400 Message-ID: <4FBB67AE.5090807@redhat.com> Date: Tue, 22 May 2012 10:17:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:12.0) Gecko/20120430 Thunderbird/12.0.1 MIME-Version: 1.0 To: "Maciej W. Rozycki" CC: Michael Eager , "gdb-patches@sourceware.org" Subject: Re: MIPS Linux signals References: <4FB850CA.7090701@eagerm.com> <4FBAB500.7010104@redhat.com> <4FBAB948.7000808@eagerm.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2012-05/txt/msg00812.txt.bz2 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