[Bug win32/12716] New: addresses of exported symbols of a DLL should be looked up by ordinal

[Bug win32/12716] New: addresses of exported symbols of a DLL should be looked up by ordinal

[pebolle at tiscali dot nl]

http://sourceware.org/bugzilla/show_bug.cgi?id=12716

           Summary: addresses of exported symbols of a DLL should be
                    looked up by ordinal
           Product: gdb
           Version: 7.2
            Status: NEW
          Severity: normal
          Priority: P2
         Component: win32
        AssignedTo: unassigned@sourceware.org
        ReportedBy: pebolle@tiscali.nl


0) I ran into a PE32 DLL that exports a number of symbols. objdump shows it
uses an "Export Address Table", a "Name Pointer Table" and an "Ordinal Table".

1) If you look at objdump's dump of the EA table and the "[Ordinal/Name
Pointer] Table" one sees that the O/NP table is (basically) unsorted: it's
neither sorted on ordinal nor (alphabetically) on the symbols. BUT (the version
of) gdb (that I use) looks up the address of a symbol in the O/NP table as if
that tables has the same order as the EA table. But what gdb actually should do
is: look up a symbol in the O/NP table, get its ordinal and look up an address
in the EA table using that ordinal.

2) I just happened to use gdb with a couple of symbols (which, as I knew from
wine's debugging output, were really used) that gdb mapped to addresses that
simply were not used at all in the code paths I apparently ran. This confusing
behaviour quickly made me think that gdb never set pending breakpoints in that
DLL (which I knew as loaded at runtime through LoadLibraryA). Hence the feeling
I ran into the issue reported in bug #9474.

3) A (still untested) patch for this issue:

diff --git a/gdb/coff-pe-read.c b/gdb/coff-pe-read.c
index ca87b72..0483158 100644
--- a/gdb/coff-pe-read.c
+++ b/gdb/coff-pe-read.c
@@ -150,6 +150,14 @@ read_pe_truncate_name (char *dll_name)
     }
 }


+static unsigned int
+pe_as16 (void *ptr)
+{
+  unsigned char *b = ptr;
+
+  return b[0] + (b[1] << 8);
+}
+
 /* Low-level support functions, direct from the ld module pe-dll.c.  */
 static unsigned int
 pe_get16 (bfd *abfd, int where)
@@ -309,11 +317,11 @@ read_pe_exported_syms (struct objfile *objfile)
   bfd_bread (expdata, (bfd_size_type) export_size, dll);
   erva = expdata - export_rva;

+  ordbase = pe_as32 (expdata + 16); /* unused */
   nexp = pe_as32 (expdata + 24);
+  exp_funcbase = pe_as32 (expdata + 28);
   name_rvas = pe_as32 (expdata + 32);
   ordinals = pe_as32 (expdata + 36);
-  ordbase = pe_as32 (expdata + 16);
-  exp_funcbase = pe_as32 (expdata + 28);

   /* Use internal dll name instead of full pathname.  */
   dll_name = pe_as32 (expdata + 12) + erva;
@@ -339,8 +347,10 @@ read_pe_exported_syms (struct objfile *objfile)
       /* Pointer to the names vector.  */
       unsigned long name_rva = pe_as32 (erva + name_rvas + i * 4);

+      unsigned long ordinal = pe_as16 (erva + ordinals + i * 2);
+
       /* Pointer to the function address vector.  */
-      unsigned long func_rva = pe_as32 (erva + exp_funcbase + i * 4);
+      unsigned long func_rva = pe_as32 (erva + exp_funcbase + ordinal * 4);

       /* Find this symbol's section in our own array.  */
       int sectix = 0;

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug win32/12716] addresses of exported symbols of a DLL should be looked up by ordinal

[pebolle at tiscali dot nl]

http://sourceware.org/bugzilla/show_bug.cgi?id=12716

--- Comment #1 from Paul Bolle <pebolle at tiscali dot nl> 2011-04-30 20:38:37 UTC ---
(In reply to comment #0)
> 3) A (still untested) patch for this issue:

0) This is not true any more. I (finally) managed to recompile gdb for mingw
and test this patch. It actually compiled cleanly and worked as I hoped.

1) Not sure how to test this more thoroughly. My feeling is that this patch
should only cause regressions for DLLs in which the "Name Pointer Table" and
the "Ordinal Table" are out of sync. Ie, for DDLs that are broken beyond
repair.

2) Review by someone familiar with PE32 DLLs would be appreciated.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug win32/12716] addresses of exported symbols of a DLL should be looked up by ordinal

[pebolle at tiscali dot nl]

http://sourceware.org/bugzilla/show_bug.cgi?id=12716

