This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFC] Fix .text section offset for windows DLL (was Calling __stdcall functions in the inferior)
- From: Joel Brobecker <brobecker at adacore dot com>
- To: Pierre Muller <pierre dot muller at ics-cnrs dot unistra dot fr>
- Cc: gdb-patches at sourceware dot org
- Date: Mon, 5 Nov 2012 09:11:21 -0800
- Subject: Re: [RFC] Fix .text section offset for windows DLL (was Calling __stdcall functions in the inferior)
- References: <83a9vs89r9.fsf@gnu.org> <201210120953.q9C9rqfu020865@glazunov.sibelius.xs4all.nl> <834nm07z0s.fsf@gnu.org> <5077FEB9.4030304@redhat.com> <83y5jb7rfe.fsf@gnu.org> <006001cdaada$00c81f00$02585d00$@muller@ics-cnrs.unistra.fr> <20121024194517.GK3555@adacore.com> <011901cdb2ab$48076b90$d81642b0$@muller@ics-cnrs.unistra.fr>
> Oh boy, you scared me there... But definitively, no, I didn't
> commit this change yet!
Sorry about the scare. I was looking at the code, and saw identical
code, so I figured it got checked in. That's perhaps a clue for us
to do more code factoring...
> I still didn't get a correct way of checking my formatting...
No problemo, I don't think it's a major issue.
> > > + printf ("%s is a forward to %s\n", funcname, forward_name);
> >
> > I don't think a printf is appropriate, here. Is that meant to be
> > a warning?
>
> I would have like any suggestion about an idea about how to
> implement such a forward... Something like a function called
> add_pe_forward_symbol Would it require the definition of a new
> minimal symbol type? If yes, I wouldn't even know where I should
> add this new type...
Are you saying, remove the need for the printf/warning by implementing
the missing feature? If complicated, I'd start with the
warning/complaint first, get that checked in, and then think about
how to lift the limitation.
> I basically reused the existing code in the function called
> read_pe_exported_syms, and, specifically, I kept most of the local
> variables and used the same names...
Let's keep it consistent for now, then...
> > > + unsigned char *expdata, *erva;
> >
> > Should we be using gdb_byte *, in this case? I'm wondering if we
> > should be adjusting the pe_get* & pe_as* routines as well...
>
> The problem is that it is used both for 'char *'
> for all the names and for RVA (relative virtual addresses)
> which are more something like a 32-bit unsigned offset.
I'm not opposed to keeping the consistency, but I think we're going
to have to re-evaluate the use of "char *" vs "gdb_byte *" at some
point. I think that if you have a buffer of bytes, it's better to
use gdb_bytes, as the type was created to deal with this sort of
data. Usually, you can't rely on what the buffer contains, because
host and target can be different.
> > > + if (!is_pe32 && !is_pe64)
> > > + {
> > > + /* This is not a recognized PE format file. Abort now, because
> > > + the code is untested on anything else. *FIXME* test on
> > > + further architectures and loosen or remove this test. */
> > > + return 0;
> > > + }
> >
> > I think a complaint would be appropriate, here. And I'm wondering
> > if there might be a better way to check which PE format it is other
> > than looking at a string...
>
> Again, this is a plain copy of the code in read_pe_exported_syms.
OK, you can keep it that way, at least for now. "plain copy" -> factorize?
--
Joel