Re: [patch] Reading coff-pe-read files

Joel Brobecker
2009-01-08T09:53:55Z

Hello Kai,

> I modified my patch, so that it doesn't need a #if clause anymore.

Thanks. This was an excellent suggestion from Pierre and I'm much
happier with the new patch.

A few comments:

First, do you think you could use a different mime type when attaching
a patch? I shows up as application/octet-stream which means that my
mailer doesn't treat it as text - This means that when I reply, your
patch isn't quoted automatically as part of the reply. text/plain would
be great. Some people also use some mailers that do not handle base64
encodings. One of them is Mark Kettenis who's still one of our active
maintainers, particularly on x86/x86_64. You'll have more chances to get
his comments if you use a 7bit ASCII file...

Can you also provide a ChangeLog entry when submitting patches?

> Index: src/gdb/coff-pe-read.c
> ===================================================================
> --- src.orig/gdb/coff-pe-read.c
> +++ src/gdb/coff-pe-read.c
> @@ -191,6 +191,8 @@ read_pe_exported_syms (struct objfile *o
> unsigned char *expdata, *erva;
> unsigned long name_rvas, ordinals, nexp, ordbase;
> char *dll_name;
> + int be64 = 0;
> + int be32 = 0;

Would you mind explaining what "be" stands for?

> + if (be64)
> + num_entries = pe_get32 (dll, opthdr_ofs + 92 + 16);
> + else
> + num_entries = pe_get32 (dll, opthdr_ofs + 92);

Not knowing the PE format all that well, could you explain
these numbers a little? 92 + 16 seems to suggest that the number
of entries is no longer at the beginning of some kind of "section"
but 16 bytes later.

(this is for my education - I tried to find some documentation
about the 64bit PE/COFF format, but the only one I found was from
MS and it's in a docx format that I can't seem to be able to open
with openoffice - I would be very grateful for a PDF)

> - export_rva = pe_get32 (dll, opthdr_ofs + 96);
> - export_size = pe_get32 (dll, opthdr_ofs + 100);
> + if (be64)
> + {
> + export_rva = pe_get32 (dll, opthdr_ofs + 96 + 16);
> + export_size = pe_get32 (dll, opthdr_ofs + 100 + 16);
> + }
> + else
> + {
> + export_rva = pe_get32 (dll, opthdr_ofs + 96);
> + export_size = pe_get32 (dll, opthdr_ofs + 100);
> + }

Same here.

I trust you that the changes are correct (and in any case only
affect x86_64-windows), but I want to understand them a little bit.

In the meantime, do you have write access to the GDB project?
If not, let's also work on that. I see that you have a copyright
assignment already in place, and an account on sourceware.org
as well.

--
Joel