--- Comment #2 from Paul Bolle <pebolle at tiscali dot nl> 2011-06-11 13:31:34 UTC ---
Please note that my (rather broad) copyright papers (discussed in bug #9474)
are in place since a week or two. So I assume there are now no more legal
issues preventing review and/or usage of the patch in comment #0.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug win32/12716] addresses of exported symbols of a DLL should be looked up by ordinal

[qiyao at gcc dot gnu.org]

http://sourceware.org/bugzilla/show_bug.cgi?id=12716

Yao Qi <qiyao at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |qiyao at gcc dot gnu.org

--- Comment #3 from Yao Qi <qiyao at gcc dot gnu.org> 2011-06-12 03:50:09 UTC ---
(In reply to comment #2)
> Please note that my (rather broad) copyright papers (discussed in bug #9474)
> are in place since a week or two. So I assume there are now no more legal
> issues preventing review and/or usage of the patch in comment #0.

Yes, there is no other legal issue once you've got copyright paper.

I am not familiar with PE.  Please send your patch to
gdb-patches@sourceware.org for review if you haven't.  Please also include
ChangeLog entry in your mail.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug win32/12716] addresses of exported symbols of a DLL should be looked up by ordinal

[tromey at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=12716

Tom Tromey <tromey at sourceware dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |tromey at sourceware dot org
             Status|NEW                         |WAITING

--- Comment #4 from Tom Tromey <tromey at sourceware dot org> ---
Did this ever get submitted?  Also do we know how to test it?
If it's a workaround for broken DLLs, do we know how they wind up broken?

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug win32/12716] addresses of exported symbols of a DLL should be looked up by ordinal

[pebolle at tiscali dot nl]

https://sourceware.org/bugzilla/show_bug.cgi?id=12716

--- Comment #5 from Paul Bolle <pebolle at tiscali dot nl> ---
Tom, I'm sorry. It's been over a decade. I forgot all about this. I do remember
the FSF (or GNU?) and I got the paperwork sorted out and I transferred as much
of my (future) copyrights to the FSF (or GNU?) as is legally possible. That
document is still in my possession. But the reason I didn't push this any
further? Plain old laziness?

I'm afraid that a decade later I really don't know which problem I was trying
to solve here. My patch might as well been submitted by someone else, because
it's all Chinese to me. I'm actually pretty impressed by what I knew a decade
ago! So if my patch ends up in GDB that would be nice but I can be of little
help at this point to make that happen.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug win32/12716] addresses of exported symbols of a DLL should be looked up by ordinal

[mark at klomp dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=12716

Mark Wielaard <mark at klomp dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mark at klomp dot org

--- Comment #6 from Mark Wielaard <mark at klomp dot org> ---
I can confirm that your paperwork is on file.
You submitted the patch and it was reviewed in this thread:
https://inbox.sourceware.org/gdb-patches/1307879815.1831.0.camel@t41.thuisdomein/

And it looks like something similar to (or inspired on) your patch was
committed as part of:

commit 3999122f0c8766df3a55462df626d607ebe5a735
Author: Pierre Muller <muller@sourceware.org>
Date:   Thu Dec 13 10:44:45 2012 +0000

            * coff-pe-read.h (pe_text_section_offset): Declare new function.
            * coff-pe-read.c (debug_coff_pe_read): New static variable.
            (struct read_pe_section_data): Add section_name field.
            (IMAGE_SCN_CNT_CODE): New macro, if not already defined.
            (IMAGE_SCN_CNT_INITIALIZED_DATA): Ditto.
            (IMAGE_SCN_CNT_UNINITIALIZED_DATA): Ditto.
            (get_pe_section_index): New function.
            (struct pe_sections_info): New type.
            (get_section_vmas): Use new struct pe_sections_info.
            (add_pe_exported_sym): Handle unnamed exported function.
            (add_pe_forwarded_sym): New function.
            (read_pe_truncate_name): Truncate at last dot.
            (pe_as16): New function.
            (read_pe_exported_syms): Use ordinal of function to
            retrieve correct RVA address of function and handle
            forwarded symbol.
            (pe_text_section_offset): New function.
            (show_debug_coff_pe_read): New function.
            (_initialize_coff_pe_read): New function adding
            'set/show debug coff_pe_read' commands.

            * windows-tdep.c (windows_xfer_shared_library): Use
            pe_text_section_offset function instead of possibly wrong
            0x1000 constant for .text sextion offset.

Which has a very long thread discussing it:
https://inbox.sourceware.org/gdb-patches/006001cdaada$00c81f00$02585d00$@muller@ics-cnrs.unistra.fr/t/#u

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug win32/12716] addresses of exported symbols of a DLL should be looked up by ordinal

[tromey at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=12716

Tom Tromey <tromey at sourceware dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|WAITING                     |RESOLVED
         Resolution|---                         |FIXED

--- Comment #7 from Tom Tromey <tromey at sourceware dot org> ---
Thanks to you both.
I'm going to close this.

-- 
You are receiving this mail because:
You are on the CC list for the bug.