[PATCH] Your patch from 20050512 b0rked on cygwin!

[PATCH] Your patch from 20050512 b0rked on cygwin!

[Dave Korn]

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

[Please keep me in the Cc: line, as I don't subscribe to gdb-patches]

      Hi Mark,

  Your patch to unconditionally include shared library code:

Original patch -
http://sources.redhat.com/ml/gdb-patches/2005-05/msg00043.html
Respin that went in -
http://sources.redhat.com/ml/gdb-patches/2005-05/msg00198.html

... appears to have borked cygwin.  No criticism of yourself should be read
into this fact: you did appeal for cygwin testing, and nobody stepped up to
the plate, so I'm as much to mea culpa as anybody.

  Anyway, the problem is that win32-nat.c implements its own versions of the
solib functions, and since your change, there is now a namespace clash over
solib_address.  I don't know if this is directly because of your patch, or
some indirect knock-on from multi-arch work that has been ongoing; I think
it is probably just because solib.o is in the COMMON_OBS now, but I haven't
actually checked out and built an earlier cvs version to confirm my theory.

  The attached patch fixes the problem, at any rate for me.  However:

a) I haven't done before-and-after testsuite runs yet, because of course
there was no way to build a 'before' version recently.
b) It may or may not impact MinGW.  I don't use MinGW and don't have an
install or setup; I don't even know if MinGW can be a native gdb target,
although I have this vague memory of having read posts here that say it
doesn't currently support gdb, and I couldn't find it mentioned in
src/gdb/config/i386/*.  So this might or might not be an issue.
c) I changed a whole bunch of function names that didn't clash.  I find this
nice and consistent, but others may feel differently about
aesthetically-pleasing but non-essential changes being applied.

  So I would appreciate a bit of review (and preferably from someone who
knows better than me what's going on in this corner of the world!).

2005-08-17  Dave Korn  <dave.korn@artimi.com>

	* win32-nat.c (solib_address): Renamed from this...
	(win32_nat_solib_address): ...to this, to avoid clash with solib.c
	(child_solib_loaded_library_pathname, child_clear_solibs)
	(dll_symbol_command): Likewise renamed to....
	(win32_nat_child_solib_loaded_library_pathname)
	(win32_nat_child_clear_solibs, win32_nat_dll_symbol_command):
...these,
	and all call sites adjusted.

	* config/i386/tm-cygwin.h (SOLIB_ADD, SOLIB_LOADED_LIBRARY_PATHNAME)
	(CLEAR_SOLIB, ADD_SHARED_SYMBOL_FILES, PC_SOLIB):  Added
'win32_nat_'
	prefix to all symbols to match renamed functions in win32-nat.c.

    cheers, 
      DaveK
-- 
Can't think of a witty .sigline today....

[-- Attachment #2: gdb-cygwin-solib-problem-patch.diff --]
[-- Type: application/octet-stream, Size: 4052 bytes --]

Index: gdb/win32-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/win32-nat.c,v
retrieving revision 1.112
diff -p -u -r1.112 win32-nat.c
--- gdb/win32-nat.c	6 Jul 2005 14:54:34 -0000	1.112
+++ gdb/win32-nat.c	17 Aug 2005 15:21:12 -0000
@@ -724,7 +724,7 @@ handle_unload_dll (void *dummy)
 }
 
 char *
-solib_address (CORE_ADDR address)
+win32_nat_solib_address (CORE_ADDR address)
 {
   struct so_stuff *so;
   for (so = &solib_start; so->next != NULL; so = so->next)
@@ -735,14 +735,14 @@ solib_address (CORE_ADDR address)
 
 /* Return name of last loaded DLL. */
 char *
-child_solib_loaded_library_pathname (int pid)
+win32_nat_child_solib_loaded_library_pathname (int pid)
 {
   return !solib_end || !solib_end->name[0] ? NULL : solib_end->name;
 }
 
 /* Clear list of loaded DLLs. */
 void
