Message ID | 20170405163332.82109-1-jhb@FreeBSD.org |
---|---|
State | New, archived |
Headers |
Received: (qmail 19196 invoked by alias); 5 Apr 2017 16:34:20 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 19187 invoked by uid 89); 5 Apr 2017 16:34:19 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-23.8 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, SPF_HELO_PASS, SPF_SOFTFAIL autolearn=ham version=3.3.2 spammy=sim, 3866, **** X-HELO: mail.baldwin.cx Received: from bigwig.baldwin.cx (HELO mail.baldwin.cx) (96.47.65.170) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 05 Apr 2017 16:34:12 +0000 Received: from ralph.baldwin.cx.com (c-73-231-226-104.hsd1.ca.comcast.net [73.231.226.104]) by mail.baldwin.cx (Postfix) with ESMTPSA id 2D58E10A888 for <gdb-patches@sourceware.org>; Wed, 5 Apr 2017 12:34:12 -0400 (EDT) From: John Baldwin <jhb@FreeBSD.org> To: gdb-patches@sourceware.org Subject: [PATCH v2] Define an error function in the PPC simulator library. Date: Wed, 5 Apr 2017 09:33:32 -0700 Message-Id: <20170405163332.82109-1-jhb@FreeBSD.org> X-IsSubscribed: yes |
Commit Message
John Baldwin
April 5, 2017, 4:33 p.m. UTC
Previously this used the error function from GDB directly when linked against GDB instead of the error method in the host callbacks structure. This was exposed via a link error when GDB was converted to C++. The error function invokes the error callback similar to sim_io_error. sim/ppc/ChangeLog: PR sim/20863 * sim_calls.c (error): New function. --- sim/ppc/ChangeLog | 5 +++++ sim/ppc/sim_calls.c | 10 ++++++++++ 2 files changed, 15 insertions(+)
Comments
On 04/05/2017 11:33 AM, John Baldwin wrote: > Previously this used the error function from GDB directly when linked > against GDB instead of the error method in the host callbacks > structure. This was exposed via a link error when GDB was converted > to C++. The error function invokes the error callback similar to > sim_io_error. > There is another implementation of error (...) in sim/ppc/main.c and sim/ppc/misc.c. Should those be kept as is or should we only use the new function you're providing? Also, i don't see error being implemented in the other sim backends. I wonder if we should just use whatever is available (sim_io_error?) instead of supplying our own ppc-specific version?
On Thursday, April 13, 2017 08:25:27 AM Luis Machado wrote: > On 04/05/2017 11:33 AM, John Baldwin wrote: > > Previously this used the error function from GDB directly when linked > > against GDB instead of the error method in the host callbacks > > structure. This was exposed via a link error when GDB was converted > > to C++. The error function invokes the error callback similar to > > sim_io_error. > > > > There is another implementation of error (...) in sim/ppc/main.c and > sim/ppc/misc.c. Should those be kept as is or should we only use the new > function you're providing? My understanding is that they should be kept as-is. This file is only used when linking the library against GDB (and actually, rereading the log message, I should reword the opening sentence to make this clearer). sim-calls.o isn't included in the actual library. Each consumer of the library is required to export a couple of symbols that libsim.a uses including "error". The dgen, igen, tmp-filter, tmp-ld-decode, tmp-ld-cache, and tmp-ld-insn programs use the functions from misc.c. psim uses the functions from main.c, and GDB uses the functions from sim-calls.o. glibc includes a global function called 'error' that libsim.a is linking against when linked into GDB on Linux which is why it doesn't fail to link on Linux (but if it ever needs to raise an error it probably blows up as error(3) doesn't have the same calling convention). > Also, i don't see error being implemented in the other sim backends. I > wonder if we should just use whatever is available (sim_io_error?) > instead of supplying our own ppc-specific version? Other sims don't use an "error" function (ppc seems to be special in this case). I think Pedro took a stab at replacing "error" (there's a thread with the subject "gdb-7.12-powerpc-rtems4.12-gdb does not build on FreeBSD" on gdb@) but thought this approach was simpler (and could also be merged to 7.12 though that may be OBE by now).
On Thursday, April 13, 2017 10:18:14 AM John Baldwin wrote: > On Thursday, April 13, 2017 08:25:27 AM Luis Machado wrote: > > On 04/05/2017 11:33 AM, John Baldwin wrote: > > > Previously this used the error function from GDB directly when linked > > > against GDB instead of the error method in the host callbacks > > > structure. This was exposed via a link error when GDB was converted > > > to C++. The error function invokes the error callback similar to > > > sim_io_error. > > > > > > > There is another implementation of error (...) in sim/ppc/main.c and > > sim/ppc/misc.c. Should those be kept as is or should we only use the new > > function you're providing? > > My understanding is that they should be kept as-is. This file is only used > when linking the library against GDB (and actually, rereading the log message, > I should reword the opening sentence to make this clearer). sim-calls.o isn't > included in the actual library. Each consumer of the library is required > to export a couple of symbols that libsim.a uses including "error". The dgen, > igen, tmp-filter, tmp-ld-decode, tmp-ld-cache, and tmp-ld-insn programs use > the functions from misc.c. psim uses the functions from main.c, and > GDB uses the functions from sim-calls.o. glibc includes a global function > called 'error' that libsim.a is linking against when linked into GDB on > Linux which is why it doesn't fail to link on Linux (but if it ever needs > to raise an error it probably blows up as error(3) doesn't have the same > calling convention). > > > Also, i don't see error being implemented in the other sim backends. I > > wonder if we should just use whatever is available (sim_io_error?) > > instead of supplying our own ppc-specific version? > > Other sims don't use an "error" function (ppc seems to be special in this > case). I think Pedro took a stab at replacing "error" (there's a thread > with the subject "gdb-7.12-powerpc-rtems4.12-gdb does not build on FreeBSD" > on gdb@) but thought this approach was simpler (and could also be merged > to 7.12 though that may be OBE by now). Ping?
On Friday, May 05, 2017 12:22:43 PM John Baldwin wrote: > On Thursday, April 13, 2017 10:18:14 AM John Baldwin wrote: > > On Thursday, April 13, 2017 08:25:27 AM Luis Machado wrote: > > > On 04/05/2017 11:33 AM, John Baldwin wrote: > > > > Previously this used the error function from GDB directly when linked > > > > against GDB instead of the error method in the host callbacks > > > > structure. This was exposed via a link error when GDB was converted > > > > to C++. The error function invokes the error callback similar to > > > > sim_io_error. > > > > > > > > > > There is another implementation of error (...) in sim/ppc/main.c and > > > sim/ppc/misc.c. Should those be kept as is or should we only use the new > > > function you're providing? > > > > My understanding is that they should be kept as-is. This file is only used > > when linking the library against GDB (and actually, rereading the log message, > > I should reword the opening sentence to make this clearer). sim-calls.o isn't > > included in the actual library. Each consumer of the library is required > > to export a couple of symbols that libsim.a uses including "error". The dgen, > > igen, tmp-filter, tmp-ld-decode, tmp-ld-cache, and tmp-ld-insn programs use > > the functions from misc.c. psim uses the functions from main.c, and > > GDB uses the functions from sim-calls.o. glibc includes a global function > > called 'error' that libsim.a is linking against when linked into GDB on > > Linux which is why it doesn't fail to link on Linux (but if it ever needs > > to raise an error it probably blows up as error(3) doesn't have the same > > calling convention). > > > > > Also, i don't see error being implemented in the other sim backends. I > > > wonder if we should just use whatever is available (sim_io_error?) > > > instead of supplying our own ppc-specific version? > > > > Other sims don't use an "error" function (ppc seems to be special in this > > case). I think Pedro took a stab at replacing "error" (there's a thread > > with the subject "gdb-7.12-powerpc-rtems4.12-gdb does not build on FreeBSD" > > on gdb@) but thought this approach was simpler (and could also be merged > > to 7.12 though that may be OBE by now). > > Ping? Ping++ This does fix a PR, is a regression in 7.12, and is relatively small, so I think it's a candidate for 8.0 if accepted.
On Friday, May 19, 2017 09:27:01 AM John Baldwin wrote: > On Friday, May 05, 2017 12:22:43 PM John Baldwin wrote: > > On Thursday, April 13, 2017 10:18:14 AM John Baldwin wrote: > > > On Thursday, April 13, 2017 08:25:27 AM Luis Machado wrote: > > > > On 04/05/2017 11:33 AM, John Baldwin wrote: > > > > > Previously this used the error function from GDB directly when linked > > > > > against GDB instead of the error method in the host callbacks > > > > > structure. This was exposed via a link error when GDB was converted > > > > > to C++. The error function invokes the error callback similar to > > > > > sim_io_error. > > > > > > > > > > > > > There is another implementation of error (...) in sim/ppc/main.c and > > > > sim/ppc/misc.c. Should those be kept as is or should we only use the new > > > > function you're providing? > > > > > > My understanding is that they should be kept as-is. This file is only used > > > when linking the library against GDB (and actually, rereading the log message, > > > I should reword the opening sentence to make this clearer). sim-calls.o isn't > > > included in the actual library. Each consumer of the library is required > > > to export a couple of symbols that libsim.a uses including "error". The dgen, > > > igen, tmp-filter, tmp-ld-decode, tmp-ld-cache, and tmp-ld-insn programs use > > > the functions from misc.c. psim uses the functions from main.c, and > > > GDB uses the functions from sim-calls.o. glibc includes a global function > > > called 'error' that libsim.a is linking against when linked into GDB on > > > Linux which is why it doesn't fail to link on Linux (but if it ever needs > > > to raise an error it probably blows up as error(3) doesn't have the same > > > calling convention). > > > > > > > Also, i don't see error being implemented in the other sim backends. I > > > > wonder if we should just use whatever is available (sim_io_error?) > > > > instead of supplying our own ppc-specific version? > > > > > > Other sims don't use an "error" function (ppc seems to be special in this > > > case). I think Pedro took a stab at replacing "error" (there's a thread > > > with the subject "gdb-7.12-powerpc-rtems4.12-gdb does not build on FreeBSD" > > > on gdb@) but thought this approach was simpler (and could also be merged > > > to 7.12 though that may be OBE by now). > > > > Ping? > > Ping++ > > This does fix a PR, is a regression in 7.12, and is relatively small, so I > think it's a candidate for 8.0 if accepted. Ping?
On 06/06/17 19:48, John Baldwin wrote: > On Friday, May 19, 2017 09:27:01 AM John Baldwin wrote: >> On Friday, May 05, 2017 12:22:43 PM John Baldwin wrote: >>> On Thursday, April 13, 2017 10:18:14 AM John Baldwin wrote: >>>> On Thursday, April 13, 2017 08:25:27 AM Luis Machado wrote: >>>>> On 04/05/2017 11:33 AM, John Baldwin wrote: >>>>>> Previously this used the error function from GDB directly when linked >>>>>> against GDB instead of the error method in the host callbacks >>>>>> structure. This was exposed via a link error when GDB was converted >>>>>> to C++. The error function invokes the error callback similar to >>>>>> sim_io_error. >>>>>> >>>>> There is another implementation of error (...) in sim/ppc/main.c and >>>>> sim/ppc/misc.c. Should those be kept as is or should we only use the new >>>>> function you're providing? >>>> My understanding is that they should be kept as-is. This file is only used >>>> when linking the library against GDB (and actually, rereading the log message, >>>> I should reword the opening sentence to make this clearer). sim-calls.o isn't >>>> included in the actual library. Each consumer of the library is required >>>> to export a couple of symbols that libsim.a uses including "error". The dgen, >>>> igen, tmp-filter, tmp-ld-decode, tmp-ld-cache, and tmp-ld-insn programs use >>>> the functions from misc.c. psim uses the functions from main.c, and >>>> GDB uses the functions from sim-calls.o. glibc includes a global function >>>> called 'error' that libsim.a is linking against when linked into GDB on >>>> Linux which is why it doesn't fail to link on Linux (but if it ever needs >>>> to raise an error it probably blows up as error(3) doesn't have the same >>>> calling convention). >>>> >>>>> Also, i don't see error being implemented in the other sim backends. I >>>>> wonder if we should just use whatever is available (sim_io_error?) >>>>> instead of supplying our own ppc-specific version? >>>> Other sims don't use an "error" function (ppc seems to be special in this >>>> case). I think Pedro took a stab at replacing "error" (there's a thread >>>> with the subject "gdb-7.12-powerpc-rtems4.12-gdb does not build on FreeBSD" >>>> on gdb@) but thought this approach was simpler (and could also be merged >>>> to 7.12 though that may be OBE by now). >>> Ping? >> Ping++ >> >> This does fix a PR, is a regression in 7.12, and is relatively small, so I >> think it's a candidate for 8.0 if accepted. > Ping? > This issue breaks also at least the PowerPC GDB 7.12 and 8.0 support on MacOS X. Its not only related to FreeBSD.
Hi John, (Since it seems that Mike doesn't have much time for the sim currently, I'm trying to help move things along by reviewing this patch.) On 04/13/2017 06:18 PM, John Baldwin wrote: > On Thursday, April 13, 2017 08:25:27 AM Luis Machado wrote: >> On 04/05/2017 11:33 AM, John Baldwin wrote: >>> Previously this used the error function from GDB directly when linked >>> against GDB instead of the error method in the host callbacks >>> structure. This was exposed via a link error when GDB was converted >>> to C++. The error function invokes the error callback similar to >>> sim_io_error. >>> >> >> There is another implementation of error (...) in sim/ppc/main.c and >> sim/ppc/misc.c. Should those be kept as is or should we only use the new >> function you're providing? > > My understanding is that they should be kept as-is. This file is only used > when linking the library against GDB (and actually, rereading the log message, > I should reword the opening sentence to make this clearer). sim-calls.o isn't > included in the actual library. Each consumer of the library is required > to export a couple of symbols that libsim.a uses including "error". The dgen, > igen, tmp-filter, tmp-ld-decode, tmp-ld-cache, and tmp-ld-insn programs use > the functions from misc.c. psim uses the functions from main.c, and > GDB uses the functions from sim-calls.o. It'd be nice to include this info somewhere, likely in the commit log. > glibc includes a global function > called 'error' that libsim.a is linking against when linked into GDB on > Linux which is why it doesn't fail to link on Linux (but if it ever needs > to raise an error it probably blows up as error(3) doesn't have the same > calling convention). > >> Also, i don't see error being implemented in the other sim backends. I >> wonder if we should just use whatever is available (sim_io_error?) >> instead of supplying our own ppc-specific version? > > Other sims don't use an "error" function (ppc seems to be special in this > case). I think Pedro took a stab at replacing "error" (there's a thread > with the subject "gdb-7.12-powerpc-rtems4.12-gdb does not build on FreeBSD" > on gdb@) but thought this approach was simpler (and could also be merged > to 7.12 though that may be OBE by now). > Yeah. Your patch looks good to me as you have it. Even though renaming the sim's "error" function to something else would be nice, I don't think that should be a requirement. We've been calling it "error" since forever. Thanks, Pedro Alves
On 9/4/17 7:19 AM, Pedro Alves wrote: > On 04/13/2017 06:18 PM, John Baldwin wrote: >> On Thursday, April 13, 2017 08:25:27 AM Luis Machado wrote: >>> On 04/05/2017 11:33 AM, John Baldwin wrote: >>>> Previously this used the error function from GDB directly when linked >>>> against GDB instead of the error method in the host callbacks >>>> structure. This was exposed via a link error when GDB was converted >>>> to C++. The error function invokes the error callback similar to >>>> sim_io_error. >>>> >>> >>> There is another implementation of error (...) in sim/ppc/main.c and >>> sim/ppc/misc.c. Should those be kept as is or should we only use the new >>> function you're providing? >> >> My understanding is that they should be kept as-is. This file is only used >> when linking the library against GDB (and actually, rereading the log message, >> I should reword the opening sentence to make this clearer). sim-calls.o isn't >> included in the actual library. Each consumer of the library is required >> to export a couple of symbols that libsim.a uses including "error". The dgen, >> igen, tmp-filter, tmp-ld-decode, tmp-ld-cache, and tmp-ld-insn programs use >> the functions from misc.c. psim uses the functions from main.c, and >> GDB uses the functions from sim-calls.o. > > It'd be nice to include this info somewhere, likely in the commit log. Will do. Is there still time to merge this into 8.0.1 (and if so, can you approve it)? Thanks!
On 09/05/2017 04:00 AM, John Baldwin wrote: > On 9/4/17 7:19 AM, Pedro Alves wrote: >> On 04/13/2017 06:18 PM, John Baldwin wrote: >>> On Thursday, April 13, 2017 08:25:27 AM Luis Machado wrote: >>>> On 04/05/2017 11:33 AM, John Baldwin wrote: >>>>> Previously this used the error function from GDB directly when linked >>>>> against GDB instead of the error method in the host callbacks >>>>> structure. This was exposed via a link error when GDB was converted >>>>> to C++. The error function invokes the error callback similar to >>>>> sim_io_error. >>>>> >>>> >>>> There is another implementation of error (...) in sim/ppc/main.c and >>>> sim/ppc/misc.c. Should those be kept as is or should we only use the new >>>> function you're providing? >>> >>> My understanding is that they should be kept as-is. This file is only used >>> when linking the library against GDB (and actually, rereading the log message, >>> I should reword the opening sentence to make this clearer). sim-calls.o isn't >>> included in the actual library. Each consumer of the library is required >>> to export a couple of symbols that libsim.a uses including "error". The dgen, >>> igen, tmp-filter, tmp-ld-decode, tmp-ld-cache, and tmp-ld-insn programs use >>> the functions from misc.c. psim uses the functions from main.c, and >>> GDB uses the functions from sim-calls.o. >> >> It'd be nice to include this info somewhere, likely in the commit log. > > Will do. Is there still time to merge this into 8.0.1 I think the plan was yesterday, but it didn't happen yet, so I think there's still time, though likely not much. (and if so, can you approve it)? Yes, I think it's safe. You'll need to be sure that there's an entry for the issue listed at: https://sourceware.org/gdb/wiki/GDB_8.0_Release Either via the bugzilla query or explicitly listed. Thanks, Pedro Alves
On 9/5/17 5:14 AM, Pedro Alves wrote: >> Will do. Is there still time to merge this into 8.0.1 > > I think the plan was yesterday, but it didn't happen yet, so I think > there's still time, though likely not much. > > (and if so, can you approve it)? > > Yes, I think it's safe. Ok, done. > You'll need to be sure that there's an entry for the issue > listed at: > > https://sourceware.org/gdb/wiki/GDB_8.0_Release > > Either via the bugzilla query or explicitly listed. The bug is closed and has the 8.0.1 milestone. Thanks!
> >> Will do. Is there still time to merge this into 8.0.1 > > > > I think the plan was yesterday, but it didn't happen yet, so I think > > there's still time, though likely not much. That's correct, because there were some tickets pending, so I said I'd give extra time. I'm reviewing the situation every couple of days, with the next update being tomorrow. > The bug is closed and has the 8.0.1 milestone. Thanks! Perfect. I was a bit worried when I saw the commits get in without the PR number in the ChangeLog. If the PR has the 8.0.1 milestone, then no need to worry about listing them explicitly in the wiki. That's what the list to bugzilla is there for.
diff --git a/sim/ppc/ChangeLog b/sim/ppc/ChangeLog index c0bb1f5b8b..8ad90077c0 100644 --- a/sim/ppc/ChangeLog +++ b/sim/ppc/ChangeLog @@ -1,3 +1,8 @@ +2017-04-04 John Baldwin <jhb@FreeBSD.org> + + PR sim/20863 + * sim_calls.c (error): New function. + 2017-02-13 Mike Frysinger <vapier@gentoo.org> * cpu.h: Include libiberty.h. diff --git a/sim/ppc/sim_calls.c b/sim/ppc/sim_calls.c index 470c95862a..eb5d1a792b 100644 --- a/sim/ppc/sim_calls.c +++ b/sim/ppc/sim_calls.c @@ -386,6 +386,16 @@ sim_io_error (SIM_DESC sd, const char *fmt, ...) /****/ +void NORETURN +error (const char *msg, ...) +{ + va_list ap; + va_start(ap, msg); + callbacks->evprintf_filtered (callbacks, msg, ap); + va_end(ap); + callbacks->error (callbacks, ""); +} + void * zalloc(long size) {