[PATCH v2] Define an error function in the PPC simulator library.

[PATCH v2] Define an error function in the PPC simulator library.

[John Baldwin]

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(+)

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)
 {
-- 
2.11.0

Re: [PATCH v2] Define an error function in the PPC simulator library.

[Luis Machado]

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?

Re: [PATCH v2] Define an error function in the PPC simulator library.

[John Baldwin]

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).

-- 
John Baldwin

Re: [PATCH v2] Define an error function in the PPC simulator library.

[John Baldwin]

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?

-- 
John Baldwin

[PING] Re: [PATCH v2] Define an error function in the PPC simulator library.

[John Baldwin]

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.

-- 
John Baldwin

Re: [PING] Re: [PATCH v2] Define an error function in the PPC simulator library.

[John Baldwin]

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?

-- 
John Baldwin

Re: Re: [PING] Re: [PATCH v2] Define an error function in the PPC simulator library.

[Sebastian Huber]

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.

-- 
Sebastian Huber, embedded brains GmbH

Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone   : +49 89 189 47 41-16
Fax     : +49 89 189 47 41-09
E-Mail  : sebastian.huber@embedded-brains.de
PGP     : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.

Re: [PATCH v2] Define an error function in the PPC simulator library.

[Pedro Alves]

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

Re: [PATCH v2] Define an error function in the PPC simulator library.

[John Baldwin]

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!

-- 
John Baldwin

Re: [PATCH v2] Define an error function in the PPC simulator library.

[Pedro Alves]

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

Re: [PATCH v2] Define an error function in the PPC simulator library.

[John Baldwin]

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!

-- 
John Baldwin

Re: [PATCH v2] Define an error function in the PPC simulator library.

[Joel Brobecker]

> >> 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.

-- 
Joel