-child_clear_solibs (void)
+win32_nat_child_clear_solibs (void)
 {
   struct so_stuff *so, *so1 = solib_start.next;
 
@@ -868,7 +868,7 @@ solib_symbols_add (char *name, int from_
 
 /* Load DLL symbol info. */
 void
-dll_symbol_command (char *args, int from_tty)
+win32_nat_dll_symbol_command (char *args, int from_tty)
 {
   int n;
   dont_repeat ();
@@ -1453,7 +1453,7 @@ do_initial_child_stuff (DWORD pid)
   memset (&current_event, 0, sizeof (current_event));
   push_target (&deprecated_child_ops);
   disable_breakpoints_in_shlibs (1);
-  child_clear_solibs ();
+  win32_nat_child_clear_solibs ();
   clear_proceed_status ();
   init_wait_for_inferior ();
 
@@ -2129,7 +2129,7 @@ _initialize_win32_nat (void)
 
   init_child_ops ();
 
-  c = add_com ("dll-symbols", class_files, dll_symbol_command,
+  c = add_com ("dll-symbols", class_files, win32_nat_dll_symbol_command,
 	       _("Load dll library symbols from FILE."));
   set_cmd_completer (c, filename_completer);
 
@@ -2429,14 +2429,14 @@ out:
 }
 
 void
-child_solib_add (char *filename, int from_tty, struct target_ops *target,
+win32_nat_child_solib_add (char *filename, int from_tty, struct target_ops *target,
 		 int readsyms)
 {
   if (!readsyms)
     return;
   if (core_bfd)
     {
-      child_clear_solibs ();
+      win32_nat_child_clear_solibs ();
       bfd_map_over_sections (core_bfd, &core_section_load_dll_symbols, target);
     }
   else
Index: gdb/config/i386/tm-cygwin.h
===================================================================
RCS file: /cvs/src/src/gdb/config/i386/tm-cygwin.h,v
retrieving revision 1.18
diff -p -u -r1.18 tm-cygwin.h
--- gdb/config/i386/tm-cygwin.h	6 Apr 2004 01:53:51 -0000	1.18
+++ gdb/config/i386/tm-cygwin.h	17 Aug 2005 15:21:12 -0000
@@ -21,16 +21,16 @@
    Boston, MA 02111-1307, USA.  */
 
 #define ATTACH_NO_WAIT
-#define SOLIB_ADD(filename, from_tty, targ, readsyms) child_solib_add(filename, from_tty, targ, readsyms)
-#define PC_SOLIB(addr) solib_address (addr)
-#define SOLIB_LOADED_LIBRARY_PATHNAME(pid) child_solib_loaded_library_pathname(pid)
-#define CLEAR_SOLIB child_clear_solibs
-#define ADD_SHARED_SYMBOL_FILES dll_symbol_command
+#define SOLIB_ADD(filename, from_tty, targ, readsyms) win32_nat_child_solib_add(filename, from_tty, targ, readsyms)
+#define PC_SOLIB(addr) win32_nat_solib_address (addr)
+#define SOLIB_LOADED_LIBRARY_PATHNAME(pid) win32_nat_child_solib_loaded_library_pathname(pid)
+#define CLEAR_SOLIB win32_nat_child_clear_solibs
+#define ADD_SHARED_SYMBOL_FILES win32_nat_dll_symbol_command
 
 struct target_ops;
 char *cygwin_pid_to_str (ptid_t ptid);
-void child_solib_add (char *, int, struct target_ops *, int);
-char *solib_address (CORE_ADDR);
-char *child_solib_loaded_library_pathname(int);
-void child_clear_solibs (void);
-void dll_symbol_command (char *, int);
+void win32_nat_child_solib_add (char *, int, struct target_ops *, int);
+char *win32_nat_solib_address (CORE_ADDR);
+char *win32_nat_child_solib_loaded_library_pathname(int);
+void win32_nat_child_clear_solibs (void);
+void win32_nat_dll_symbol_command (char *, int);

Re: [PATCH] Your patch from 20050512 b0rked on cygwin!

[Mark Kettenis]

> From: "Dave Korn" <dave.korn@artimi.com>
> Date: Wed, 17 Aug 2005 17:39:05 +0100
> 
> [Please keep me in the Cc: line, as I don't subscribe to gdb-patches]
> 
>       Hi Mark,
> 
>   Your patch to unconditionally include shared library code:
> 
> Original patch -
> http://sources.redhat.com/ml/gdb-patches/2005-05/msg00043.html
> Respin that went in -
> http://sources.redhat.com/ml/gdb-patches/2005-05/msg00198.html
> 
> ... appears to have borked cygwin.  No criticism of yourself should be read
> into this fact: you did appeal for cygwin testing, and nobody stepped up to
> the plate, so I'm as much to mea culpa as anybody.

Sorry 'bout that.

>   Anyway, the problem is that win32-nat.c implements its own versions of the
> solib functions, and since your change, there is now a namespace clash over
> solib_address.  I don't know if this is directly because of your patch, or
> some indirect knock-on from multi-arch work that has been ongoing; I think
> it is probably just because solib.o is in the COMMON_OBS now, but I haven't
> actually checked out and built an earlier cvs version to confirm my theory.

No need to check that out.  We need to concentrate on getting this
fixed ;-).

>   The attached patch fixes the problem, at any rate for me.  However:
> 
> a) I haven't done before-and-after testsuite runs yet, because of course
> there was no way to build a 'before' version recently.

No problem.

> b) It may or may not impact MinGW.  I don't use MinGW and don't have an
> install or setup; I don't even know if MinGW can be a native gdb target,
> although I have this vague memory of having read posts here that say it
> doesn't currently support gdb, and I couldn't find it mentioned in
> src/gdb/config/i386/*.  So this might or might not be an issue.

Currently you can only use a MinGW gdb created from official sources
to debug remote targets.  So MinGW isn't an issue here.

> c) I changed a whole bunch of function names that didn't clash.  I find this
> nice and consistent, but others may feel differently about
> aesthetically-pleasing but non-essential changes being applied.

I like consistency ;-).

>   So I would appreciate a bit of review (and preferably from someone who
> knows better than me what's going on in this corner of the world!).

Ideally, the cygwin shared library code would be converted to use the
solib.c mechanism.  However, that's a bit more work, so I can image
we'd want this "quick" fix in first.  But Chris Faylor is the
maintainer of this code, so you'll need his approval.

Mark

Re: [PATCH] Your patch from 20050512 b0rked on cygwin!

[Daniel Jacobowitz]

Dave, is Cygwin really still broken?  I never saw a followup to this.

On Wed, Aug 17, 2005 at 09:07:16PM +0200, Mark Kettenis wrote:
> > From: "Dave Korn" <dave.korn@artimi.com>
> > Date: Wed, 17 Aug 2005 17:39:05 +0100
> > 
> > [Please keep me in the Cc: line, as I don't subscribe to gdb-patches]
> > 
> >       Hi Mark,
> > 
> >   Your patch to unconditionally include shared library code:
> > 
> > Original patch -
> > http://sources.redhat.com/ml/gdb-patches/2005-05/msg00043.html
> > Respin that went in -
> > http://sources.redhat.com/ml/gdb-patches/2005-05/msg00198.html
> > 
> > ... appears to have borked cygwin.  No criticism of yourself should be read
> > into this fact: you did appeal for cygwin testing, and nobody stepped up to
> > the plate, so I'm as much to mea culpa as anybody.
> 
> Sorry 'bout that.

[snip; see the archives]

> >   So I would appreciate a bit of review (and preferably from someone who
> > knows better than me what's going on in this corner of the world!).
> 
> Ideally, the cygwin shared library code would be converted to use the
> solib.c mechanism.  However, that's a bit more work, so I can image
> we'd want this "quick" fix in first.  But Chris Faylor is the
> maintainer of this code, so you'll need his approval.

Copying Chris, in case he didn't see the original.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: [PATCH] Your patch from 20050512 b0rked on cygwin!

[Christopher Faylor]

On Sat, Sep 17, 2005 at 09:23:30PM -0400, Daniel Jacobowitz wrote:
>Dave, is Cygwin really still broken?  I never saw a followup to this.

Uh.  Huh?

http://sources.redhat.com/ml/gdb-patches/2005-09/msg00088.html

cgf

Re: [PATCH] Your patch from 20050512 b0rked on cygwin!

[Daniel Jacobowitz]

On Sat, Sep 17, 2005 at 09:31:07PM -0400, Christopher Faylor wrote:
> On Sat, Sep 17, 2005 at 09:23:30PM -0400, Daniel Jacobowitz wrote:
> >Dave, is Cygwin really still broken?  I never saw a followup to this.
> 
> Uh.  Huh?
> 
> http://sources.redhat.com/ml/gdb-patches/2005-09/msg00088.html

Oh, sorry.  This is why patch pings should be threaded with the others;
it sorted out somewhere else in my mailbox.

-- 
Daniel Jacobowitz
CodeSourcery, LLC