[PATCH] Fix up msymbol type of dll trampoline to mst_solib_trampoline

[PATCH] Fix up msymbol type of dll trampoline to mst_solib_trampoline

[Yao Qi]

Hi,
I find the following fail when testing native mingw32 gdb,

(gdb) br foo
Breakpoint 3 at 0x401418 (2 locations)
FAIL: gdb.base/solib-symbol.exp: foo in libmd

two breakpoint locations are set on "<foo>" (the dll trampoline) and
"foo" (the function in dll).  It is wrong because dll has been loaded,
and it is expected to set one breakpoint on "foo" in dll only.  Native
Linux GDB behaves correctly in this case.

When breakpoint is set in CLI, like "b foo", GDB will search the
minimal symbols of "foo" when parsing the linespec "foo".  During the
search, GDB will sort minimal symbols in classification order and only
record the minimal symbols with the lowest classification.  See how
classify_mtype is called in search_minsyms_for_name.  On Linux, on
this point, there are two minimal symbols "foo" with different types,
one is the function in shared lib (mst_text) and the other is the plt
stub (mst_solib_trampoline).  According to the rule in classify_mtype
and algorithm in search_minsyms_for_name, only one minimal symbol,
whose type is mst_text, is record, and only one breakpoint location is
set on it finally.

However, on windows, there are two minimal symbols "foo" too, one is
the function in dll, and the other is the dll trampoline (which is
similar to plt stub, IMO).  The type of both of these minimal symbols
is "mst_text", so the have the same priority in classify_mtype, and
two of them are recorded.  That is the root cause of this problem.

I think the right fix could be setting the type of minimal symbol to
"mst_solib_trampoline" if it is the dll trampoline.  Read the comments
to mst_solib_trampoline get this idea confirmed,

  /* GDB uses mst_solib_trampoline for the start address of a shared
     library trampoline entry.  Breakpoints for shared library functions
     are put there if the shared library is not yet loaded.
     After the shared library is loaded, lookup_minimal_symbol will
     prefer the minimal symbol from the shared library (usually
     a mst_text symbol) over the mst_solib_trampoline symbol, and the
     breakpoints will be moved to their true address in the shared
     library via breakpoint_re_set.  */
  mst_solib_trampoline,		/* Shared library trampoline code */

The rationale of this patch is to fix up the type of minimal symbol to
"mst_solib_trampoline" when reading the file if the minimal symbol is
a dll trampoline.  We find that there is always a symbol "_impl_foo"
coexists with dll trampoline "foo" in the current minimal symbols red
in.  Our approach is to collect all minimal symbols which name has
prefix "_imp_", remove the prefix, and find minimal symbols.  If
found, it is a dll trampoline.  For example, there are several
symbols,

  foo, bar, _imp_foo, _imp_baz,

we record symbols with "_imp_" prefix in a set {_imp_foo, _imp_baz},
remove prefix {foo, baz}, and look for them in these symbols.  Symbol
foo is found, and it is a dll trampoline.

Regression test mingw32 native gdb with a remote host board file.  The
fail I mentioned above is fixed.

gdb:

2013-06-24  Yao Qi  <yao@codesourcery.com>

	* coffread.c: Use DEF_VEC_P to define vector type.
	(coff_symtab_read): Define local variable 'dll_trampoline'.
	Record minimal symbols into 'dll_trampoline' if their names
	have prefix "_imp_ or "__imp_".  Set the type of minimal
	symbol to 'mst_solib_trampoline' if it is a dll trampoline.
	* minsyms.c (prim_find_minimal_symbol): New function.
	* minsyms.h (prim_find_minimal_symbol): Declare.
---
 gdb/coffread.c |   43 +++++++++++++++++++++++++++++++++++++++++++
 gdb/minsyms.c  |   27 +++++++++++++++++++++++++++
 gdb/minsyms.h  |    2 ++
 3 files changed, 72 insertions(+), 0 deletions(-)

diff --git a/gdb/coffread.c b/gdb/coffread.c
index bf39085..c0f08b8 100644
--- a/gdb/coffread.c
+++ b/gdb/coffread.c
@@ -724,6 +724,9 @@ coff_symfile_finish (struct objfile *objfile)
 }
 \f
 
+typedef struct minimal_symbol *msymbolp;
+DEF_VEC_P (msymbolp);
+
 /* Given pointers to a symbol table in coff style exec file,
    analyze them and create struct symtab's describing the symbols.
    NSYMS is the number of symbols in the symbol table.
@@ -757,6 +760,8 @@ coff_symtab_read (long symtab_offset, unsigned int nsyms,
   int val;
   CORE_ADDR tmpaddr;
   struct minimal_symbol *msym;
+  /* A set of minimal_symbol which has prefix "__imp_" or "_imp_".  */
+  VEC (msymbolp) *name_prefix_imp = NULL;
 
   /* Work around a stdio bug in SunOS4.1.1 (this makes me nervous....
      it's hard to know I've really worked around it.  The fix should
@@ -1006,6 +1011,16 @@ coff_symtab_read (long symtab_offset, unsigned int nsyms,
 		SYMBOL_VALUE (sym) = tmpaddr;
 		SYMBOL_SECTION (sym) = sec;
 	      }
+
+	    /* Record minimal symbols which name are prefixed by "__imp_"
+	       or "_imp_" in set NAME_PREFIX_IMP if their type is
+	       mst_data.  Note that 'maintenance print msymbols' shows
+	       that type of these "_imp_XXXX" symbols is mst_data.  */
+	    if (msym != NULL && ms_type == mst_data
+		&& (strncmp (cs->c_name, "__imp_", 6) == 0
+		    || strncmp (cs->c_name, "_imp_", 5) == 0))
+	      VEC_safe_push (msymbolp, name_prefix_imp, msym);
+
 	  }
 	  break;
 
