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

[Pedro Alves <palves@redhat.com>]

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