[RFC-v4] Fix .text section offset for windows DLL (was Calling __stdcall functions in the inferior)

["Pierre Muller" <pierre.muller@ics-cnrs.unistra.fr>]

[-- Attachment #1: Type: text/plain, Size: 11698 bytes --]

  I attach a new version of my patch to
cope with variable offset of .text section  for Windows OS DLL's.

  I hope I have taken all advices of Joel in account.
In fact, I was unable to satisfy one suggestion...
See below, the answer about my lack of C knowledge. 


  Hoping we are getting closer...

Pierre Muller


(unchanged) ChangeLog entry:

2012-11-25  Pierre Muller  <muller@sourceware.org>

        * 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.
        (pe_as16): New function.
        (IMAGE_SCN_CNT_CODE): New macro, if not already defined.
        (IMAGE_SCN_CNT_INITIALIZED_DATA): Ditto.
        (IMAGE_SCN_CNT_UNINITIALIZED_DATA): Ditto.
        (add_pe_exported_sym): Handle unnamed exported function.
        (add_pe_forwarded_sym): 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.

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Joel Brobecker
> Envoyé : jeudi 22 novembre 2012 18:30
> À : Pierre Muller
> Cc : 'Pedro Alves'; 'Eli Zaretskii'; gdb-patches@sourceware.org
> Objet : Re: [RFC-v3] Fix .text section offset for windows DLL (was Calling
> __stdcall functions in the inferior)
> 
> Hello Pierre,
> 
> > 2012-11-08  Pierre Muller  <muller@sourceware.org>
> >
> > 	* 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.
> > 	(pe_as16): New function.
> > 	(IMAGE_SCN_CNT_CODE): New macro, if not already defined.
> > 	(IMAGE_SCN_CNT_INITIALIZED_DATA): Ditto.
> > 	(IMAGE_SCN_CNT_UNINITIALIZED_DATA): Ditto.
> > 	(add_pe_exported_sym): Handle unnamed exported function.
> > 	(add_pe_forwarded_sym): 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.
> 
> Just a few minor comments...
> 
> > +/* Extract for ABFD the offset of the .text section.
>              ^^^^^ from?
  Of course, thanks for catching this.

> > +   Returns default value 0x1000 if information is not found.  */
> > +extern CORE_ADDR pe_text_section_offset (struct bfd *abfd);
> 
> > +#ifndef IMAGE_SCN_CNT_CODE
> > +# define IMAGE_SCN_CNT_CODE 0x20
> > +#endif
> > +#ifndef IMAGE_SCN_CNT_INITIALIZED_DATA
> > +# define IMAGE_SCN_CNT_INITIALIZED_DATA 0x40
> > +#endif
> > +#ifndef IMAGE_SCN_CNT_UNINITIALIZED_DATA
> > +# define IMAGE_SCN_CNT_UNINITIALIZED_DATA 0x80
> > +#endif
> Do you have an idea of when these macros might not be defined?
> (and where they are normally coming from?). It'd be nice to add
> a comment providing the answer to those questions.


  These are windows specific macros,
but can also be compiled on any other target, if one
of the windows target is included in the target list.
  Thus, I do not really know what type of comment I should add here...

 
> >  static void
> >  add_pe_exported_sym (char *sym_name,
> >  		     unsigned long func_rva,
> > +		     int ordinal,
> >  		     const struct read_pe_section_data *section_data,
> >  		     const char *dll_name, struct objfile *objfile)
> 
> Can you update the funtion documentation to explain what each
> parameter is?

  Done.
 
> > +  if ((section_data->ms_type == mst_unknown) && debug_coff_pe_read)
> > +    printf_filtered (_("Unknown section type for \"%s\" for entry
\"%s\"
> \
> > +in dll \"%s\"\n"),
> > +		     section_data->section_name, sym_name, dll_name);
> 
> There was a discussion in the past about continuations of string
> messages, and we decided to avoid putting the continuation of the
> first column, as it affects the -p switch of the diff. I think you
> can simply do:
> 
>     printf_filtered (_("Unknown section type for \"%s\" for entry"
>                        " \"%s\" in dll \"%s\"\n"),
> 		     section_data->section_name, sym_name, dll_name);
  Modified as suggested.

> > +  if (debug_coff_pe_read > 1)
> > +    printf_filtered (_("Adding exported symbol \"%s\" in dll
\"%s\"\n"),
> > +		     sym_name, dll_name);
> 
> Can you use printf_unfiltered for debug traces? There are several
> instances of this...

Modified using fprintf_unfitered (gdb_stdlog, as per Pedro's suggestion...
 
> > +/* Create a minimal symbol entry for an exported forward symbol.
> > +   Returns 1 if the forwarded function was found 0 otherwise.  */
> 
> "Return" (our style is more "do this, do that", rather than "does this,
> does that"). Can you also document what each parameter is?
> 
> > +  int dll_name_len = strlen (dll_name);
> > +  char *forward_minimal_name = xmalloc (forward_dll_name_len +
> > +					forward_func_name_len + 2);
> 
> Can you use alloca, here? It avoids the need to xfree it later.
> If you need to xmalloc, then you need to xfree it, and I think
> that you'd be safer using a cleanup - some of the functions you
> use might throw an error.
  I used alloca here...
And tried  to use if for the qualified_name too, but that
turned out to be a bad idea...
  The name was apparently still accessed after the function 
returned, despite the fact that the symbol_name is copied...
(This is probably in the hash table, but I didn't completely
understand how that works...)
 
> > +  strncpy (forward_minimal_name, forward_dll_name,
forward_dll_name_len);
> > +  forward_minimal_name[forward_dll_name_len] = '!';
> > +  strcpy (forward_minimal_name + forward_dll_name_len + 1,
> forward_func_name);
> 
> You also have the "concat" function, if that makes things a little
> simple for you.

  xsnprintf ("%s!%s", should be even simpler...
 
> > +      int i;
> > +      for (i = 0; i < forward_dll_name_len; i++)
> 
> Empty line between variable declaration and the rest of the code...

  Still forgetting basics :(
Fixed.
 
> > +  if (debug_coff_pe_read > 1)
> > +    printf_filtered (_("Adding forwarded exported symbol \"%s\" "
> > +		       "in dll \"%s\", pointing to \"%s\"\n"),
> > +		     sym_name, dll_name, forward_minimal_name);
> [l..]
> > +      if (debug_coff_pe_read)
> > +	printf_filtered (_("Unable to find function \"%s\" in dll \"%s\" "
> > +			   ", forward of \"%s\" in dll \"%s\"\n"),
> > +			 forward_func_name, forward_dll_name, sym_name,
> > +			 dll_name);
> 
> printf_unfiltered.
> 
> > +  qualified_name = xmalloc (dll_name_len + strlen (sym_name) + 2);
> 
> Same remarks as for "forward_minimal_name"...

  OK, I change it to alloca...
and did the same in the existing add_pe_exportd_sym function
But as I said above, this was a bad idea :(

> > +  char * last_point = strrchr (dll_name, '.');
> > +  if (last_point != NULL)
> > +    *last_point = '\0';
> 
> Empty line after var declarations...

  Fixed.

 
> > -  struct read_pe_section_data section_data[PE_SECTION_TABLE_SIZE]
> > -    = { {0, 1, 0, mst_text},
> > -  {0, 1, 0, mst_data},
> > -  {0, 1, 0, mst_bss}
> > -  };
> > +  struct read_pe_section_data *section_data;
> [...]
> > +  section_data = xzalloc (PE_SECTION_TABLE_SIZE
> > +			 * sizeof (struct read_pe_section_data));
> > +
> 
> Are we missing a cleanup/xfree?

  I added some, please check that part, as I have
no experience at all with using make_cleanup  
related functions...
  In particular, I didn't really get if it is OK to call
do_cleanups with a possibly NULL argument...
 
> > +  for (i=0; i < PE_SECTION_TABLE_SIZE; i++)
> > +    {
> > +      section_data[i].vma_offset = 0;
> > +      section_data[i].rva_start = 1;
> > +      section_data[i].rva_end = 0;
> > +    };
> > +  section_data[PE_SECTION_INDEX_TEXT].ms_type = mst_text;
> > +  section_data[PE_SECTION_INDEX_TEXT].section_name = ".text";
> > +  section_data[PE_SECTION_INDEX_DATA].ms_type = mst_data;
> > +  section_data[PE_SECTION_INDEX_DATA].section_name = ".data";
> > +  section_data[PE_SECTION_INDEX_BSS].ms_type = mst_bss;
> > +  section_data[PE_SECTION_INDEX_BSS].section_name = ".bss";
> 
> Also, I think it makes it harder to determine what the contents of the
> table is. I suggest you go back to the static definition above, but
> updated with the extra field.

  Sorry, but here my C knowledge is not sufficent:
section_data is now a pointer allocated on heap,
how can I reconcile this with the old static definition?

 
> > -      if (vaddr <= export_rva && vaddr + vsize > export_rva)
> > +      if ((strcmp (sname, ".edata") == 0)
> > +	  || ((vaddr <= export_opthdrrva)
> > +	      && (export_opthdrrva < vaddr + vsize)))
> 
> The extra parentheses around the numerical comparison operators are
> unnecessary, I believe. I think the following is equivalent:
> 
>       if (strcmp (sname, ".edata") == 0
> 	  || (vaddr <= export_opthdrrva && export_opthdrrva < vaddr +
vsize))
> 
> > +	  else if ((export_opthdrrva != vaddr) && debug_coff_pe_read)
> 
> Same here...
> 
> > +      /* Retrieve ordinal value */
> 
> Missing period at end of comment.
> 
> > +      /* This is relatived to ordinal value. */
> 
> Missing second space after the period...
> 
> > +      if ((func_rva >= export_rva)
> > +	  && (func_rva < export_rva + export_size))
> 
> Unnecessary parentheses.
> 
> > +	  if (sep)
> > +	    {
> > +	      int len = (int) (sep - forward_name);
> > +	      forward_dll_name = xmalloc (len + 1);
> 
> Missing empty line after variable declaration.  You might want to
> use a cleanup, just in case something in add_pe_forwarded_sym calls
> error.
> 
> > +	      static char null_char = '\0';
> > +
> > +	      add_pe_exported_sym (&null_char, func_rva, ordinal,
> > +				   section_data, dll_name, objfile);
> 
> Why does this have to be static?

  
 
> Can you make prim_record_minimal_symbol sym_name parameter a "const",
> and then declare...

  I hope you meant changing add_pe_exported_sym only...
I don't want to touch prim_record_minimal_symbol function which is also
used in other C sources...
 
  I removed the null_char by handling NULL also in add_pe_exportd_sym
instead of only supporting a pointer to '\0'.

 
>         const char *empty_name = "";
> 
> ... and pass that to add_pe_exported_sym?
> 
> 
> > +      char sname[8];
> [...]
> > +      bfd_bread (sname, (bfd_size_type) 8, abfd);
> 
> Use sizeof (sname) instead of litteral 8?

  Yes...
> > +static void
> > +show_debug_coff_pe_read (struct ui_file *file, int from_tty,
> > +		struct cmd_list_element *c, const char *value)
> 
> This function needs a short description ("implements ..." is good
> enough).
> 
> --
> Joel

[-- Attachment #2: fix-dll-offset-v5.patch --]
[-- Type: application/octet-stream, Size: 24412 bytes --]

projecttype:gdb
revision:HEAD
email:muller@ics.u-strasbg.fr

2012-11-25  Pierre Muller  <muller@sourceware.org>

	* 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.
	(pe_as16): New function.
	(IMAGE_SCN_CNT_CODE): New macro, if not already defined.
	(IMAGE_SCN_CNT_INITIALIZED_DATA): Ditto.
	(IMAGE_SCN_CNT_UNINITIALIZED_DATA): Ditto.
	(add_pe_exported_sym): Handle unnamed exported function.
	(add_pe_forwarded_sym): 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.

Index: src/gdb/coff-pe-read.h
===================================================================
RCS file: /cvs/src/src/gdb/coff-pe-read.h,v
retrieving revision 1.11
diff -u -p -r1.11 coff-pe-read.h
--- src/gdb/coff-pe-read.h	4 Jan 2012 08:17:00 -0000	1.11
+++ src/gdb/coff-pe-read.h	25 Nov 2012 22:36:50 -0000
@@ -23,9 +23,14 @@
 #define COFF_PE_READ_H
 
 struct objfile;
+struct bfd;
 
 /* Read the export table and convert it to minimal symbol table
    entries */
 extern void read_pe_exported_syms (struct objfile *objfile);
 
+/* Extract from ABFD the offset of the .text section.
+   Returns default value 0x1000 if information is not found.  */
+extern CORE_ADDR pe_text_section_offset (struct bfd *abfd);
+
 #endif /* !defined (COFF_PE_READ_H) */
Index: src/gdb/coff-pe-read.c
===================================================================
RCS file: /cvs/src/src/gdb/coff-pe-read.c,v
retrieving revision 1.18
diff -u -p -r1.18 coff-pe-read.c
--- src/gdb/coff-pe-read.c	7 Nov 2012 20:10:13 -0000	1.18
+++ src/gdb/coff-pe-read.c	25 Nov 2012 22:36:50 -0000
@@ -28,12 +28,23 @@
 #include "bfd.h"
 #include "gdbtypes.h"
 
+#include "command.h"
+#include "gdbcmd.h"
 #include "symtab.h"
 #include "symfile.h"
 #include "objfiles.h"
+#include "common/common-utils.h"
+
+#include <ctype.h>
 
 /* Internal section information */
 
+/* Coff PE read debugging flag:
+   default value is 0,
+   value 1 outputs problems encountered while parsing PE file,
+   value above 1 also lists all generated minimal symbols.  */
+static unsigned int debug_coff_pe_read;
+
 struct read_pe_section_data
 {
   CORE_ADDR vma_offset;		/* Offset to loaded address of section.  */
@@ -41,8 +52,18 @@ struct read_pe_section_data
   unsigned long rva_end;	/* End offset within the pe.  */
   enum minimal_symbol_type ms_type;	/* Type to assign symbols in
 					   section.  */
+  char *section_name;		/* Recorded section name.  */
 };
 
+#ifndef IMAGE_SCN_CNT_CODE
+# define IMAGE_SCN_CNT_CODE 0x20
+#endif
+#ifndef IMAGE_SCN_CNT_INITIALIZED_DATA
+# define IMAGE_SCN_CNT_INITIALIZED_DATA 0x40
+#endif
+#ifndef IMAGE_SCN_CNT_UNINITIALIZED_DATA
+# define IMAGE_SCN_CNT_UNINITIALIZED_DATA 0x80
+#endif
 #define PE_SECTION_INDEX_TEXT     0
 #define PE_SECTION_INDEX_DATA     1
 #define PE_SECTION_INDEX_BSS      2
@@ -95,58 +116,147 @@ get_section_vmas (bfd *abfd, asection *s
     }
 }
 \f
-/* Create a minimal symbol entry for an exported symbol.  */
+/* Create a minimal symbol entry for an exported symbol.
+   SYM_NAME contains the exported name or NULL if exported by ordinal,
+   FUNC_RVA contains the Relative Virtual Address of the symbol,
+   ORDINAL is the ordinal index value of the symbol,
+   SECTION_DATA contains information about the section in which the
+   symbol is declared,
+   DLL_NAME is the internal name of the DLL file,
+   OBJFILE is the objfile struct of DLL_NAME.  */
 
 static void
 add_pe_exported_sym (char *sym_name,
 		     unsigned long func_rva,
+		     int ordinal,
 		     const struct read_pe_section_data *section_data,
 		     const char *dll_name, struct objfile *objfile)
 {
+  char ordinal_name[10];
+  char *qualified_name;
   /* Add the stored offset to get the loaded address of the symbol.  */
-
   CORE_ADDR vma = func_rva + section_data->vma_offset;
-
-  char *qualified_name = 0;
   int dll_name_len = strlen (dll_name);
 
   /* Generate a (hopefully unique) qualified name using the first part
      of the dll name, e.g. KERNEL32!AddAtomA.  This matches the style
      used by windbg from the "Microsoft Debugging Tools for Windows".  */
 
-  qualified_name = xmalloc (dll_name_len + strlen (sym_name) + 2);
+  if (sym_name == NULL || *sym_name == '\0')
+    {
+      xsnprintf (ordinal_name, sizeof (ordinal_name), "#%d", ordinal);
+      sym_name = ordinal_name;
+    }
 
-  strncpy (qualified_name, dll_name, dll_name_len);
-  qualified_name[dll_name_len] = '!';
-  strcpy (qualified_name + dll_name_len + 1, sym_name);
+  qualified_name = xstrprintf ("%s!%s", dll_name, sym_name);
 
-  prim_record_minimal_symbol (qualified_name,
-			      vma, section_data->ms_type, objfile);
+  if ((section_data->ms_type == mst_unknown) && debug_coff_pe_read)
+    fprintf_unfiltered (gdb_stdlog , _("Unknown section type for \"%s\""
+			" for entry \"%s\" in dll \"%s\"\n"),
+			section_data->section_name, sym_name, dll_name);
 
-  xfree (qualified_name);
+  prim_record_minimal_symbol (qualified_name, vma,
+			      section_data->ms_type, objfile);
 
   /* Enter the plain name as well, which might not be unique.  */
-  prim_record_minimal_symbol (sym_name, vma, 
-			      section_data->ms_type, objfile);
+  prim_record_minimal_symbol (sym_name, vma, section_data->ms_type, objfile);
+  if (debug_coff_pe_read > 1)
+    fprintf_unfiltered (gdb_stdlog, _("Adding exported symbol \"%s\""
+			" in dll \"%s\"\n"), sym_name, dll_name);
 }
 
-/* Truncate a dll_name at the first dot character.  */
+/* Create a minimal symbol entry for an exported forward symbol.
+   Return 1 if the forwarded function was found 0 otherwise.
+   SYM_NAME contains the exported name or NULL if exported by ordinal,
+   FORWARD_DLL_NAME is the name of the DLL in which the target symobl resides,
+   FORWARD_FUNC_NAME is the name of the target symbol in that DLL,
+   ORDINAL is the ordinal index value of the symbol,
+   DLL_NAME is the internal name of the DLL file,
+   OBJFILE is the objfile struct of DLL_NAME.  */
 
-static void
-read_pe_truncate_name (char *dll_name)
+
+static int
+add_pe_forwarded_sym (char *sym_name, char *forward_dll_name,
+		      char *forward_func_name, int ordinal,
+		      const char *dll_name, struct objfile *objfile)
 {
-  while (*dll_name)
+  char ordinal_name[10];
+  CORE_ADDR vma;
+  struct objfile *forward_objfile;
+  struct minimal_symbol *msymbol;
+  short section;
+  enum minimal_symbol_type msymtype;
+  int dll_name_len = strlen (dll_name);
+  char *qualified_name;
+  int forward_dll_name_len = strlen (forward_dll_name);
+  int forward_func_name_len = strlen (forward_func_name);
+  int forward_len = forward_dll_name_len + forward_func_name_len + 2;
+  char *forward_minimal_name = alloca (forward_len);
+
+  xsnprintf (forward_minimal_name, forward_len, "%s!%s", forward_dll_name,
+	     forward_func_name);
+
+
+  msymbol = lookup_minimal_symbol_and_objfile (forward_minimal_name,
+					       &forward_objfile);
+
+  if (!msymbol)
     {
-      if ((*dll_name) == '.')
-	{
-	  *dll_name = '\0';	/* truncates and causes loop exit.  */
-	}
+      int i;
 
-      else
-	{
-	  ++dll_name;
-	}
+      for (i = 0; i < forward_dll_name_len; i++)
+	forward_minimal_name[i] = tolower (forward_minimal_name[i]);
+      msymbol = lookup_minimal_symbol_and_objfile (forward_minimal_name,
+						   &forward_objfile);
+    }
+
+  if (!msymbol)
+    {
+      if (debug_coff_pe_read)
+	fprintf_unfiltered (gdb_stdlog, _("Unable to find function \"%s\" in"
+			    " dll \"%s\", forward of \"%s\" in dll \"%s\"\n"),
+			    forward_func_name, forward_dll_name, sym_name,
+			    dll_name);
+      return 0;
+    }
+
+  if (debug_coff_pe_read > 1)
+    fprintf_unfiltered (gdb_stdlog, _("Adding forwarded exported symbol"
+			" \"%s\" in dll \"%s\", pointing to \"%s\"\n"),
+			sym_name, dll_name, forward_minimal_name);
+
+  vma = SYMBOL_VALUE_ADDRESS (msymbol);
+  section = SYMBOL_SECTION (msymbol);
+  msymtype = MSYMBOL_TYPE (msymbol);
+
+  /* Generate a (hopefully unique) qualified name using the first part
+     of the dll name, e.g. KERNEL32!AddAtomA.  This matches the style
+     used by windbg from the "Microsoft Debugging Tools for Windows".  */
+
+  if (sym_name == NULL || *sym_name == '\0')
+    {
+      xsnprintf (ordinal_name, sizeof (ordinal_name), "#%d", ordinal);
+      sym_name = ordinal_name;
     }
+
+  qualified_name = xstrprintf ("%s!%s", dll_name, sym_name);
+
+  prim_record_minimal_symbol (qualified_name, vma, msymtype, objfile);
+
+  /* Enter the plain name as well, which might not be unique.  */
+  prim_record_minimal_symbol (sym_name, vma, msymtype, objfile);
+  return 1;
+}
+
+/* Truncate a dll_name at the last dot character.  */
+
+static void
+read_pe_truncate_name (char *dll_name)
+{
+  char * last_point = strrchr (dll_name, '.');
+
+  if (last_point != NULL)
+    *last_point = '\0';
 }
 \f
 /* Low-level support functions, direct from the ld module pe-dll.c.  */
@@ -171,6 +281,14 @@ pe_get32 (bfd *abfd, int where)
 }
 
 static unsigned int
+pe_as16 (void *ptr)
+{
+  unsigned char *b = ptr;
+
+  return b[0] + (b[1] << 8);
+}
+
+static unsigned int
 pe_as32 (void *ptr)
 {
   unsigned char *b = ptr;
@@ -186,35 +304,53 @@ void
 read_pe_exported_syms (struct objfile *objfile)
 {
   bfd *dll = objfile->obfd;
+  unsigned long nbnormal, nbforward;
   unsigned long pe_header_offset, opthdr_ofs, num_entries, i;
+  unsigned long export_opthdrrva, export_opthdrsize;
   unsigned long export_rva, export_size, nsections, secptr, expptr;
   unsigned long exp_funcbase;
   unsigned char *expdata, *erva;
   unsigned long name_rvas, ordinals, nexp, ordbase;
-  char *dll_name;
+  char *dll_name = (char *) dll->filename;
+  int otherix = PE_SECTION_TABLE_SIZE;
+  int exportix = -1;
   int is_pe64 = 0;
   int is_pe32 = 0;
 
   /* Array elements are for text, data and bss in that order
-     Initialization with start_rva > end_rva guarantees that
+     Initialization with RVA_START > RVA_END guarantees that
      unused sections won't be matched.  */
-  struct read_pe_section_data section_data[PE_SECTION_TABLE_SIZE]
-    = { {0, 1, 0, mst_text},
-  {0, 1, 0, mst_data},
-  {0, 1, 0, mst_bss}
-  };
+  struct read_pe_section_data *section_data;
 
   struct cleanup *back_to = 0;
+  struct cleanup *section_cleanup = 0;
 
   char const *target = bfd_get_target (objfile->obfd);
 
+  section_data = xzalloc (PE_SECTION_TABLE_SIZE
+			 * sizeof (struct read_pe_section_data));
+
+  section_cleanup = make_cleanup (xfree, section_data);
+
+  for (i=0; i < PE_SECTION_TABLE_SIZE; i++)
+    {
+      section_data[i].vma_offset = 0;
+      section_data[i].rva_start = 1;
+      section_data[i].rva_end = 0;
+    };
+  section_data[PE_SECTION_INDEX_TEXT].ms_type = mst_text;
+  section_data[PE_SECTION_INDEX_TEXT].section_name = ".text";
+  section_data[PE_SECTION_INDEX_DATA].ms_type = mst_data;
+  section_data[PE_SECTION_INDEX_DATA].section_name = ".data";
+  section_data[PE_SECTION_INDEX_BSS].ms_type = mst_bss;
+  section_data[PE_SECTION_INDEX_BSS].section_name = ".bss";
+
   is_pe64 = (strcmp (target, "pe-x86-64") == 0
 	     || strcmp (target, "pei-x86-64") == 0);
   is_pe32 = (strcmp (target, "pe-i386") == 0
 	     || strcmp (target, "pei-i386") == 0
 	     || strcmp (target, "pe-arm-wince-little") == 0
 	     || strcmp (target, "pei-arm-wince-little") == 0);
-
   if (!is_pe32 && !is_pe64)
     {
       /* This is not a recognized PE format file.  Abort now, because
@@ -235,21 +371,21 @@ read_pe_exported_syms (struct objfile *o
     {
       return;
     }
-
   if (is_pe64)
     {
-      export_rva = pe_get32 (dll, opthdr_ofs + 112);
-      export_size = pe_get32 (dll, opthdr_ofs + 116);
+      export_opthdrrva = pe_get32 (dll, opthdr_ofs + 112);
+      export_opthdrsize = pe_get32 (dll, opthdr_ofs + 116);
     }
   else
     {
-      export_rva = pe_get32 (dll, opthdr_ofs + 96);
-      export_size = pe_get32 (dll, opthdr_ofs + 100);
+      export_opthdrrva = pe_get32 (dll, opthdr_ofs + 96);
+      export_opthdrsize = pe_get32 (dll, opthdr_ofs + 100);
     }
   nsections = pe_get16 (dll, pe_header_offset + 4 + 2);
   secptr = (pe_header_offset + 4 + 20 +
 	    pe_get16 (dll, pe_header_offset + 4 + 16));
   expptr = 0;
+  export_size = 0;
 
   /* Get the rva and size of the export section.  */
   for (i = 0; i < nsections; i++)
@@ -261,17 +397,31 @@ read_pe_exported_syms (struct objfile *o
       unsigned long fptr = pe_get32 (dll, secptr1 + 20);
 
       bfd_seek (dll, (file_ptr) secptr1, SEEK_SET);
-      bfd_bread (sname, (bfd_size_type) 8, dll);
+      bfd_bread (sname, (bfd_size_type) sizeof (sname), dll);
 
-      if (vaddr <= export_rva && vaddr + vsize > export_rva)
+      if ((strcmp (sname, ".edata") == 0)
+	  || (vaddr <= export_opthdrrva && export_opthdrrva < vaddr + vsize))
 	{
-	  expptr = fptr + (export_rva - vaddr);
-	  if (export_rva + export_size > vaddr + vsize)
-	    export_size = vsize - (export_rva - vaddr);
+	  if (strcmp (sname, ".edata") != 0)
+	    {
+	      if (debug_coff_pe_read)
+		fprintf_unfiltered (gdb_stdlog, _("Export RVA for dll "
+				    "\"%s\" is in section \"%s\"\n"),
+				    dll_name, sname);
+	    }
+	  else if (export_opthdrrva != vaddr && debug_coff_pe_read)
+	    fprintf_unfiltered (gdb_stdlog, _("Wrong value of export RVA"
+				" for dll \"%s\": 0x%lx instead of 0x%lx\n"),
+				dll_name, export_opthdrrva, vaddr);
+	  expptr = fptr + (export_opthdrrva - vaddr);
+	  exportix = i;
 	  break;
 	}
     }
 
+  export_rva = export_opthdrrva;
+  export_size = export_opthdrsize;
+
   if (export_size == 0)
     {
       /* Empty export table.  */
@@ -285,6 +435,7 @@ read_pe_exported_syms (struct objfile *o
       unsigned long secptr1 = secptr + 40 * i;
       unsigned long vsize = pe_get32 (dll, secptr1 + 8);
       unsigned long vaddr = pe_get32 (dll, secptr1 + 12);
+      unsigned long characteristics = pe_get32 (dll, secptr1 + 36);
       char sec_name[9];
       int sectix;
 
@@ -299,6 +450,24 @@ read_pe_exported_syms (struct objfile *o
 	  section_data[sectix].rva_start = vaddr;
 	  section_data[sectix].rva_end = vaddr + vsize;
 	}
+      else
+	{
+	  otherix++;
+	  section_data = xrealloc (section_data, otherix
+				   * sizeof (struct read_pe_section_data));
+	  section_data[otherix - 1].section_name = xstrdup (sec_name);
+	  section_data[otherix - 1].rva_start = vaddr;
+	  section_data[otherix - 1].rva_end = vaddr + vsize;
+	  section_data[otherix - 1].vma_offset = 0;
+	  if (characteristics & IMAGE_SCN_CNT_CODE)
+	    section_data[otherix - 1].ms_type = mst_text;
+	  else if (characteristics & IMAGE_SCN_CNT_INITIALIZED_DATA)
+	    section_data[otherix - 1].ms_type = mst_data;
+	  else if (characteristics & IMAGE_SCN_CNT_UNINITIALIZED_DATA)
+	    section_data[otherix - 1].ms_type = mst_bss;
+	  else
+	    section_data[otherix - 1].ms_type = mst_unknown;
+	}
     }
 
   expdata = (unsigned char *) xmalloc (export_size);
@@ -322,7 +491,7 @@ read_pe_exported_syms (struct objfile *o
   /* Adjust the vma_offsets in case this PE got relocated. This
      assumes that *all* sections share the same relocation offset
      as the text section.  */
-  for (i = 0; i < PE_SECTION_TABLE_SIZE; i++)
+  for (i = 0; i < otherix; i++)
     {
       section_data[i].vma_offset
 	+= ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
@@ -332,31 +501,191 @@ read_pe_exported_syms (struct objfile *o
      lower case for convenience on Windows.  */
   read_pe_truncate_name (dll_name);
 
+  if (debug_coff_pe_read)
+    fprintf_unfiltered (gdb_stdlog, _("DLL \"%s\" has %ld export entries,"
+			" base=%ld\n"), dll_name, nexp, ordbase);
+  nbforward = 0;
+  nbnormal = 0;
   /* Iterate through the list of symbols.  */
   for (i = 0; i < nexp; i++)
     {
       /* Pointer to the names vector.  */
       unsigned long name_rva = pe_as32 (erva + name_rvas + i * 4);
+      /* Retrieve ordinal value.  */
+
+      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);
+      /* This is relatived to ordinal value. */
+      unsigned long func_rva = pe_as32 (erva + exp_funcbase +
+                                        ordinal * 4);
 
       /* Find this symbol's section in our own array.  */
       int sectix = 0;
+      int section_found = 0;
 
-      for (sectix = 0; sectix < PE_SECTION_TABLE_SIZE; ++sectix)
+      /* First handle forward cases.  */
+      if ((func_rva >= export_rva)
+	  && (func_rva < export_rva + export_size))
+	{
+	  char *forward_name = (char *) (erva + func_rva);
+	  char *funcname = (char *) (erva + name_rva);
+	  char *forward_dll_name = forward_name;
+	  char *forward_func_name = forward_name;
+	  char *sep = strrchr (forward_name, '.');
+	  struct cleanup *sep_cleanup = NULL;
+
+	  if (sep)
+	    {
+	      int len = (int) (sep - forward_name);
+
+	      forward_dll_name = xmalloc (len + 1);
+	      sep_cleanup = make_cleanup (xfree, forward_dll_name);
+	      strncpy (forward_dll_name, forward_name, len);
+	      forward_dll_name[len] = '\0';
+	      forward_func_name = ++sep;
+	    }
+	  if (add_pe_forwarded_sym (funcname, forward_dll_name,
+				    forward_func_name, ordinal,
+				    dll_name, objfile) != 0)
+	    ++nbforward;
+	  if (sep_cleanup)
+	    do_cleanups (sep_cleanup);
+	  continue;
+	}
+
+      for (sectix = 0; sectix < otherix; ++sectix)
 	{
 	  if ((func_rva >= section_data[sectix].rva_start)
 	      && (func_rva < section_data[sectix].rva_end))
 	    {
+	      section_found = 1;
 	      add_pe_exported_sym (erva + name_rva,
-				   func_rva,
+				   func_rva, ordinal,
 				   section_data + sectix, dll_name, objfile);
+	      ++nbnormal;
 	      break;
 	    }
 	}
+      if (!section_found)
+	{
+	  char *funcname = (char *) (erva + name_rva);
+
+	  if (name_rva == 0)
+	    {
+	      add_pe_exported_sym (NULL, func_rva, ordinal,
+				   section_data, dll_name, objfile);
+	      ++nbnormal;
+	    }
+	  else if (debug_coff_pe_read)
+	    fprintf_unfiltered (gdb_stdlog, _("Export name \"%s\" ord. %lu,"
+				" RVA 0x%lx in dll \"%s\" not handled\n"),
+				funcname, ordinal, func_rva, dll_name);
+	}
     }
 
+  if (debug_coff_pe_read)
+    fprintf_unfiltered (gdb_stdlog, _("Finished reading \"%s\", exports %ld,"
+			" forwards %ld, total %ld/%ld.\n"), dll_name, nbnormal,
+			nbforward, nbnormal + nbforward, nexp);
   /* Discard expdata.  */
   do_cleanups (back_to);
+  /* Discard section_data.  */
+  do_cleanups (section_cleanup);
+}
+
+/* Extract from ABFD the offset of the .text section.
+   This offset is mainly related to the offset within the file.
+   The value was previously expected to be 0x1000 for all files,
+   but some Windows OS core DLLs seem to use 0x10000 section alignement
+   which modified the return value of that function.
+   Still return default 0x1000 value if ABFD is NULL or
+   if '.text' section is not found, but that should not happen...  */
+
+#define DEFAULT_COFF_PE_TEXT_SECTION_OFFSET 0x1000
+
+CORE_ADDR
+pe_text_section_offset (struct bfd *abfd)
+
+{
+  unsigned long pe_header_offset, opthdr_ofs, num_entries, i;
+  unsigned long export_rva, export_size, nsections, secptr, expptr;
+  unsigned long exp_funcbase;
+  unsigned char *expdata, *erva;
+  unsigned long name_rvas, ordinals, nexp, ordbase;
+  char *dll_name;
+  int is_pe64 = 0;
+  int is_pe32 = 0;
+  char const *target;
+
+  if (!abfd)
+    return DEFAULT_COFF_PE_TEXT_SECTION_OFFSET;
+
+  target = bfd_get_target (abfd);
+
+  is_pe64 = (strcmp (target, "pe-x86-64") == 0
+	     || strcmp (target, "pei-x86-64") == 0);
+  is_pe32 = (strcmp (target, "pe-i386") == 0
+	     || strcmp (target, "pei-i386") == 0
+	     || strcmp (target, "pe-arm-wince-little") == 0
+	     || strcmp (target, "pei-arm-wince-little") == 0);
+
+  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 DEFAULT_COFF_PE_TEXT_SECTION_OFFSET;
+    }
+
+  /* Get pe_header, optional header and numbers of sections.  */
+  pe_header_offset = pe_get32 (abfd, 0x3c);
+  opthdr_ofs = pe_header_offset + 4 + 20;
+  nsections = pe_get16 (abfd, pe_header_offset + 4 + 2);
+  secptr = (pe_header_offset + 4 + 20 +
+	    pe_get16 (abfd, pe_header_offset + 4 + 16));
+
+  /* Get the rva and size of the export section.  */
+  for (i = 0; i < nsections; i++)
+    {
+      char sname[8];
+      unsigned long secptr1 = secptr + 40 * i;
+      unsigned long vaddr = pe_get32 (abfd, secptr1 + 12);
+
+      bfd_seek (abfd, (file_ptr) secptr1, SEEK_SET);
+      bfd_bread (sname, (bfd_size_type) 8, abfd);
+      if (strcmp (sname, ".text") == 0)
+	return vaddr;
+    }
+
+  return DEFAULT_COFF_PE_TEXT_SECTION_OFFSET;
+}
+
+/* Implements "show debug coff_pe_read" command.  */
+
+static void
+show_debug_coff_pe_read (struct ui_file *file, int from_tty,
+			 struct cmd_list_element *c, const char *value)
+{
+  fprintf_filtered (file, _("Coff PE read debugging is %s.\n"), value);
+}
+
+/* Provide a prototype to silence -Wmissing-prototypes.  */
+
+void _initialize_coff_pe_read (void);
+
+/* Adds "Set/show debug coff_pe_read" commands.  */
+
+void
+_initialize_coff_pe_read (void)
+{
+  add_setshow_uinteger_cmd ("coff_pe_read", class_maintenance,
+			    &debug_coff_pe_read,
+			    _("Set coff PE read debugging."),
+			    _("Show coff PE read debugging."),
+			    _("When set, debugging messages for coff reading "
+			      "of exported symbols are displayed."),
+			    NULL, show_debug_coff_pe_read,
+			    &setdebuglist, &showdebuglist);
 }
Index: src/gdb/windows-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/windows-tdep.c,v
retrieving revision 1.17
diff -u -p -r1.17 windows-tdep.c
--- src/gdb/windows-tdep.c	9 Nov 2012 19:58:02 -0000	1.17
+++ src/gdb/windows-tdep.c	25 Nov 2012 22:36:51 -0000
@@ -27,6 +27,10 @@
 #include "gdbcmd.h"
 #include "gdbthread.h"
 #include "objfiles.h"
+#include "symfile.h"
+#include "coff-pe-read.h"
+#include "gdb_bfd.h"
+#include "complaints.h"
 
 struct cmd_list_element *info_w32_cmdlist;
 
@@ -387,15 +391,21 @@ windows_xfer_shared_library (const char*
 			     struct gdbarch *gdbarch, struct obstack *obstack)
 {
   char *p;
+  struct bfd * dll;
+  CORE_ADDR text_offset;
+
   obstack_grow_str (obstack, "<library name=\"");
   p = xml_escape_text (so_name);
   obstack_grow_str (obstack, p);
   xfree (p);
   obstack_grow_str (obstack, "\"><segment address=\"");
-  /* The symbols in a dll are offset by 0x1000, which is the
-     offset from 0 of the first byte in an image - because of the file
-     header and the section alignment.  */
-  obstack_grow_str (obstack, paddress (gdbarch, load_addr + 0x1000));
+  dll = gdb_bfd_open_maybe_remote (so_name);
+  /* The following calls are OK even if dll is NULL.
+     The default value 0x1000 is returned by pe_text_section_offset
+     in that case.  */
+  text_offset = pe_text_section_offset (dll);
+  gdb_bfd_unref (dll);
+  obstack_grow_str (obstack, paddress (gdbarch, load_addr + text_offset));
   obstack_grow_str (obstack, "\"/></library>");
 }