@@ -1151,6 +1166,34 @@ coff_symtab_read (long symtab_offset, unsigned int nsyms,
 	}
     }
 
+  if (pe_file)
+    {
+      int ix;
+      struct minimal_symbol *msym_dll;
+
+      /* Set NAME_PREFIX_IMP contains minimal symbols which name is
+	 prefixed by "__imp_" or "_imp_".  In each iteration, look for the
+	 minimal symbols just red in by matching the minimal symbol name
+	 without the prefix.  */
+      for (ix = 0;
+	   VEC_iterate (msymbolp, name_prefix_imp, ix, msym_dll);
+	   ix++)
+	{
+	  char *buffer = xstrdup (SYMBOL_LINKAGE_NAME (msym_dll));
+	  const char *name = (buffer[1] == '_' ? &buffer[7] : &buffer[6]);
+	  struct minimal_symbol *found
+	    = prim_find_minimal_symbol (name);
+
+	  /* If found, there are symbols named "_imp_foo" and "foo"
+	     respectively red in from the current objfile.  Set the type 
+	  of symbol "foo" as 'mst_solib_trampoline'.  */
+	  if (found != NULL && MSYMBOL_TYPE (found) == mst_text)
+	    MSYMBOL_TYPE (found) = mst_solib_trampoline;
+
+	  xfree (buffer);
+	}
+    }
+
   if ((nsyms == 0) && (pe_file))
     {
       /* We've got no debugging symbols, but it's a portable
diff --git a/gdb/minsyms.c b/gdb/minsyms.c
index 89e538a..f5c96db 100644
--- a/gdb/minsyms.c
+++ b/gdb/minsyms.c
@@ -1257,6 +1257,33 @@ install_minimal_symbols (struct objfile *objfile)
     }
 }
 
+/* Look for the minimal symbol which name is NAME.  Return NULL if not
+   found.  */
+
+struct minimal_symbol *
+prim_find_minimal_symbol (const char *name)
+{
+  struct msym_bunch *bunch;
+  int max;
+
+  max = msym_bunch_index;
+  for (bunch = msym_bunch; bunch != NULL; bunch = bunch->next)
+    {
+      int bindex;
+
+      for (bindex = 0; bindex < max; bindex++)
+	{
+	  struct minimal_symbol *msym = &bunch->contents[bindex];
+
+	  if (strcmp (name, SYMBOL_LINKAGE_NAME (msym)) == 0)
+	    return msym;
+	}
+      max = BUNCH_SIZE;
+    }
+
+  return NULL;
+}
+
 /* See minsyms.h.  */
 
 void
diff --git a/gdb/minsyms.h b/gdb/minsyms.h
index 4d48477..137ee00 100644
--- a/gdb/minsyms.h
+++ b/gdb/minsyms.h
@@ -252,4 +252,6 @@ void iterate_over_minimal_symbols (struct objfile *objf,
 						     void *),
 				   void *user_data);
 
+struct minimal_symbol* prim_find_minimal_symbol (const char *name);
+
 #endif /* MINSYMS_H */
-- 
1.7.7.6

Re: [PATCH] Fix up msymbol type of dll trampoline to mst_solib_trampoline

[Tom Tromey]

>>>>> "Yao" == Yao Qi <yao@codesourcery.com> writes:

Yao> The rationale of this patch is to fix up the type of minimal symbol to
Yao> "mst_solib_trampoline" when reading the file if the minimal symbol is
Yao> a dll trampoline.  We find that there is always a symbol "_impl_foo"
Yao> coexists with dll trampoline "foo" in the current minimal symbols red
Yao> in.  Our approach is to collect all minimal symbols which name has
Yao> prefix "_imp_", remove the prefix, and find minimal symbols.  If
Yao> found, it is a dll trampoline.

This seems reasonable to me.

Yao> +	 minimal symbols just red in by matching the minimal symbol name

s/red/read/

Yao> +	  char *buffer = xstrdup (SYMBOL_LINKAGE_NAME (msym_dll));

I don't think you need to copy the name here.

Yao> +/* Look for the minimal symbol which name is NAME.  Return NULL if not

"whose name".

I think this comment needs to be expanded (and moved, see below).
It should at least mention that this only searches minsyms that are
currently being constructed.  Otherwise it isn't clear why you would use
this function as opposed to lookup_minimal_symbol_and_objfile.

Yao> +struct minimal_symbol* prim_find_minimal_symbol (const char *name);

The comment should go here, the way it does for other functions in the
minsyms module.

Also, the first "*" is in the wrong place.

thanks,
Tom

Re: [PATCH] Fix up msymbol type of dll trampoline to mst_solib_trampoline

[Yao Qi]

On 06/28/2013 04:42 AM, Tom Tromey wrote:
> Yao> +	 minimal symbols just red in by matching the minimal symbol name
> 
> s/red/read/
> 

Fixed.

> Yao> +	  char *buffer = xstrdup (SYMBOL_LINKAGE_NAME (msym_dll));
> 
> I don't think you need to copy the name here.
> 

Oh, yes.

> Yao> +/* Look for the minimal symbol which name is NAME.  Return NULL if not
> 
> "whose name".

Fixed.

> 
> I think this comment needs to be expanded (and moved, see below).
> It should at least mention that this only searches minsyms that are
> currently being constructed.  Otherwise it isn't clear why you would use
> this function as opposed to lookup_minimal_symbol_and_objfile.
> 

OK, the comments are moved to the declaration in minsyms.h, and mention
that look for minsyms "in minimal symbols that are currently being
constructed".

> Yao> +struct minimal_symbol* prim_find_minimal_symbol (const char *name);
> 
> The comment should go here, the way it does for other functions in the
> minsyms module.
> 
> Also, the first "*" is in the wrong place.

Fixed.

-- 
Yao (齐尧)

gdb:

2013-06-28  Yao Qi  <yao@codesourcery.com>

	* coffread.c: Use DEF_VEC_P to define vector type.
	(coff_symtab_read): Define local variable 'dll_trampoline'.
	Record minimal symbols into 'dll_trampoline' if their names
	have prefix "_imp_ or "__imp_".  Set the type of minimal
	symbol to 'mst_solib_trampoline' if it is a dll trampoline.
	* minsyms.c (prim_find_minimal_symbol): New function.
	* minsyms.h (prim_find_minimal_symbol): Declare.
---
 gdb/coffread.c |   41 +++++++++++++++++++++++++++++++++++++++++
 gdb/minsyms.c  |   26 ++++++++++++++++++++++++++
 gdb/minsyms.h  |    5 +++++
 3 files changed, 72 insertions(+), 0 deletions(-)

diff --git a/gdb/coffread.c b/gdb/coffread.c
index bf39085..62f164c 100644
--- a/gdb/coffread.c
+++ b/gdb/coffread.c
@@ -724,6 +724,9 @@ coff_symfile_finish (struct objfile *objfile)
 }
 \f
 
+typedef struct minimal_symbol *msymbolp;
+DEF_VEC_P (msymbolp);
+
 /* Given pointers to a symbol table in coff style exec file,
    analyze them and create struct symtab's describing the symbols.
    NSYMS is the number of symbols in the symbol table.
@@ -757,6 +760,8 @@ coff_symtab_read (long symtab_offset, unsigned int nsyms,
   int val;
   CORE_ADDR tmpaddr;
   struct minimal_symbol *msym;
+  /* A set of minimal_symbol which has prefix "__imp_" or "_imp_".  */
+  VEC (msymbolp) *name_prefix_imp = NULL;
 
   /* Work around a stdio bug in SunOS4.1.1 (this makes me nervous....
      it's hard to know I've really worked around it.  The fix should
@@ -1006,6 +1011,16 @@ coff_symtab_read (long symtab_offset, unsigned int nsyms,
 		SYMBOL_VALUE (sym) = tmpaddr;
 		SYMBOL_SECTION (sym) = sec;
 	      }
+
+	    /* Record minimal symbols whose name are prefixed by "__imp_"
+	       or "_imp_" in set NAME_PREFIX_IMP if their type is
+	       mst_data.  Note that 'maintenance print msymbols' shows
+	       that type of these "_imp_XXXX" symbols is mst_data.  */
+	    if (msym != NULL && ms_type == mst_data
+		&& (strncmp (cs->c_name, "__imp_", 6) == 0
+		    || strncmp (cs->c_name, "_imp_", 5) == 0))
+	      VEC_safe_push (msymbolp, name_prefix_imp, msym);
+
 	  }
 	  break;
 
@@ -1151,6 +1166,32 @@ coff_symtab_read (long symtab_offset, unsigned int nsyms,
 	}
     }
 
+  if (pe_file)
+    {
+      int ix;
+      struct minimal_symbol *msym_dll;
+
+      /* Set NAME_PREFIX_IMP contains minimal symbols whose name is
+	 prefixed by "__imp_" or "_imp_".  In each iteration, look for the
+	 minimal symbols just read in by matching the minimal symbol name
+	 without the prefix.  */
+      for (ix = 0;
+	   VEC_iterate (msymbolp, name_prefix_imp, ix, msym_dll);
+	   ix++)
+	{
+	  const char *sname = SYMBOL_LINKAGE_NAME (msym_dll);
+	  const char *name = (sname[1] == '_' ? &sname[7] : &sname[6]);
+	  struct minimal_symbol *found
+	    = prim_find_minimal_symbol (name);
+
+	  /* If found, there are symbols named "_imp_foo" and "foo"
+	     respectively read in from the current objfile.  Set the type
+	     of symbol "foo" as 'mst_solib_trampoline'.  */
+	  if (found != NULL && MSYMBOL_TYPE (found) == mst_text)
+	    MSYMBOL_TYPE (found) = mst_solib_trampoline;
+	}
+    }
+
   if ((nsyms == 0) && (pe_file))
     {
       /* We've got no debugging symbols, but it's a portable
diff --git a/gdb/minsyms.c b/gdb/minsyms.c
index 89e538a..2d18545 100644
--- a/gdb/minsyms.c
+++ b/gdb/minsyms.c
@@ -1259,6 +1259,32 @@ install_minimal_symbols (struct objfile *objfile)
 
 /* See minsyms.h.  */
 
+struct minimal_symbol *
+prim_find_minimal_symbol (const char *name)
+{
+  struct msym_bunch *bunch;
+  int max;
+
+  max = msym_bunch_index;
+  for (bunch = msym_bunch; bunch != NULL; bunch = bunch->next)
+    {
+      int bindex;
+
+      for (bindex = 0; bindex < max; bindex++)
+	{
+	  struct minimal_symbol *msym = &bunch->contents[bindex];
+
+	  if (strcmp (name, SYMBOL_LINKAGE_NAME (msym)) == 0)
+	    return msym;
+	}
+      max = BUNCH_SIZE;
+    }
+
+  return NULL;
+}
+
+/* See minsyms.h.  */
+
 void
 terminate_minimal_symbol_table (struct objfile *objfile)
 {
diff --git a/gdb/minsyms.h b/gdb/minsyms.h
index 4d48477..3500ec5 100644
--- a/gdb/minsyms.h
+++ b/gdb/minsyms.h
@@ -252,4 +252,9 @@ void iterate_over_minimal_symbols (struct objfile *objf,
 						     void *),
 				   void *user_data);
 
+/* Look for the minimal symbol whose name is NAME in minimal symbols that
+   are currently being constructed.  Return NULL if not found.  */
+
+struct minimal_symbol *prim_find_minimal_symbol (const char *name);
+
 #endif /* MINSYMS_H */
-- 
1.7.7.6

Re: [PATCH] Fix up msymbol type of dll trampoline to mst_solib_trampoline

[Tom Tromey]

>>>>> "Yao" == Yao Qi <yao@codesourcery.com> writes:

Yao> +      for (ix = 0;
Yao> +	   VEC_iterate (msymbolp, name_prefix_imp, ix, msym_dll);
Yao> +	   ix++)
Yao> +	{
Yao> +	  const char *sname = SYMBOL_LINKAGE_NAME (msym_dll);
Yao> +	  const char *name = (sname[1] == '_' ? &sname[7] : &sname[6]);
Yao> +	  struct minimal_symbol *found
Yao> +	    = prim_find_minimal_symbol (name);
Yao> +
Yao> +	  /* If found, there are symbols named "_imp_foo" and "foo"
Yao> +	     respectively read in from the current objfile.  Set the type
Yao> +	     of symbol "foo" as 'mst_solib_trampoline'.  */
Yao> +	  if (found != NULL && MSYMBOL_TYPE (found) == mst_text)
Yao> +	    MSYMBOL_TYPE (found) = mst_solib_trampoline;

It seems to me that it would be more efficient to keep a hash table of
minsyms under construction, and then do this lookup when entering a new
minsym.  This would avoid repeated loops over all minsyms being defined.

That is, if the new symbol is _imp_x, look up x.  If the symbol is x,
look up _imp_x.  Then modify a symbol if needed.

Tom

Re: [PATCH] Fix up msymbol type of dll trampoline to mst_solib_trampoline

[Yao Qi]

On 06/28/2013 11:43 PM, Tom Tromey wrote:
> It seems to me that it would be more efficient to keep a hash table of
> minsyms under construction, and then do this lookup when entering a new
> minsym.  This would avoid repeated loops over all minsyms being defined.
> 
> That is, if the new symbol is _imp_x, look up x.  If the symbol is x,
> look up _imp_x.  Then modify a symbol if needed.

In the updated patch, I use hash table when looking for up minsyms, but
in a little different way.  After currently constructing minsyms are
installed to OBJFILE, there is a hash table built, we can use that one,
instead of maintaining a separate one.  That is, after the hash table
of minsyms of OBJFILE is built up, we can iterate all minsyms, if
symbol is _imp_x, look up x in the hash table.  If found, modify the
found's type.

Regression tested on i686-pc-mingw32.  The following fail is fixed.

 FAIL: gdb.base/solib-symbol.exp: foo in libmd

-- 
Yao (齐尧)

gdb:

2013-07-03  Yao Qi  <yao@codesourcery.com>

	* coffread.c (coff_symfile_read): Iterate over minimal symbols,
	if the name is prefixed by "__imp_" or "_imp_", look for minimal
	symbol without prefix.  If found, set its type to
	'mst_solib_trampoline'.
---
 gdb/coffread.c |   30 ++++++++++++++++++++++++++++++
 1 files changed, 30 insertions(+), 0 deletions(-)

diff --git a/gdb/coffread.c b/gdb/coffread.c
index bf39085..f9bb59f 100644
--- a/gdb/coffread.c
+++ b/gdb/coffread.c
@@ -650,6 +650,36 @@ coff_symfile_read (struct objfile *objfile, int symfile_flags)
 
   install_minimal_symbols (objfile);
 
+  if (pe_file)
+    {
+      int i;
+
+      for (i = objfile->minimal_symbol_count; i > 0; i--)
+	{
+	  struct minimal_symbol *msym = &objfile->msymbols[i];
+	  const char *name = SYMBOL_LINKAGE_NAME (msym);
+
+	  /* If the minimal symbols whose name are prefixed by "__imp_"
+	     or "_imp_", get rid of the prefix, and search the minimal
+	     symbol in OBJFILE.  Note that 'maintenance print msymbols'
+	     shows that type of these "_imp_XXXX" symbols is mst_data.  */
+	  if (MSYMBOL_TYPE (msym) == mst_data
+	      && (strncmp (name, "__imp_", 6) == 0
+		  || strncmp (name, "_imp_", 5) == 0))
+	    {
+	      const char *name1 = (name[1] == '_' ? &name[7] : &name[6]);
+	      struct minimal_symbol *found;
+
+	      found = lookup_minimal_symbol (name1, NULL, objfile);
+	      /* If found, there are symbols named "_imp_foo" and "foo"
+		 respectively in OBJFILE.  Set the type of symbol "foo"
+		 as 'mst_solib_trampoline'.  */
+	      if (found != NULL && MSYMBOL_TYPE (found) == mst_text)
+		MSYMBOL_TYPE (found) = mst_solib_trampoline;
+	    }
+	}
+    }
+
   /* Free the installed minimal symbol data.  */
   do_cleanups (cleanup_minimal_symbols);
 
-- 
1.7.7.6

Re: [PATCH] Fix up msymbol type of dll trampoline to mst_solib_trampoline

[asmwarrior]

This is my testing on the patch, it works fine.
It looks like the issue only happens when __declspec(dllimport) is NOT used when building the exe file.

Here is the source code:
add.c:
--------------------------------------------------------------
//this file will generate a dll

__declspec(dllexport) int __cdecl Add_C(int a, int b)
{
  return (a + b);
}

__declspec(dllexport) int __stdcall Add_S(int a, int b)
{
  return (a + b);
}
--------------------------------------------------------------
main.c:
--------------------------------------------------------------
//calling the dll

#include <stdlib.h>
#include <stdio.h>

#if USE_DLLIMPORT
__declspec(dllimport)  int __cdecl   Add_C(int a, int b);
__declspec(dllimport)  int __stdcall Add_S(int a, int b);
#else
extern int __cdecl   Add_C(int a, int b);
extern int __stdcall Add_S(int a, int b);
#endif // USE_DLLIMPORT


int main(int argc, char** argv)
{

  int a = Add_C(1, 2);
  printf("%d\n", a);
  a = Add_S(3,4);
  printf("%d\n", a);

  return 0;
}
--------------------------------------------------------------
I have two GDB, the old GDB named gdb.exe does not have your patch, 
the new gdbnew.exe has your patch applied.
See the log below (I'm using mingw gcc 4.6.3)


--------------------------------------------------------------
E:\code\cb\test_code\learndll>gcc -g -c -o add.o add.c

E:\code\cb\test_code\learndll>gcc -o add.dll add.o -shared -Wl,--out-implib,liba
dd.a
Creating library file: libadd.a

E:\code\cb\test_code\learndll>gcc -g -c -o main.o main.c

E:\code\cb\test_code\learndll>gcc -o main.exe main.o -L. -ladd

E:\code\cb\test_code\learndll>gdb main.exe
GNU gdb (GDB) 7.6.50.20130607-cvs
Copyright (C) 2013 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "mingw32".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>...
Reading symbols from E:\code\cb\test_code\learndll\main.exe...done.
(gdb) b main.c:18
Breakpoint 1 at 0x4016f6: file main.c, line 18.
(gdb) r
Starting program: E:\code\cb\test_code\learndll\main.exe
[New Thread 2596.0x9d0]

Breakpoint 1, main (argc=1, argv=0x3e3f80) at main.c:18
18        int a = Add_C(1, 2);
(gdb) b Add_C
Breakpoint 2 at 0x40175c (2 locations)
(gdb) info symbol 0x40175c
Add_C in section .text of E:\code\cb\test_code\learndll\main.exe
(gdb) b Add_S
Function "Add_S" not defined.
Make breakpoint pending on future shared library load? (y or [n]) n
(gdb) c
Continuing.

Breakpoint 2, 0x0040175c in Add_C ()
(gdb) c
Continuing.

Breakpoint 2, Add_C (a=1, b=2) at add.c:5
5         return (a + b);
(gdb) l
1       //this file will generate a dll
2
3       __declspec(dllexport) int __cdecl Add_C(int a, int b)
4       {
5         return (a + b);
6       }
7
8       __declspec(dllexport) int __stdcall Add_S(int a, int b)
9       {
10        return (a + b);
(gdb) c
Continuing.
3
7
[Inferior 1 (process 2596) exited normally]
(gdb) q
--------------------------------------------------------------


The main.exe above does not use the __declspec(dllimport), so you will see 
(gdb) b Add_C
Breakpoint 2 at 0x40175c (2 locations)

Now, with the new gdb, it works quite well.
--------------------------------------------------------------
E:\code\cb\test_code\learndll>gdbnew main.exe
GNU gdb (GDB) 7.6.50.20130705-cvs
Copyright (C) 2013 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "mingw32".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>...
Reading symbols from E:\code\cb\test_code\learndll\main.exe...done.
(gdb) b main.c:18
Breakpoint 1 at 0x4016f6: file main.c, line 18.
(gdb) r
Starting program: E:\code\cb\test_code\learndll\main.exe
[New Thread 2496.0xcac]

Breakpoint 1, main (argc=1, argv=0x3e3f80) at main.c:18
18        int a = Add_C(1, 2);
(gdb) b Add_C
Breakpoint 2 at 0x70f41703: file add.c, line 5.
(gdb) c
Continuing.

Breakpoint 2, Add_C (a=1, b=2) at add.c:5
5         return (a + b);
(gdb) l
1       //this file will generate a dll
2
3       __declspec(dllexport) int __cdecl Add_C(int a, int b)
4       {
5         return (a + b);
6       }
7
8       __declspec(dllexport) int __stdcall Add_S(int a, int b)
9       {
10        return (a + b);
(gdb) c
Continuing.
3
7
[Inferior 1 (process 2496) exited normally]
(gdb) q
--------------------------------------------------------------




Now, I build the main.exe with __declspec(dllimport)
--------------------------------------------------------------

E:\code\cb\test_code\learndll>gcc -g -c -o main.o main.c -DUSE_DLLIMPORT

E:\code\cb\test_code\learndll>gcc -o main.exe main.o -L. -ladd

E:\code\cb\test_code\learndll>gdb main.exe
GNU gdb (GDB) 7.6.50.20130607-cvs
Copyright (C) 2013 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "mingw32".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>...
Reading symbols from E:\code\cb\test_code\learndll\main.exe...done.
(gdb) b main.c:18
Breakpoint 1 at 0x4016f6: file main.c, line 18.
(gdb) r
Starting program: E:\code\cb\test_code\learndll\main.exe
[New Thread 3916.0x5bc]

Breakpoint 1, main (argc=1, argv=0x3e3f80) at main.c:18
18        int a = Add_C(1, 2);
(gdb) b Add_C
Breakpoint 2 at 0x70f41703: file add.c, line 5.
(gdb) c
Continuing.

Breakpoint 2, Add_C (a=1, b=2) at add.c:5
5         return (a + b);
(gdb) c
Continuing.
3
7
[Inferior 1 (process 3916) exited normally]
(gdb) q
--------------------------------------------------------------
Both the gdb.exe and gdbnew.exe has the same result. (only one location when using "b Add_C")


BTW:
1, It looks like I can't set a breakpoint on a function with is __stdcall calling convention
When I type: 
(gdb) b Add_S
Function "Add_S" not defined.
In fact, the symbol name about "Add_S" function is "Add_S@8", (This can be seen from the disassembler)
I don't know how to set such breakpoint by function names.

2, I see there are symbol names for Add_C: one is named "Add_C" and the other is "_imp__Add_C", 
so it has "_imp__" before "Add_C". (One underline prefix and double underlines postfix).

Yuanhui Zhang

Re: [PATCH] Fix up msymbol type of dll trampoline to mst_solib_trampoline

[Yao Qi]

On 07/05/2013 04:50 PM, asmwarrior wrote:
> This is my testing on the patch, it works fine.
> It looks like the issue only happens when __declspec(dllimport) is NOT used when building the exe file.
>

Thanks for the clarification of this issue and playing with this patch.

>
> BTW:
> 1, It looks like I can't set a breakpoint on a function with is __stdcall calling convention
> When I type:
> (gdb) b Add_S
> Function "Add_S" not defined.
> In fact, the symbol name about "Add_S" function is "Add_S@8", (This can be seen from the disassembler)
> I don't know how to set such breakpoint by function names.

I am not an expert on windows, but it is a bug to me.  People, who know 
more about windows/mingw, can give comments here.

>
> 2, I see there are symbol names for Add_C: one is named "Add_C" and the other is "_imp__Add_C",
> so it has "_imp__" before "Add_C". (One underline prefix and double underlines postfix).

Yes, I can see "_imp__Add_C" too in the output of 'maintenance print 
msymbols'.

-- 
Yao (齐尧)

Re: [PATCH] Fix up msymbol type of dll trampoline to mst_solib_trampoline

[Eli Zaretskii]

> Date: Fri, 5 Jul 2013 20:22:18 +0800
> From: Yao Qi <yao@codesourcery.com>
> CC: Tom Tromey <tromey@redhat.com>, <gdb-patches@sourceware.org>
> 
> > BTW:
> > 1, It looks like I can't set a breakpoint on a function with is __stdcall calling convention
> > When I type:
> > (gdb) b Add_S
> > Function "Add_S" not defined.
> > In fact, the symbol name about "Add_S" function is "Add_S@8", (This can be seen from the disassembler)
> > I don't know how to set such breakpoint by function names.
> 
> I am not an expert on windows, but it is a bug to me.

What exactly do you see as a bug here?

Re: [PATCH] Fix up msymbol type of dll trampoline to mst_solib_trampoline

[Yao Qi]

On 07/05/2013 10:29 PM, Eli Zaretskii wrote:
>> Date: Fri, 5 Jul 2013 20:22:18 +0800
>> From: Yao Qi <yao@codesourcery.com>
>> CC: Tom Tromey <tromey@redhat.com>, <gdb-patches@sourceware.org>
>>
>>> BTW:
>>> 1, It looks like I can't set a breakpoint on a function with is __stdcall calling convention
>>> When I type:
>>> (gdb) b Add_S
>>> Function "Add_S" not defined.
>>> In fact, the symbol name about "Add_S" function is "Add_S@8", (This can be seen from the disassembler)
>>> I don't know how to set such breakpoint by function names.
>>
>> I am not an expert on windows, but it is a bug to me.
>
> What exactly do you see as a bug here?
>

We can't set breakpoint on 'Add_S' in current GDB,

(gdb) b Add_S
Function "Add_S" not defined.
Make breakpoint pending on future shared library load? (y or [n])

IMO, It is expected that 'b Add_S' can set a breakpoint on Add_S.

-- 
Yao (齐尧)

Re: [PATCH] Fix up msymbol type of dll trampoline to mst_solib_trampoline

[Eli Zaretskii]

> Date: Sat, 6 Jul 2013 15:20:35 +0800
> From: Yao Qi <yao@codesourcery.com>
> CC: <asmwarrior@gmail.com>, <tromey@redhat.com>, <gdb-patches@sourceware.org>
> 
> On 07/05/2013 10:29 PM, Eli Zaretskii wrote:
> >> Date: Fri, 5 Jul 2013 20:22:18 +0800
> >> From: Yao Qi <yao@codesourcery.com>
> >> CC: Tom Tromey <tromey@redhat.com>, <gdb-patches@sourceware.org>
> >>
> >>> BTW:
> >>> 1, It looks like I can't set a breakpoint on a function with is __stdcall calling convention
> >>> When I type:
> >>> (gdb) b Add_S
> >>> Function "Add_S" not defined.
> >>> In fact, the symbol name about "Add_S" function is "Add_S@8", (This can be seen from the disassembler)
> >>> I don't know how to set such breakpoint by function names.
> >>
> >> I am not an expert on windows, but it is a bug to me.
> >
> > What exactly do you see as a bug here?
> >
> 
> We can't set breakpoint on 'Add_S' in current GDB,
> 
> (gdb) b Add_S
> Function "Add_S" not defined.
> Make breakpoint pending on future shared library load? (y or [n])
> 
> IMO, It is expected that 'b Add_S' can set a breakpoint on Add_S.

Then GDB should look for Add_S@n symbols, where n is the number of
bytes in the function's arguments.

Re: [PATCH] Fix up msymbol type of dll trampoline to mst_solib_trampoline

[asmwarrior]

On 2013-7-6 15:41, Eli Zaretskii wrote:
>> We can't set breakpoint on 'Add_S' in current GDB,
>> > 
>> > (gdb) b Add_S
>> > Function "Add_S" not defined.
>> > Make breakpoint pending on future shared library load? (y or [n])
>> > 
>> > IMO, It is expected that 'b Add_S' can set a breakpoint on Add_S.
> Then GDB should look for Add_S@n symbols, where n is the number of
> bytes in the function's arguments.
Currently, I think GDB did not have this function implemented.
But if I expilicity write the "b Add_S@8", then I can set the breakpoint correctly, see the log below:
---------------------------------------------------------------------
E:\code\cb\test_code\learndll>gdb main.exe
GNU gdb (GDB) 7.6.50.20130705-cvs
Copyright (C) 2013 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "mingw32".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>...
Reading symbols from E:\code\cb\test_code\learndll\main.exe...done.
(gdb) b main.c:18
Breakpoint 1 at 0x4016f6: file main.c, line 18.
(gdb) r
Starting program: E:\code\cb\test_code\learndll\main.exe
[New Thread 4176.0x1048]

Breakpoint 1, main (argc=1, argv=0x3e3f80) at main.c:18
18        int a = Add_C(1, 2);
(gdb) b Add_S
Function "Add_S" not defined.
Make breakpoint pending on future shared library load? (y or [n]) n
(gdb) b Add_S@8
Breakpoint 2 at 0x70f41710: file add.c, line 10.
(gdb) c
Continuing.
3

Breakpoint 2, Add_S@8 (a=3, b=4) at add.c:10
10        return (a + b);
(gdb)
----------------------------------------------------------------------
Yuanhui Zhang

Re: [PATCH] Fix up msymbol type of dll trampoline to mst_solib_trampoline

[Yao Qi]

On 07/03/2013 08:26 AM, Yao Qi wrote:
> In the updated patch, I use hash table when looking for up minsyms, but
> in a little different way.  After currently constructing minsyms are
> installed to OBJFILE, there is a hash table built, we can use that one,
> instead of maintaining a separate one.  That is, after the hash table
> of minsyms of OBJFILE is built up, we can iterate all minsyms, if
> symbol is _imp_x, look up x in the hash table.  If found, modify the
> found's type.
>
> Regression tested on i686-pc-mingw32.  The following fail is fixed.
>
>   FAIL: gdb.base/solib-symbol.exp: foo in libmd

Ping.  http://sourceware.org/ml/gdb-patches/2013-07/msg00100.html

-- 
Yao (齐尧)

Re: [PATCH] Fix up msymbol type of dll trampoline to mst_solib_trampoline

[Tom Tromey]

>>>>> "Yao" == Yao Qi <yao@codesourcery.com> writes:

Yao> That is, after the hash table of minsyms of OBJFILE is built up, we
Yao> can iterate all minsyms, if symbol is _imp_x, look up x in the hash
Yao> table.  If found, modify the found's type.

Yao> +      for (i = objfile->minimal_symbol_count; i > 0; i--)

This should use ALL_OBJFILE_MSYMBOLS.

I'm mildly concerned that this exposes an implementation detail of the
minsym storage -- namely, it assumes that it is ok to modify a minsym
after the minsym is installed.

This approach would also block constification of the minsym API.

Neither of these seem like blocking considerations though.  Presumably
bugs arising from the first would be caught in testing; and for the
second, casting away const would be obviously ok.

So, ok with the ALL_OBJFILE_MSYMBOLS change.

Tom

Re: [PATCH] Fix up msymbol type of dll trampoline to mst_solib_trampoline

[Yao Qi]

On 07/11/2013 12:56 AM, Tom Tromey wrote:
> This should use ALL_OBJFILE_MSYMBOLS.
>
> I'm mildly concerned that this exposes an implementation detail of the
> minsym storage -- namely, it assumes that it is ok to modify a minsym
> after the minsym is installed.
>

Right, we had this assumption in this patch.  If MSYMBOL_TYPE is used in 
the computation of hash key, we can't do this after minsym is installed 
(hashtab is set up).

> This approach would also block constification of the minsym API.
>
> Neither of these seem like blocking considerations though.  Presumably
> bugs arising from the first would be caught in testing; and for the
> second, casting away const would be obviously ok.
>
> So, ok with the ALL_OBJFILE_MSYMBOLS change.

Thanks for the review.  Change to use ALL_OBJFILE_MSYMBOLS and 
regression tested again.  The fail is still fixed.

  -FAIL: gdb.base/solib-symbol.exp: foo in libmd
  +PASS: gdb.base/solib-symbol.exp: foo in libmd

Committed.

-- 
Yao (齐尧)

Re: [PATCH] Fix up msymbol type of dll trampoline to mst_solib_trampoline

[Yao Qi]

On 07/18/2013 10:08 AM, Yao Qi wrote:
> 
> Committed.
> 

Forget to attach the patch.  Here is it.

-- 
Yao (齐尧)

gdb:

2013-07-18  Yao Qi  <yao@codesourcery.com>

	* coffread.c (coff_symfile_read): Iterate over minimal symbols,
	if the name is prefixed by "__imp_" or "_imp_", look for minimal
	symbol without prefix.  If found, set its type to
	'mst_solib_trampoline'.
---
 gdb/coffread.c |   29 +++++++++++++++++++++++++++++
 1 files changed, 29 insertions(+), 0 deletions(-)

diff --git a/gdb/coffread.c b/gdb/coffread.c
index bf39085..1402247 100644
--- a/gdb/coffread.c
+++ b/gdb/coffread.c
@@ -650,6 +650,35 @@ coff_symfile_read (struct objfile *objfile, int symfile_flags)
 
   install_minimal_symbols (objfile);
 
+  if (pe_file)
+    {
+      struct minimal_symbol *msym;
+
+      ALL_OBJFILE_MSYMBOLS (objfile, msym)
+	{
+	  const char *name = SYMBOL_LINKAGE_NAME (msym);
+
+	  /* If the minimal symbols whose name are prefixed by "__imp_"
+	     or "_imp_", get rid of the prefix, and search the minimal
+	     symbol in OBJFILE.  Note that 'maintenance print msymbols'
+	     shows that type of these "_imp_XXXX" symbols is mst_data.  */
+	  if (MSYMBOL_TYPE (msym) == mst_data
+	      && (strncmp (name, "__imp_", 6) == 0
+		  || strncmp (name, "_imp_", 5) == 0))
+	    {
+	      const char *name1 = (name[1] == '_' ? &name[7] : &name[6]);
+	      struct minimal_symbol *found;
+
+	      found = lookup_minimal_symbol (name1, NULL, objfile);
+	      /* If found, there are symbols named "_imp_foo" and "foo"
+		 respectively in OBJFILE.  Set the type of symbol "foo"
+		 as 'mst_solib_trampoline'.  */
+	      if (found != NULL && MSYMBOL_TYPE (found) == mst_text)
+		MSYMBOL_TYPE (found) = mst_solib_trampoline;
+	    }
+	}
+    }
+
   /* Free the installed minimal symbol data.  */
   do_cleanups (cleanup_minimal_symbols);
 
-- 
1.7.7.6