Support of gdb for Windows 64 native systems

Support of gdb for Windows 64 native systems

[Kai Tietz]

Hello,

I want to ask for a volunteer to port gdb for the target 
x86_64-pc-mingw32. The support is already available in binutils and gcc, 
but there is currently no gdb (supporting dwarf2) available for this host. 
Is there somebody, who is interested to port it ? Or is willing to help me 
to do this port ?

Thanks in advance,
 i.A. Kai Tietz

|  (\_/)  This is Bunny. Copy and paste Bunny
| (='.'=) into your signature to help him gain
| (")_(") world domination.

------------------------------------------------------------------------------------------
  OneVision Software Entwicklungs GmbH & Co. KG
  Dr.-Leo-Ritter-Straße 9 - 93049 Regensburg
  Tel: +49.(0)941.78004.0 - Fax: +49.(0)941.78004.489 - www.OneVision.com
  Commerzbank Regensburg - BLZ 750 400 62 - Konto 6011050
  Handelsregister: HRA 6744, Amtsgericht Regensburg
  Komplementärin: OneVision Software Entwicklungs Verwaltungs GmbH
  Dr.-Leo-Ritter-Straße 9 – 93049 Regensburg
  Handelsregister: HRB 8932, Amtsgericht Regensburg - Geschäftsführer: 
Ulrike Döhler, Manuela Kluger

Re: Support of gdb for Windows 64 native systems

[Daniel Jacobowitz]

On Thu, Oct 11, 2007 at 03:53:28PM +0200, Kai Tietz wrote:
> Hello,
> 
> I want to ask for a volunteer to port gdb for the target 
> x86_64-pc-mingw32. The support is already available in binutils and gcc, 
> but there is currently no gdb (supporting dwarf2) available for this host. 
> Is there somebody, who is interested to port it ? Or is willing to help me 
> to do this port ?

So far there hasn't even been anyone interested in i686-pc-mingw32 for
the FSF GDB tree; the mingw project has some unsubmitted and
unassigned patches.

-- 
Daniel Jacobowitz
CodeSourcery

Re: Support of gdb for Windows 64 native systems

[Kai Tietz]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="UTF-8", Size: 1776 bytes --]

Daniel Jacobowitz wrote on 11.10.2007 16:05:10:

> On Thu, Oct 11, 2007 at 03:53:28PM +0200, Kai Tietz wrote:
> > Hello,
> > 
> > I want to ask for a volunteer to port gdb for the target 
> > x86_64-pc-mingw32. The support is already available in binutils and 
gcc, 
> > but there is currently no gdb (supporting dwarf2) available for this 
host. 
> > Is there somebody, who is interested to port it ? Or is willing to 
help me 
> > to do this port ?
> 
> So far there hasn't even been anyone interested in i686-pc-mingw32 for
> the FSF GDB tree; the mingw project has some unsubmitted and
> unassigned patches.

Hmm, wouldn't this be a good chance to bring back the mingw32 support to 
the FSF GDB tree ? For sure most of those patches for i?86-mingw are 
needed for 64-bit mingw32 too (especially the file-I/O and shared object 
stuff). But the calling convention for 64-bit is more simple than for the 
32-bit version.
Do you think there is no chance for this support ?

Cheers,
 i.A. Kai Tietz

|  (\_/)  This is Bunny. Copy and paste Bunny
| (='.'=) into your signature to help him gain
| (")_(") world domination.

------------------------------------------------------------------------------------------
  OneVision Software Entwicklungs GmbH & Co. KG
  Dr.-Leo-Ritter-Straße 9 - 93049 Regensburg
  Tel: +49.(0)941.78004.0 - Fax: +49.(0)941.78004.489 - www.OneVision.com
  Commerzbank Regensburg - BLZ 750 400 62 - Konto 6011050
  Handelsregister: HRA 6744, Amtsgericht Regensburg
  Komplementärin: OneVision Software Entwicklungs Verwaltungs GmbH
  Dr.-Leo-Ritter-Straße 9 – 93049 Regensburg
  Handelsregister: HRB 8932, Amtsgericht Regensburg - Geschäftsführer: 
Ulrike Döhler, Manuela Kluger

\x16º&ÖëzÛ«Ÿo|ã¹b²Ö«r\x18\x1d

Re: Support of gdb for Windows 64 native systems

[Kai Tietz]

Daniel Jacobowitz wrote on 11.10.2007 16:23:48:

> On Thu, Oct 11, 2007 at 04:18:40PM +0200, Kai Tietz wrote:
> > Hmm, wouldn't this be a good chance to bring back the mingw32 support 
to 
> > the FSF GDB tree ? For sure most of those patches for i?86-mingw are 
> > needed for 64-bit mingw32 too (especially the file-I/O and shared 
object 
> > stuff). But the calling convention for 64-bit is more simple than for 
the 
> > 32-bit version.
> > Do you think there is no chance for this support ?
> 
> All it needs is someone who is interested to do the work.  And we
> can't start from the MinGW project's patches, because they are not
> assigned to the FSF (I don't know why not but that's what I was told).

Ok, I questioned mingw, why they did not assigned those patches to the 
FSF. But on the otherhand, I would do this work (I need to make an other 
FSF assignement for gdb), if I get some advice. Until now, I haven't 
worked with gdb sources, so the architecture is a bit strange for me. Is 
there a good documentation about the subject how to add an new target 
(TUI) ?

Cheers,
 i.A. Kai Tietz

|  (\_/)  This is Bunny. Copy and paste Bunny
| (='.'=) into your signature to help him gain
| (")_(") world domination.

------------------------------------------------------------------------------------------
  OneVision Software Entwicklungs GmbH & Co. KG
  Dr.-Leo-Ritter-Straße 9 - 93049 Regensburg
  Tel: +49.(0)941.78004.0 - Fax: +49.(0)941.78004.489 - www.OneVision.com
  Commerzbank Regensburg - BLZ 750 400 62 - Konto 6011050
  Handelsregister: HRA 6744, Amtsgericht Regensburg
  Komplementärin: OneVision Software Entwicklungs Verwaltungs GmbH
  Dr.-Leo-Ritter-Straße 9 – 93049 Regensburg
  Handelsregister: HRB 8932, Amtsgericht Regensburg - Geschäftsführer: 
Ulrike Döhler, Manuela Kluger

Re: Support of gdb for Windows 64 native systems

[Joel Brobecker]

> There's documentation in the internals manual.  For a native Windows
> target, you'd have to talk to Chris about how to avoid duplicating
> or complicating the Cygwin support.

Just for the record, AdaCore has a native win32 port of GDB, inspired
mostly from the mingw project. Our patch is kept up to date against
the FSF head too, so it should be relatively easy to submit it.
Except, as Daniel mentions, there are probably some issues relative
to co-existence with cygwin support that need to be addressed. This
is high on my list, so if I wasn't so busy for this end of year, it's
a project I would have already started. (and there is the copyright
assignment issue, but I'm hoping this is solvable)

-- 
Joel

Re: Support of gdb for Windows 64 native systems

[Christopher Faylor]

On Thu, Oct 11, 2007 at 09:00:58AM -0700, Joel Brobecker wrote:
>>There's documentation in the internals manual.  For a native Windows
>>target, you'd have to talk to Chris about how to avoid duplicating or
>>complicating the Cygwin support.
>
>Just for the record, AdaCore has a native win32 port of GDB, inspired
>mostly from the mingw project.  Our patch is kept up to date against
>the FSF head too, so it should be relatively easy to submit it.
>Except, as Daniel mentions, there are probably some issues relative to
>co-existence with cygwin support that need to be addressed.  This is
>high on my list, so if I wasn't so busy for this end of year, it's a
>project I would have already started.  (and there is the copyright
>assignment issue, but I'm hoping this is solvable)

If you're using modified mingw patches then it seems to me like you've
actually compounded the whole issue of copyright assignment.

cgf

Re: Support of gdb for Windows 64 native systems

[Joel Brobecker]

> If you're using modified mingw patches then it seems to me like you've
> actually compounded the whole issue of copyright assignment.

That's completely possible; this was a while ago, and I didn't think
about assignment issues as much as I do today. I'll try to find out
more precisely what needs to be done...

-- 
Joel

Re: Support of gdb for Windows 64 native systems

[Mark Kettenis]

> Date: Thu, 11 Oct 2007 10:55:49 -0400
> From: Daniel Jacobowitz <drow@false.org>
> 
> On Thu, Oct 11, 2007 at 04:47:28PM +0200, Kai Tietz wrote:
> > Ok, I questioned mingw, why they did not assigned those patches to the 
> > FSF. But on the otherhand, I would do this work (I need to make an other 
> > FSF assignement for gdb), if I get some advice. Until now, I haven't 
> > worked with gdb sources, so the architecture is a bit strange for me. Is 
> > there a good documentation about the subject how to add an new target 
> > (TUI) ?
> 
> What does the TUI have to do with it?  TUI in particular is
> troublesome for mingw because of the lack of good curses libraries.

That's easily solved by disabling the TUI on mingw.

Re: Support of gdb for Windows 64 native systems

[Kai Tietz]

Daniel,

> On Thu, Oct 11, 2007 at 04:47:28PM +0200, Kai Tietz wrote:
> > Ok, I questioned mingw, why they did not assigned those patches to the 

> > FSF. But on the otherhand, I would do this work (I need to make an 
other 
> > FSF assignement for gdb), if I get some advice. Until now, I haven't 
> > worked with gdb sources, so the architecture is a bit strange for me. 
Is 
> > there a good documentation about the subject how to add an new target 
> > (TUI) ?
> 
> What does the TUI have to do with it?  TUI in particular is
> troublesome for mingw because of the lack of good curses libraries.
> 
> There's documentation in the internals manual.  For a native Windows
> target, you'd have to talk to Chris about how to avoid duplicating
> or complicating the Cygwin support.

Thank you for your help. I will contact cfg when the paper-work with FSF 
for the gdb assignment is done.

Cheers,
 i.A. Kai Tietz

|  (\_/)  This is Bunny. Copy and paste Bunny
| (='.'=) into your signature to help him gain
| (")_(") world domination.

------------------------------------------------------------------------------------------
  OneVision Software Entwicklungs GmbH & Co. KG
  Dr.-Leo-Ritter-Straße 9 - 93049 Regensburg
  Tel: +49.(0)941.78004.0 - Fax: +49.(0)941.78004.489 - www.OneVision.com
  Commerzbank Regensburg - BLZ 750 400 62 - Konto 6011050
  Handelsregister: HRA 6744, Amtsgericht Regensburg
  Komplementärin: OneVision Software Entwicklungs Verwaltungs GmbH
  Dr.-Leo-Ritter-Straße 9 – 93049 Regensburg
  Handelsregister: HRB 8932, Amtsgericht Regensburg - Geschäftsführer: 
Ulrike Döhler, Manuela Kluger

Re: Support of gdb for Windows 64 native systems

[Eli Zaretskii]

> Date: Thu, 11 Oct 2007 10:55:49 -0400
> From: Daniel Jacobowitz <drow@false.org>
> Cc: gdb@sourceware.org
> 
> For a native Windows target, you'd have to talk to Chris about how
> to avoid duplicating or complicating the Cygwin support.

Actually, wouldn't it be better to separate the two completely?  That
should avoid a lot of ugly ifdefs, and make each native backend much
cleaner, I think.

Re: Support of gdb for Windows 64 native systems

[Joel Brobecker]

> Actually, wouldn't it be better to separate the two completely?  That
> should avoid a lot of ugly ifdefs, and make each native backend much
> cleaner, I think.

The issue with that is that we'll end up duplicating a bit of code.
In our merge, I counted 5 instances of "ifdef/ifndef __MINGW32__
in total, all of them in win32-nat.c:
  - One to define MAXPATHLEN: Should really be done in a proper way,
    so should go away
  - One to avoid the #include <sys/procfs.h>: Should also be done
    using HAVE_SYS_PROCFS_H, so should also go away
  - 3 #ifdef __MINGW32__ in create_inferior to replace unix-style
    IO handling with Windows-style IO handling. Can we call functions
    like GetStdHandle from the cygwin compiler, or do we need the
    MinGW headers for that? Perhaps the mingw code could also
    work with cygwin?

The rest seems to be in i386-win32-tdep.c which is a separate file.

-- 
Joel

Re: Support of gdb for Windows 64 native systems

[Eli Zaretskii]

> Date: Fri, 12 Oct 2007 09:11:32 -0700
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: Daniel Jacobowitz <drow@false.org>, Kai.Tietz@onevision.com,
> 	gdb@sourceware.org
> 
> > Actually, wouldn't it be better to separate the two completely?  That
> > should avoid a lot of ugly ifdefs, and make each native backend much
> > cleaner, I think.
> 
> The issue with that is that we'll end up duplicating a bit of code.

So what? the duplicates will never be linked into the same build.  We
already have duplicate code in targets that are alike, one more cannot
hurt.

In my experience, mixing two different targets is asking for trouble
in the long run.

> In our merge, I counted 5 instances of "ifdef/ifndef __MINGW32__

You need to count "ifdef __CYGWIN__" as well.

> in total, all of them in win32-nat.c:
>   - One to define MAXPATHLEN: Should really be done in a proper way,
>     so should go away

I don't see this one in the current CVS; am I missing something?

> The rest seems to be in i386-win32-tdep.c which is a separate file.

I don't see this file, either, so I cannot comment on that.

Re: Support of gdb for Windows 64 native systems

[Joel Brobecker]

> So what? the duplicates will never be linked into the same build.  We
> already have duplicate code in targets that are alike, one more cannot
> hurt.

I have to disagree on that. We're looking at duplicating 95% of
the code. That means duplicating 95% of the maintenance. I just
had a look at our code and I was surprised to see how little code
is needed now to add support for native win32 support. If it wasn't
for copyright issues, I'm thinking that I could be submitting this
for inclusion with at most a day or two of work!

I'm happy to keep discussing this design aspect, but I think we should
leave that discussion to when someone is ready to contribute something.

> In my experience, mixing two different targets is asking for trouble
> in the long run.

It depends. I somehow we could abstract out the code that handles
IO in a way that it is transparent to the windows nat code, just
the same way we introduced gdb_select, then we can share everything.

> > In our merge, I counted 5 instances of "ifdef/ifndef __MINGW32__
> 
> You need to count "ifdef __CYGWIN__" as well.

Actually, currently all except one such ifdef are used as "if on
windows". They look like this:

   #if defined(_WIN32) || defined(__CYGWIN__)

The only one is in gdbserver, to convert a solib path into a
posix path when using cygwin.

> > in total, all of them in win32-nat.c:
> >   - One to define MAXPATHLEN: Should really be done in a proper way,
> >     so should go away
> 
> I don't see this one in the current CVS; am I missing something?

Yes, this is not necessary on cygwin, and mingw is not supported,
so it's only in our (AdaCore) tree (and probably the mingw tree as
well). It is:

    #ifdef __MINGW32__
      #define MAXPATHLEN PATH_MAX
    #endif

> > The rest seems to be in i386-win32-tdep.c which is a separate file.
> 
> I don't see this file, either, so I cannot comment on that.

Yes, this is normal, because this file is not part of the FSF CVS.
It's a separate file that would only be needed for native win32 support.
And now that I'm having a closer look at it, it looks like this file
is a subset of the cygwin-tdep file, so I should probably experiment
with the idea of replacing the win32-tdep file by the cygwin-tdep one!

It's been a profitable discussion. Thanks a lot!

-- 
Joel

Re: Support of gdb for Windows 64 native systems

[Eli Zaretskii]

> Date: Fri, 12 Oct 2007 10:42:18 -0700
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: drow@false.org, Kai.Tietz@onevision.com, gdb@sourceware.org
> 
> > So what? the duplicates will never be linked into the same build.  We
> > already have duplicate code in targets that are alike, one more cannot
> > hurt.
> 
> I have to disagree on that. We're looking at duplicating 95% of
> the code. That means duplicating 95% of the maintenance.

I assumed the two files will have different maintainers, too.

> > > In our merge, I counted 5 instances of "ifdef/ifndef __MINGW32__
> > 
> > You need to count "ifdef __CYGWIN__" as well.
> 
> Actually, currently all except one such ifdef are used as "if on
> windows". They look like this:
> 
>    #if defined(_WIN32) || defined(__CYGWIN__)

Not in the GDB CVS.

>     #ifdef __MINGW32__
>       #define MAXPATHLEN PATH_MAX
>     #endif

This, for example, is strictly speaking wrong on Windows: Windows
supports much longer file names (up to 32K), if you use Unicode APIs.

> > > The rest seems to be in i386-win32-tdep.c which is a separate file.
> > 
> > I don't see this file, either, so I cannot comment on that.
> 
> Yes, this is normal, because this file is not part of the FSF CVS.
> It's a separate file that would only be needed for native win32 support.

Well, in that case, maybe you already refactored the code in a way
that makes my points moot.

Re: Support of gdb for Windows 64 native systems

[Joel Brobecker]

> > I have to disagree on that. We're looking at duplicating 95% of
> > the code. That means duplicating 95% of the maintenance.
> 
> I assumed the two files will have different maintainers, too.

I would like us to avoid this situation if we can because
we'll waste time keeping the two ports in sync. I'm actually
ready to accept a situation where Chris doesn't have to worry
at all about breaking MingW, and I'll fix the broken pieces as
I discover them.

> > Actually, currently all except one such ifdef are used as "if on
> > windows". They look like this:
> > 
> >    #if defined(_WIN32) || defined(__CYGWIN__)
> 
> Not in the GDB CVS.

Sorry, you're right. But I think my point still stands: Most __CYGWIN__
ifdefs are here to mean "if we are on windows", and the same code
would apply for both ports.

> >     #ifdef __MINGW32__
> >       #define MAXPATHLEN PATH_MAX
> >     #endif
> 
> This, for example, is strictly speaking wrong on Windows: Windows
> supports much longer file names (up to 32K), if you use Unicode APIs.

You are probably correct, I really know very little about Windows.
My point was that this is the wrong way of doing it anyway, so this
too will go, thus reducing a bit more the differences.

> > > > The rest seems to be in i386-win32-tdep.c which is a separate file.
> > > 
> > > I don't see this file, either, so I cannot comment on that.
> > 
> > Yes, this is normal, because this file is not part of the FSF CVS.
> > It's a separate file that would only be needed for native win32 support.
> 
> Well, in that case, maybe you already refactored the code in a way
> that makes my points moot.

I did chop a lot of code off, that's for sure :). That was thanks
to CodeSourcery's work in porting GDB to MinGW as a host. I have
created some action items on my side to investigate how much
I can get rid of... So far today, I'm working on getting my latest
merge to work, and then I'll take a look at this.

-- 
Joel

Re: Support of gdb for Windows 64 native systems

[Joel Brobecker]

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

Hello Eli,

I was able to get rid of i386-win32-tdep.c and use i386-cygwin-tdep.c
instead. So one more difference gone. I fixed the problem with
including <sys/procfs.h> trivially, since we already check for
this include file during the configure phase. The size of the
patch has reduced quite a bit thanks to our discussing this together.

I'm attaching a rough sketch of the changes I think are needed
to adapt the cygwin port to also support mingw. It turns out
that there are more changes that I was claiming. My approach
of serching for ifdefs was a bit over simplistic. Since we have
copyright assignment issues anyway, I haven't tried to be extremely
precise in the patch, as I can't officially submit it for inclusion.
However, this gives a rough idea of the magnitude of the task.

Please ignore the TO_HOST_DIR_SPEC etc changes. These are relics
from the times when we used to provide a cygwin debugger that
pretended to be mingw debugger by outputing/accepting mingw paths.
We should really clean these out of our tree someday...

Back when I worked on this, my approach was to patch only the
files that I thought needed to be patched, rather than importing
the entire mega patch. However, I did tend to apply blindly
the pieces I took, so I don't necessarily know that all the
changes in win32-nat.c are necessary. That's a cleanup left
for later.

I haven't included the configure.tgt change, since it is trivial.
I might be missing some configure.ac changes, I'm not sure.

Here is the mingw.mt makefile fragment:

    # Target: Intel x86 running Win32 (MinGW)
    TDEPFILES= i386-tdep.o i386-cygwin-tdep.o i387-tdep.o solib-target.o
    DEPRECATED_TM_FILE= tm-mingw32.h
    GDBSERVER_DEPFILES=

Unfortunately, as you can see, we're still using a tm file, which
only contains the following definition:

    #define ATTACH_NO_WAIT

It should be pretty easy to work around this. It's strange that
mingw would require it and not cygwin, but I haven't looked deaper
into this. Perhaps cygwin does some magic to fake an event.
It just happens that I had to do the same in my wait loop on
my WTX backend (for VxWorks).

> > >     #ifdef __MINGW32__
> > >       #define MAXPATHLEN PATH_MAX
> > >     #endif
> > 
> > This, for example, is strictly speaking wrong on Windows: Windows
> > supports much longer file names (up to 32K), if you use Unicode APIs.
> 
> You are probably correct, I really know very little about Windows.

This, I still haven't fixed, because I don't know what the proper
thing to do is. What do you think I should use on Windows? I'll
happily fix it.

-- 
Joel

[-- Attachment #2: mingw.diff --]
[-- Type: text/plain, Size: 15359 bytes --]

Index: win32-nat.c
===================================================================
--- win32-nat.c	(.../branches/gdb/FSF/current/gdb)	(revision 15044)
+++ win32-nat.c	(.../trunk/gdb/gdb-head/gdb)	(revision 15044)
@@ -22,8 +22,6 @@
 
 /* Originally by Steve Chamberlain, sac@cygnus.com */
 
-/* We assume we're being built with and will be used for cygwin.  */
-
 #include "defs.h"
 #include "frame.h"		/* required by inferior.h */
 #include "inferior.h"
@@ -40,7 +38,9 @@
 #include <stdlib.h>
 #include <windows.h>
 #include <imagehlp.h>
+#ifdef HAVE_SYS_CYGWIN_H
 #include <sys/cygwin.h>
+#endif
 #include <signal.h>
 
 #include "buildsym.h"
@@ -51,6 +51,9 @@
 #include "gdbthread.h"
 #include "gdbcmd.h"
 #include <sys/param.h>
+#ifdef __MINGW32__
+  #define MAXPATHLEN PATH_MAX
+#endif
 #include <unistd.h>
 #include "exec.h"
 #include "solist.h"
@@ -81,7 +84,9 @@ enum
     CONTEXT_DEBUGGER = (CONTEXT_FULL | CONTEXT_FLOATING_POINT)
   };
 #endif
+#ifdef HAVE_SYS_PROCFS_H
 #include <sys/procfs.h>
+#endif
 #include <psapi.h>
 
 #define CONTEXT_DEBUGGER_DR CONTEXT_DEBUGGER | CONTEXT_DEBUG_REGISTERS \
@@ -105,6 +110,7 @@ static int debug_registers_used;
 
 static void win32_stop (void);
 static int win32_win32_thread_alive (ptid_t);
+static char *win32_pid_to_str (ptid_t ptid);
 static void win32_kill_inferior (void);
 
 static enum target_signal last_sig = TARGET_SIGNAL_0;
@@ -237,10 +243,52 @@ static void
 check (BOOL ok, const char *file, int line)
 {
   if (!ok)
-    printf_filtered ("error return %s:%d was %lu\n", file, line,
-		     GetLastError ());
+  {
+    LPVOID lpMsgBuf;
+    DWORD  last_error = GetLastError ();
+    if (!FormatMessage(FORMAT_MESSAGE_ALLOCATE_BUFFER | 
+                       FORMAT_MESSAGE_FROM_SYSTEM | 
+                       FORMAT_MESSAGE_IGNORE_INSERTS,
+                       NULL,
+                       last_error,
+                       MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT),
+                       (LPTSTR) &lpMsgBuf,
+                       0,
+                       NULL))
+    {
+      printf_filtered ("error return %s:%d was %lu\n", file, line,
+                       last_error);
+    }
+    else
+    {
+      printf_filtered ("error return %s:%d: [%lu] %s\n", file, line,
+                       last_error, (LPCTSTR) lpMsgBuf);
+      LocalFree (lpMsgBuf);
+    }
+  }
 }
 
+#ifndef HAVE_CYGWIN_CONV_TO_POSIX_PATH
+/* A basic replacement implementation for cygwin_conv_to_posix_path ()
+   when not available (basically, when building using a MinGW compiler).  */
+
+static void
+cygwin_conv_to_posix_path (const char *path, char *posix_path)
+{
+  strcpy (posix_path, path);
+}
+#endif
+
+#ifndef HAVE_CYGWIN_CONV_TO_WIN32_PATH
+/* A basic replacement implementation for cygwin_conv_to_win32_path ()
+   when not available (basically, when building using a MinGW compiler).  */
+static void
+cygwin_conv_to_win32_path (const char *path, char *win32_path)
+{
+  strcpy (win32_path, path);
+}
+#endif
+
 /* Find a thread record given a thread id.
    If get_context then also retrieve the context for this
    thread. */
@@ -446,30 +494,22 @@ static BOOL WINAPI (*psapi_EnumProcessMo
 static BOOL WINAPI (*psapi_GetModuleInformation) (HANDLE, HMODULE, LPMODULEINFO, DWORD) = NULL;
 static DWORD WINAPI (*psapi_GetModuleFileNameExA) (HANDLE, HMODULE, LPSTR, DWORD) = NULL;
 
-static int
-psapi_get_dll_name (DWORD BaseAddress, char *dll_name_ret)
+BOOL
+load_psapi ()
 {
-  DWORD len;
-  MODULEINFO mi;
-  int i;
-  HMODULE dh_buf[1];
-  HMODULE *DllHandle = dh_buf;
-  DWORD cbNeeded;
-  BOOL ok;
-
   if (!psapi_loaded ||
       psapi_EnumProcessModules == NULL ||
       psapi_GetModuleInformation == NULL ||
       psapi_GetModuleFileNameExA == NULL)
     {
       if (psapi_loaded)
-	goto failed;
+	return FALSE;
       psapi_loaded = 1;
       psapi_module_handle = LoadLibrary ("psapi.dll");
       if (!psapi_module_handle)
 	{
 	  /* printf_unfiltered ("error loading psapi.dll: %u", GetLastError ()); */
-	  goto failed;
+	  return FALSE;
 	}
       psapi_EnumProcessModules = GetProcAddress (psapi_module_handle, "EnumProcessModules");
       psapi_GetModuleInformation = GetProcAddress (psapi_module_handle, "GetModuleInformation");
@@ -478,9 +518,25 @@ psapi_get_dll_name (DWORD BaseAddress, c
       if (psapi_EnumProcessModules == NULL ||
 	  psapi_GetModuleInformation == NULL ||
 	  psapi_GetModuleFileNameExA == NULL)
-	goto failed;
+	    return FALSE;
     }
+  return TRUE;
+}
 
+int
+psapi_get_dll_name (DWORD BaseAddress, char *dll_name_ret)
+{
+  DWORD len;
+  MODULEINFO mi;
+  int i;
+  HMODULE dh_buf[1];
+  HMODULE *DllHandle = dh_buf;
+  DWORD cbNeeded;
+  BOOL ok;
+
+  if (!load_psapi ())
+    goto failed;
+  
   cbNeeded = 0;
   ok = (*psapi_EnumProcessModules) (current_process_handle,
 				    DllHandle,
@@ -525,6 +581,29 @@ failed:
   return 0;
 }
 
+int
+psapi_get_module_filename (HANDLE hProcess,
+                           HMODULE hModule,
+                           LPTSTR lpFilename,
+                           DWORD nSize)
+{
+  DWORD len;
+
+  if (!load_psapi ())
+    goto failed;
+  
+  len = (*psapi_GetModuleFileNameExA) (hProcess,
+                                       hModule,
+                                       lpFilename,
+                                       nSize);
+  if (len == 0)
+    error ("Error getting file name: %u\n", (unsigned) GetLastError ());
+
+failed:
+  lpFilename[0] = '\0';
+  return 0;
+}
+
 /* Encapsulate the information required in a call to
    symbol_file_add_args */
 struct safe_symbol_file_add_args
@@ -542,6 +621,9 @@ struct safe_symbol_file_add_args
 struct lm_info
 {
   DWORD load_addr;
+
+  /* The ImageBase, aka the prefered load address.  */
+  CORE_ADDR image_base;
 };
 
 static struct so_list solib_start, *solib_end;
@@ -637,6 +719,7 @@ win32_make_so (const char *name, DWORD l
   so = XZALLOC (struct so_list);
   so->lm_info = (struct lm_info *) xmalloc (sizeof (struct lm_info));
   so->lm_info->load_addr = load_addr;
+  so->lm_info->image_base = 0; /* Will be filled in later.  */
   cygwin_conv_to_posix_path (buf, so->so_name);
   strcpy (so->so_original_name, name);
 
@@ -769,7 +852,14 @@ handle_unload_dll (void *dummy)
 	return 1;
       }
 
-  error (_("Error: dll starting at 0x%lx not found."), (DWORD) lpBaseOfDll);
+  /* brobecker/2006-06-15: If we reach that point, that means that
+     the debugger doesn't know of any DLL loaded at this particular
+     address.  The reason for this discrepancy is unknown, but what
+     we have found out is that all the users have found an error
+     message to be confusing, and useless (no real information as
+     to what is happening such as the DLL name for instance can be
+     computed).  In any case, it's just fine to ignore this unload
+     event and continue silently.  */
 
   return 0;
 }
@@ -1227,6 +1317,10 @@ win32_resume (ptid_t ptid, int step, enu
 	       FIXME: should we set dr6 also ?? */
 	      th->context.Dr7 = dr[7];
 	    }
+	  
+	  DEBUG_EVENTS (("gdb: child_resume.SetThreadContext: %s\n",
+			 target_pid_to_str (pid_to_ptid (th->id))));
+  
 	  CHECK (SetThreadContext (th->h, &th->context));
 	  th->context.ContextFlags = 0;
 	}
@@ -1465,6 +1559,9 @@ do_initial_win32_stuff (DWORD pid)
   extern int stop_after_trap;
   int i;
 
+  DEBUG_EVENTS (("gdb: do_initial_child_stuff: %s\n",
+                 target_pid_to_str (pid_to_ptid (pid))));
+  
   last_sig = TARGET_SIGNAL_0;
   event_count = 0;
   exception_count = 0;
@@ -1627,6 +1724,10 @@ win32_attach (char *args, int from_tty)
 
   if (!ok)
     {
+#ifdef __CYGWIN__
+      /* FIXME: This can be implemented better.  Just moved the last
+         if (!ok) block out of this if block, and put this entire
+         if block under #ifdef __CYGWIN__.  */
       /* Try fall back to Cygwin pid */
       pid = cygwin_internal (CW_CYGWIN_PID_TO_WINPID, pid);
 
@@ -1634,6 +1735,7 @@ win32_attach (char *args, int from_tty)
 	ok = DebugActiveProcess (pid);
 
       if (!ok)
+#endif /* __CYGWIN__ */
 	error (_("Can't attach to process."));
     }
 
@@ -1694,13 +1796,14 @@ win32_detach (char *args, int from_tty)
 static char *
 win32_pid_to_exec_file (int pid)
 {
+  static char path[MAX_PATH + 1];
+  char *path_ptr = NULL;
+#ifdef __CYGWIN__
   /* Try to find the process path using the Cygwin internal process list
      pid isn't a valid pid, unfortunately.  Use current_event.dwProcessId
      instead.  */
   /* TODO: Also find native Windows processes using CW_GETPINFO_FULL.  */
 
-  static char path[MAX_PATH + 1];
-  char *path_ptr = NULL;
   int cpid;
   struct external_pinfo *pinfo;
 
@@ -1718,7 +1821,15 @@ win32_pid_to_exec_file (int pid)
        }
     }
   cygwin_internal (CW_UNLOCK_PINFO);
+#else
+  if (!psapi_get_module_filename (current_process_handle, NULL, path, MAX_PATH))
+    printf_unfiltered ("error reading the process's file name: %lu",
+                       GetLastError ());
+  else
+    path_ptr = path;
+#endif
   return path_ptr;
+
 }
 
 /* Print status information about what we're accessing.  */
@@ -1736,6 +1847,15 @@ win32_open (char *arg, int from_tty)
   error (_("Use the \"run\" command to start a Unix child process."));
 }
 
+/* Function called by qsort to sort environment strings.  */
+static int
+env_sort (const void *a, const void *b)
+{
+  const char **p = (const char **) a;
+  const char **q = (const char **) b;
+  return strcasecmp (*p, *q);
+}
+
 /* Start an inferior win32 child process and sets inferior_ptid to its pid.
    EXEC_FILE is the file to run.
    ALLARGS is a string containing the arguments to the program.
@@ -1754,8 +1874,16 @@ win32_create_inferior (char *exec_file, 
   char *toexec;
   char shell[MAX_PATH + 1]; /* Path to shell */
   const char *sh;
+#if defined (__MINGW32__)
+  /* BEGIN: Fragment of Al Stevens's patch for GDB on Win9x */
+  HANDLE hStdInput = 0;
+  HANDLE hStdOutput = 0;
+  HANDLE hStdError = 0;
+  /* END: Fragment of Al Stevens's patch for GDB on Win9x */
+#else /* !__MINGW32__ */
   int tty;
   int ostdin, ostdout, ostderr;
+#endif /* !__MINGW32__ */
   const char *inferior_io_terminal = get_inferior_io_terminal ();
 
   if (!exec_file)
@@ -1799,8 +1927,90 @@ win32_create_inferior (char *exec_file, 
   strcat (args, allargs);
 
   /* Prepare the environment vars for CreateProcess.  */
+#ifdef __CYGWIN__
   cygwin_internal (CW_SYNC_WINENV);
+#else
+ {
+    static const char *conv_path_names[] =
+    {
+      "PATH=",
+      0
+    };
+    int envlen;
+    int i;
+    size_t envsize;
+    char **env;
+    char *winenv;
+    char *temp;
+
+    /* CreateProcess takes the environment list as a null terminated set of
+       strings (i.e. two nulls terminate the list).  */
+
+    /* Get total size for env strings.  */
+    for (envlen = 0, i = 0; in_env[i] && *in_env[i]; i++)
+      {
+	int j, len;
+
+	for (j = 0; conv_path_names[j]; j++)
+	  {
+	    len = strlen (conv_path_names[j]);
+	    if (strncmp (conv_path_names[j], in_env[i], len) == 0)
+	      {
+		envlen += strlen (in_env[i]) + 1;
+		break;
+	      }
+	  }
+	if (conv_path_names[j] == NULL)
+	  envlen += strlen (in_env[i]) + 1;
+      }
+
+    envsize = sizeof (in_env[0]) * (i + 1);
+    env = (char **) alloca (envsize);
+    memcpy (env, in_env, envsize);
+    /* Windows programs expect the environment block to be sorted.  */
+    qsort (env, i, sizeof (char *), env_sort);
+
+    winenv = alloca (envlen + 1);
+
+    /* Copy env strings into new buffer.  */
+    for (temp = winenv, i = 0; env[i] && *env[i]; i++)
+      {
+	int j, len;
+
+	for (j = 0; conv_path_names[j]; j++)
+	  {
+	    len = strlen (conv_path_names[j]);
+	    if (strncmp (conv_path_names[j], env[i], len) == 0)
+	      {
+		strcpy (temp, env[i]);
+		break;
+	      }
+	  }
+	if (conv_path_names[j] == NULL)
+	  strcpy (temp, env[i]);
 
+	temp += strlen (temp) + 1;
+      }
+
+    /* Final nil string to terminate new env.  */
+    *temp = 0;
+ }
+#endif
+
+#if defined (__MINGW32__)
+  /* BEGIN: Fragment of Al Stevens's patch for GDB on Win9x */
+  if ( new_console)
+    {
+      hStdInput = GetStdHandle( STD_INPUT_HANDLE);
+      hStdOutput = GetStdHandle( STD_OUTPUT_HANDLE);
+      hStdError = GetStdHandle( STD_ERROR_HANDLE);
+
+      SetStdHandle( STD_INPUT_HANDLE, INVALID_HANDLE_VALUE);
+      SetStdHandle( STD_OUTPUT_HANDLE, INVALID_HANDLE_VALUE);
+      SetStdHandle( STD_ERROR_HANDLE, INVALID_HANDLE_VALUE);
+    }
+  /* END: Fragment of Al Stevens's patch for GDB on Win9x */
+#else /* !__MINGW32__ */
   if (!inferior_io_terminal)
     tty = ostdin = ostdout = ostderr = -1;
   else
@@ -1821,6 +2031,7 @@ win32_create_inferior (char *exec_file, 
 	  dup2 (tty, 2);
 	}
     }
+#endif /* !__MINGW32__ */
 
   win32_init_thread_list ();
   ret = CreateProcess (0,
@@ -1833,6 +2044,16 @@ win32_create_inferior (char *exec_file, 
 		       NULL,	/* current directory */
 		       &si,
 		       &pi);
+#if defined (__MINGW32__)
+  /* BEGIN: Fragment of Al Stevens's patch for GDB on Win9x */
+  if ( new_console)
+    {
+      SetStdHandle( STD_INPUT_HANDLE, hStdInput);
+      SetStdHandle( STD_OUTPUT_HANDLE, hStdOutput);
+      SetStdHandle( STD_ERROR_HANDLE, hStdError);
+    }
+  /* END: Fragment of Al Stevens's patch for GDB on Win9x */
+#else /* !__MINGW32__ */
   if (tty >= 0)
     {
       close (tty);
@@ -1843,7 +2064,7 @@ win32_create_inferior (char *exec_file, 
       close (ostdout);
       close (ostderr);
     }
-
+#endif /* !__MINGW32__ */
   if (!ret)
     error (_("Error creating process %s, (error %d)."),
 	   exec_file, (unsigned) GetLastError ());
@@ -1947,11 +2168,15 @@ win32_close (int x)
 {
   DEBUG_EVENTS (("gdb: win32_close, inferior_ptid=%d\n",
 		PIDGET (inferior_ptid)));
+  win32_init_thread_list ();
+  disable_breakpoints_in_shlibs ();
+  win32_clear_solib ();
+  clear_proceed_status ();
 }
 
 /* Convert pid to printable format. */
 static char *
-cygwin_pid_to_str (ptid_t ptid)
+win32_pid_to_str (ptid_t ptid)
 {
   static char buf[80];
   int pid = PIDGET (ptid);
@@ -2055,7 +2280,7 @@ init_win32_ops (void)
   win32_ops.to_mourn_inferior = win32_mourn_inferior;
   win32_ops.to_can_run = win32_can_run;
   win32_ops.to_thread_alive = win32_win32_thread_alive;
-  win32_ops.to_pid_to_str = cygwin_pid_to_str;
+  win32_ops.to_pid_to_str = win32_pid_to_str;
   win32_ops.to_stop = win32_stop;
   win32_ops.to_stratum = process_stratum;
   win32_ops.to_has_all_memory = 1;
Index: symfile.c
===================================================================
--- symfile.c	(.../branches/gdb/FSF/current/gdb)	(revision 15044)
+++ symfile.c	(.../trunk/gdb/gdb-head/gdb)	(revision 15044)
@@ -1641,10 +1674,21 @@ symfile_bfd_open (char *name)
     {
       close (desc);
       make_cleanup (xfree, name);
-      error (_("\"%s\": can't open to read symbols: %s."), name,
+      error (_("\"%s\": can't open to read symbols: %s."), 
+	     TO_HOST_FILE_SPEC (name),
 	     bfd_errmsg (bfd_get_error ()));
     }
-  bfd_set_cacheable (sym_bfd, 1);
+
+#ifdef _WIN32
+  /* The executable must not be closed because it will not been possible to
+     reopen it later under Windows NT if this executable is the one being
+     debugged. */
+
+  if (strstr (name, ".exe") != NULL)
+    sym_bfd->cacheable = FALSE;
+  else
+#endif
+    bfd_set_cacheable (sym_bfd, 1);
 
   if (!bfd_check_format (sym_bfd, bfd_object))
     {

Re: Support of gdb for Windows 64 native systems

[Eli Zaretskii]

> Date: Fri, 12 Oct 2007 19:41:16 -0700
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: Kai.Tietz@onevision.com, gdb@sourceware.org
> 
> I'm attaching a rough sketch of the changes I think are needed
> to adapt the cygwin port to also support mingw.

Thanks!

> It turns out that there are more changes that I was claiming.

I'm not surprised, in particular because I've seen the patches applied
by the MinGW folks to produce a native MinGW port.  Personally, I
still think that at this magnitude of ifdef'ing we can certainly
justify two different targets.  But this is not my call, and the
prospects of finally having MinGW support part of the official
repository are so thrilling for me that I'd hate if this argument
would stand in the way.  IOW, let's have MinGW support in the CVS, and
argue about separation later!

What follows are some more or less random comments on the patch; HTH.

> > > >     #ifdef __MINGW32__
> > > >       #define MAXPATHLEN PATH_MAX
> > > >     #endif
> > > 
> > > This, for example, is strictly speaking wrong on Windows: Windows
> > > supports much longer file names (up to 32K), if you use Unicode APIs.
> > 
> > You are probably correct, I really know very little about Windows.
> 
> This, I still haven't fixed, because I don't know what the proper
> thing to do is. What do you think I should use on Windows? I'll
> happily fix it.

Well, I don't think you should do anything with the issue of
supporting longer file names, because I'm quite sure there are lots of
problems with that elsewhere in GDB.  I believe GDB currently can only
support UTF-8 encoded file names, whereas Windows filesystems use
UTF-16.  Adding support for Windows non-ASCII file names to GDB will
need much more work.

However, I'm surprised you needed to define MAXPATHLEN at all, since
sys/param.h in the MinGW headers already does that.  Can you tell why
you need this, and after including sys/param.h on top of that?

>  #include <sys/param.h>
> +#ifdef __MINGW32__
> +  #define MAXPATHLEN PATH_MAX
> +#endif

What am I missing here?  Are you perhaps targeting some old MinGW
versions that didn't have that in sys/param.h?

> +#ifndef HAVE_CYGWIN_CONV_TO_POSIX_PATH
> +/* A basic replacement implementation for cygwin_conv_to_posix_path ()
> +   when not available (basically, when building using a MinGW compiler).  */
> +
> +static void
> +cygwin_conv_to_posix_path (const char *path, char *posix_path)
> +{
> +  strcpy (posix_path, path);
> +}
> +#endif
> +
> +#ifndef HAVE_CYGWIN_CONV_TO_WIN32_PATH
> +/* A basic replacement implementation for cygwin_conv_to_win32_path ()
> +   when not available (basically, when building using a MinGW compiler).  */
> +static void
> +cygwin_conv_to_win32_path (const char *path, char *win32_path)
> +{
> +  strcpy (win32_path, path);
> +}
> +#endif

I hope these will be eventually rewritten to do something useful,
otherwise that's a gratuitous ugliness.

> +#ifdef _WIN32
> +  /* The executable must not be closed because it will not been possible to
> +     reopen it later under Windows NT if this executable is the one being
> +     debugged. */
> +
> +  if (strstr (name, ".exe") != NULL)
> +    sym_bfd->cacheable = FALSE;
> +  else
> +#endif

Doesn't Cygwin define _WIN32?  If it does, why do you need to
condition this fragment?

Re: Support of gdb for Windows 64 native systems

[Joel Brobecker]

> I'm not surprised, in particular because I've seen the patches applied
> by the MinGW folks to produce a native MinGW port.  Personally, I
> still think that at this magnitude of ifdef'ing we can certainly
> justify two different targets.

Yes, it's less clear at this point. Perhaps we can reduce further
the differences. As this thread has shown me, the chopping I have
already done was not nearly enough...

> But this is not my call, and the prospects of finally having MinGW
> support part of the official repository are so thrilling for me that
> I'd hate if this argument would stand in the way.

I'm like you, if keeping them separate is what the group decides,
that's also fine with me. Might be more like Chris' call.

Note however, it's not a done deal yet, because the patches are slightly
tainted by copyright assignment issues. Some of the changes have been
replaced by my work, so you can say they are AdaCore's, but some others
have been imported straight from the MinGW patch. Let's try to reduce it
as much as we can, and then I'll try to track down all the authors of
the pieces that are left.

> >  #include <sys/param.h>
> > +#ifdef __MINGW32__
> > +  #define MAXPATHLEN PATH_MAX
> > +#endif
> 
> What am I missing here?  Are you perhaps targeting some old MinGW
> versions that didn't have that in sys/param.h?

You're right - it turns out this one is not needed anymore.

> > +#ifndef HAVE_CYGWIN_CONV_TO_WIN32_PATH
> > +/* A basic replacement implementation for cygwin_conv_to_win32_path ()
> > +   when not available (basically, when building using a MinGW compiler).  */
> > +static void
> > +cygwin_conv_to_win32_path (const char *path, char *win32_path)
> > +{
> > +  strcpy (win32_path, path);
> > +}
> > +#endif
> 
> I hope these will be eventually rewritten to do something useful,
> otherwise that's a gratuitous ugliness.

The thing is that they don't need to do anything useful.
The PATH is already in win32 format.

> > +#ifdef _WIN32
> > +  /* The executable must not be closed because it will not been possible to
> > +     reopen it later under Windows NT if this executable is the one being
> > +     debugged. */
> > +
> > +  if (strstr (name, ".exe") != NULL)
> > +    sym_bfd->cacheable = FALSE;
> > +  else
> > +#endif
> 
> Doesn't Cygwin define _WIN32?  If it does, why do you need to
> condition this fragment?

I wasn't the author of this change, I think it was Jerome Guitton.
Nonetheless, I think that the intent was to try to avoid unexpected
impact on other platforms. The #ifdef can certainly be removed.

In fact, this change is not necessary at all, it's just a convenience in
the following scenario (IIRC): After having debugged your exe, you found
the bug, so you use the "kill" command to kill the process and, while
leaving the debugger up and running, try to rebuild your program. Because
GDB still has a handle on the EXE, the windows filesystem doesn't let you
delete the file to replace with the new one. So this can be handled as
a separate patch.

I think at this point that the next thing to do is to review carefully
the patch, and evaluate what is needed and what isn't anymore. Then
we can tackle the issue of copyright assignement, and then finally
submit/discuss a new patch.

-- 
Joel

Re: Support of gdb for Windows 64 native systems

[Christopher Faylor]

On Sat, Oct 13, 2007 at 08:47:15AM -0700, Joel Brobecker wrote:
>> I'm not surprised, in particular because I've seen the patches applied
>> by the MinGW folks to produce a native MinGW port.  Personally, I
>> still think that at this magnitude of ifdef'ing we can certainly
>> justify two different targets.
>
>Yes, it's less clear at this point. Perhaps we can reduce further
>the differences. As this thread has shown me, the chopping I have
>already done was not nearly enough...
>
>> But this is not my call, and the prospects of finally having MinGW
>> support part of the official repository are so thrilling for me that
>> I'd hate if this argument would stand in the way.
>
>I'm like you, if keeping them separate is what the group decides,
>that's also fine with me. Might be more like Chris' call.

I'm ok with a limited number of ifdefs since I know that, as you say, it
is likely that 95% of the code will be similar.  It would be a
maintenance nightmare otherwise.

>> > +#ifdef _WIN32
>> > +  /* The executable must not be closed because it will not been possible to
>> > +     reopen it later under Windows NT if this executable is the one being
>> > +     debugged. */
>> > +
>> > +  if (strstr (name, ".exe") != NULL)
>> > +    sym_bfd->cacheable = FALSE;
>> > +  else
>> > +#endif
>> 
>> Doesn't Cygwin define _WIN32?  If it does, why do you need to
>> condition this fragment?
>
>I wasn't the author of this change, I think it was Jerome Guitton.
>Nonetheless, I think that the intent was to try to avoid unexpected
>impact on other platforms. The #ifdef can certainly be removed.

Cygwin doesn't define _WIN32 but I'm wondering why the above isn't
needed for Cygwin, too.

If we do go with the ifdef route, it would probably make sense to define
a WINDOWSISH or a MINGW_OR_CYGWIN variable.  I think there are other parts
of the build which do something like that.

cgf

Re: Support of gdb for Windows 64 native systems

[Pedro Alves]

Joel Brobecker wrote:

> I think at this point that the next thing to do is to review carefully
> the patch, and evaluate what is needed and what isn't anymore. Then
> we can tackle the issue of copyright assignement, and then finally
> submit/discuss a new patch.
> 

That is IMHO, the wrong way to do it.  The copyright assignment
should be resolved first, before everyone looks into the patches.
Otherwise, it taints whoever looks at the thread, and inhibits that
someone from redoing it from scratch, if the copyright issues end up
unresolved doesn't it?

I propose goind the other way around.  Start by adding the needed configury
bits needed, a mingw.mt file, and disabling the cygwin specific bits in
win32-nat.c around #ifdef __CYGWIN__ blocks.  I'd like to keep the
changes mostly in sync with gdbserver/win32-low.c too, but that shouldn't
be a priority.  We can then peacewise enhance the mingw port and bring
its features up.  We can even forget about stdio redirection and
environment passing to the inferior on the first step - that leaves mostly
only path handling to care about - really, look at
gdbserver/win32-low.c (#ifdef USE_WIN32API).

Cheers,
Pedro Alves

Re: Support of gdb for Windows 64 native systems

[Eli Zaretskii]

> Date: Sat, 13 Oct 2007 18:36:28 +0100
> From: Pedro Alves <pedro_alves@portugalmail.pt>
> CC: Eli Zaretskii <eliz@gnu.org>,  Kai.Tietz@onevision.com, 
>  gdb@sourceware.org
> 
> Joel Brobecker wrote:
> 
> > I think at this point that the next thing to do is to review carefully
> > the patch, and evaluate what is needed and what isn't anymore. Then
> > we can tackle the issue of copyright assignement, and then finally
> > submit/discuss a new patch.
> > 
> 
> That is IMHO, the wrong way to do it.  The copyright assignment
> should be resolved first, before everyone looks into the patches.
> Otherwise, it taints whoever looks at the thread, and inhibits that
> someone from redoing it from scratch, if the copyright issues end up
> unresolved doesn't it?

If someone describes the changes, and someone else codes them, there's
no need to resolve the copyright issues.  Copyright only deals with
verbatim copying of the code; ideas cannot be copyrighted.

However, I agree that having everyone look at the patches is not a
very good idea, since it will make it harder not to copy code.

> I propose goind the other way around.  Start by adding the needed configury
> bits needed, a mingw.mt file, and disabling the cygwin specific bits in
> win32-nat.c around #ifdef __CYGWIN__ blocks.  I'd like to keep the
> changes mostly in sync with gdbserver/win32-low.c too, but that shouldn't
> be a priority.  We can then peacewise enhance the mingw port and bring
> its features up.

We don't need to go that slow.

Re: Support of gdb for Windows 64 native systems

[Pedro Alves]

Eli Zaretskii wrote:

>> I propose goind the other way around.  Start by adding the needed configury
>> bits needed, a mingw.mt file, and disabling the cygwin specific bits in
>> win32-nat.c around #ifdef __CYGWIN__ blocks.  I'd like to keep the
>> changes mostly in sync with gdbserver/win32-low.c too, but that shouldn't
>> be a priority.  We can then peacewise enhance the mingw port and bring
>> its features up.
> 
> We don't need to go that slow.
> 

Sure, but it's the natural way to introduce the new target.  I'm not saying
it should be done slowly timewise.  I'm saying it should start from the
bottom up patchwise.  It can be submitted in a patch series in the
same minute for all I care.  The point is, we shouldn't be looking at
a patch and deciding what we *don't* need; we should be looking at
what we have, and deciding what we *do* need.

-- 
Cheers,
Pedro Alves

Re: Support of gdb for Windows 64 native systems

[Joel Brobecker]

> I propose goind the other way around.  Start by adding the needed configury
> bits needed, a mingw.mt file, and disabling the cygwin specific bits in
> win32-nat.c around #ifdef __CYGWIN__ blocks.

That's mostly how I did it, except for the win32-nat.c changes.
I like your suggestions. I think we can get something minimal
pretty quickly. Just to avoid duplicating efforts, who's in charge?
:-)

-- 
Joel

Re: Support of gdb for Windows 64 native systems

[Pedro Alves]

Joel Brobecker wrote:
>> I propose goind the other way around.  Start by adding the needed configury
>> bits needed, a mingw.mt file, and disabling the cygwin specific bits in
>> win32-nat.c around #ifdef __CYGWIN__ blocks.
> 
> That's mostly how I did it, except for the win32-nat.c changes.
> I like your suggestions. I think we can get something minimal
> pretty quickly. Just to avoid duplicating efforts, who's in charge?
> :-)
> 

Not sure who you're asking that, but, I'm not doing or going to do
anything, since you're already about to :)

Cheers,
Pedro Alves

Re: Support of gdb for Windows 64 native systems

[Joel Brobecker]

> Not sure who you're asking that, but, I'm not doing or going to do
> anything, since you're already about to :)

OK :). I don't have much time these days, and in fact, I wasn't
really supposed to dedicate the 4+ hours I have already spent on
this, but since we seem to be on a roll, I'll try to spend some
time on it next week to start the process and get us a minimal
MinGW debugger.

-- 
Joel

Re: Support of gdb for Windows 64 native systems

[Pedro Alves]

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

Joel Brobecker wrote:
>> Not sure who you're asking that, but, I'm not doing or going to do
>> anything, since you're already about to :)
> 
> OK :). I don't have much time these days, and in fact, I wasn't
> really supposed to dedicate the 4+ hours I have already spent on
> this, but since we seem to be on a roll, I'll try to spend some
> time on it next week to start the process and get us a minimal
> MinGW debugger.
> 

Nah, nah, na-na-naaah.  I've done it, and I said wouldn't  :)

Seriously, and rethinking a bit after sleep, it is safer to do
it from scratch, so the copyright issues are cleared.

The initial changes are pretty mechanic, so, here goes.

What's done:

	- Added mingw.mh, mingw.mt as copies from the cygwin files.
	- Updated configure.tgt to set gdb_target.
	- Disabled cygwin specific functionality in win32-nat.c.  As can
	be seen, the changes are very small.
	- profs.h isn't needed anymore, since I've implemented cygwin cross-core 
support a few weeks ago (in i386-cygwin-tdep.c).  The include can be
	unconditionally removed.

What's next:

	Test with a mingw built libexpat to test dll support.

	win32_pid_to_exec_file:  Could be done with GetModuleFileNameEx
	from PSAPI and/or with toolhelp.

	Rename a few things, which really aren't cygwin specific, like:
		* cygwin_get_dr6 -> win32_get_dr6
		* i386-cygwin-tdep.h -> i386-win32-tdep.h
		* nm-cygwin.h -> nm-cygming.h

What's disabled:
	Use a shell to start the subprocess.

I'm sure there are other problems, but it is useable already.

cygcheck.exe ./gdb.exe
.\gdb.exe
   C:\WINDOWS\system32\KERNEL32.dll
     C:\WINDOWS\system32\ntdll.dll
   C:\WINDOWS\system32\msvcrt.dll
   C:\WINDOWS\system32\USER32.dll
     C:\WINDOWS\system32\GDI32.dll
   C:\WINDOWS\system32\WS2_32.DLL
     C:\WINDOWS\system32\WS2HELP.dll
       C:\WINDOWS\system32\ADVAPI32.dll
         C:\WINDOWS\system32\RPCRT4.dll
           C:\WINDOWS\system32\Secur32.dll

build-gdb_mingw\gdb>gdb ./gdb.exe
GNU gdb 6.7.50-20071012-cvs
Copyright (C) 2007 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"...
Setting up the environment for debugging gdb.
During symbol reading, struct/union type gets multiply defined: struct type.
Breakpoint 1 at 0x40b513: file ../../gdb-server_submit/src/gdb/utils.c, line 817
.
Breakpoint 2 at 0x4185c6: file ../../gdb-server_submit/src/gdb/cli/cli-cmds.c, l
ine 199.
(top-gdb) start
Breakpoint 3 at 0x401309: file ../../gdb-server_submit/src/gdb/gdb.c, line 28.
Starting program: D:\cegccsf\cegcc\cegcc\src\build-gdb_mingw\gdb/./gdb.exe
warning: Can not parse XML library list; XML support was disabled at compile tim
e
main (argc=1, argv=0x3d24f0) at ../../gdb-server_submit/src/gdb/gdb.c:28
28        memset (&args, 0, sizeof args);

Cheers,
Pedro Alves

[-- Attachment #2: mingw.diff --]
[-- Type: text/x-diff, Size: 7187 bytes --]

2007-10-14  Pedro Alves  <pedro_alves@portugalmail.pt>

	* config/i386/mingw.mh, config/i386/mingw.mt: New files.
	* configure.tgt	(i[34567]86-*-mingw32*): Set gdb_target = mingw.
	* win32-nat.c: Only include cygwin.h on Cygwin host.  Don't
	include procfs.h.
	(win32_make_so): Don't convert path to posix on MinGW.
	(win32_attach): Only fallback to Cygwin pids on Cygwin.
	(win32_pid_to_exec_file): Disable Cygwin specific code.
	(win32_create_inferior): Disable handle starting the inferior
	through a shell on MinGW.  Disable environment var processing.
	Disable tty handling.
	(cygwin_pid_to_str): Rename to ...
	(win32_pid_to_str): ... this.
	(init_win32_ops): Update use of win32_pid_to_str.  Disable "shell"
	command on MinGW.

---
 gdb/config/i386/mingw.mh |    4 ++++
 gdb/config/i386/mingw.mt |    3 +++
 gdb/configure.tgt        |    2 +-
 gdb/win32-nat.c          |   44 ++++++++++++++++++++++++++++++++++----------
 4 files changed, 42 insertions(+), 11 deletions(-)

Index: src/gdb/config/i386/mingw.mh
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ src/gdb/config/i386/mingw.mh	2007-10-14 12:19:44.000000000 +0100
@@ -0,0 +1,4 @@
+MH_CFLAGS=
+NATDEPFILES= i386-nat.o win32-nat.o
+NAT_FILE=nm-cygwin.h
+XM_CLIBS=
Index: src/gdb/config/i386/mingw.mt
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ src/gdb/config/i386/mingw.mt	2007-10-14 10:36:20.000000000 +0100
@@ -0,0 +1,3 @@
+# Target: Intel 386 run win32
+TDEPFILES= i386-tdep.o i386-cygwin-tdep.o i387-tdep.o \
+	   solib-target.o corelow.o
Index: src/gdb/configure.tgt
===================================================================
--- src.orig/gdb/configure.tgt	2007-10-12 20:25:20.000000000 +0100
+++ src/gdb/configure.tgt	2007-10-14 11:19:50.000000000 +0100
@@ -120,7 +120,7 @@ i[34567]86-*-gnu*)	gdb_target=i386gnu ;;
 i[34567]86-*-cygwin*)	gdb_target=cygwin
 			build_gdbserver=yes
 			;;
-i[34567]86-*-mingw32*)	gdb_target=i386
+i[34567]86-*-mingw32*)	gdb_target=mingw
 			build_gdbserver=yes
 			;;
 i[34567]86-*-*)		gdb_target=i386 ;;
Index: src/gdb/win32-nat.c
===================================================================
--- src.orig/gdb/win32-nat.c	2007-10-13 02:27:04.000000000 +0100
+++ src/gdb/win32-nat.c	2007-10-14 12:23:22.000000000 +0100
@@ -22,8 +22,6 @@
 
 /* Originally by Steve Chamberlain, sac@cygnus.com */
 
-/* We assume we're being built with and will be used for cygwin.  */
-
 #include "defs.h"
 #include "frame.h"		/* required by inferior.h */
 #include "inferior.h"
@@ -40,7 +38,9 @@
 #include <stdlib.h>
 #include <windows.h>
 #include <imagehlp.h>
+#ifdef __CYGWIN__
 #include <sys/cygwin.h>
+#endif
 #include <signal.h>
 
 #include "buildsym.h"
@@ -81,7 +81,6 @@ enum
     CONTEXT_DEBUGGER = (CONTEXT_FULL | CONTEXT_FLOATING_POINT)
   };
 #endif
-#include <sys/procfs.h>
 #include <psapi.h>
 
 #define CONTEXT_DEBUGGER_DR CONTEXT_DEBUGGER | CONTEXT_DEBUG_REGISTERS \
@@ -637,7 +636,11 @@ win32_make_so (const char *name, DWORD l
   so = XZALLOC (struct so_list);
   so->lm_info = (struct lm_info *) xmalloc (sizeof (struct lm_info));
   so->lm_info->load_addr = load_addr;
+#ifdef __CYGWIN__
   cygwin_conv_to_posix_path (buf, so->so_name);
+#else
+  strcpy (so->so_name, buf);
+#endif
   strcpy (so->so_original_name, name);
 
   /* Record cygwin1.dll .text start/end.  */
@@ -1625,6 +1628,7 @@ win32_attach (char *args, int from_tty)
   ok = DebugActiveProcess (pid);
   saw_create = 0;
 
+#ifdef __CYGWIN__
   if (!ok)
     {
       /* Try fall back to Cygwin pid */
@@ -1632,10 +1636,11 @@ win32_attach (char *args, int from_tty)
 
       if (pid > 0)
 	ok = DebugActiveProcess (pid);
+  }
+#endif
 
-      if (!ok)
-	error (_("Can't attach to process."));
-    }
+  if (!ok)
+    error (_("Can't attach to process."));
 
   if (has_detach_ability ())
     DebugSetProcessKillOnExit (FALSE);
@@ -1697,10 +1702,12 @@ win32_pid_to_exec_file (int pid)
   /* Try to find the process path using the Cygwin internal process list
      pid isn't a valid pid, unfortunately.  Use current_event.dwProcessId
      instead.  */
-  /* TODO: Also find native Windows processes using CW_GETPINFO_FULL.  */
 
   static char path[MAX_PATH + 1];
   char *path_ptr = NULL;
+
+#ifdef __CYGWIN__
+  /* TODO: Also find native Windows processes using CW_GETPINFO_FULL.  */
   int cpid;
   struct external_pinfo *pinfo;
 
@@ -1718,6 +1725,10 @@ win32_pid_to_exec_file (int pid)
        }
     }
   cygwin_internal (CW_UNLOCK_PINFO);
+  if (path_ptr)
+    return path_ptr;
+#endif
+
   return path_ptr;
 }
 
@@ -1764,9 +1775,12 @@ win32_create_inferior (char *exec_file, 
   memset (&si, 0, sizeof (si));
   si.cb = sizeof (si);
 
+  flags = DEBUG_ONLY_THIS_PROCESS;
+  toexec = exec_file;
+
+#ifdef __CYGWIN__
   if (!useshell)
     {
-      flags = DEBUG_ONLY_THIS_PROCESS;
       cygwin_conv_to_win32_path (exec_file, real_path);
       toexec = real_path;
     }
@@ -1784,6 +1798,7 @@ win32_create_inferior (char *exec_file, 
       toexec = shell;
       flags = DEBUG_PROCESS;
     }
+#endif
 
   if (new_group)
     flags |= CREATE_NEW_PROCESS_GROUP;
@@ -1798,6 +1813,8 @@ win32_create_inferior (char *exec_file, 
   strcat (args, " ");
   strcat (args, allargs);
 
+#ifdef __CYGWIN__
+
   /* Prepare the environment vars for CreateProcess.  */
   cygwin_internal (CW_SYNC_WINENV);
 
@@ -1822,6 +1839,8 @@ win32_create_inferior (char *exec_file, 
 	}
     }
 
+#endif
+
   win32_init_thread_list ();
   ret = CreateProcess (0,
 		       args,	/* command line */
@@ -1833,6 +1852,8 @@ win32_create_inferior (char *exec_file, 
 		       NULL,	/* current directory */
 		       &si,
 		       &pi);
+
+#ifdef __CYGWIN__
   if (tty >= 0)
     {
       close (tty);
@@ -1843,6 +1864,7 @@ win32_create_inferior (char *exec_file, 
       close (ostdout);
       close (ostderr);
     }
+#endif
 
   if (!ret)
     error (_("Error creating process %s, (error %d)."),
@@ -1951,7 +1973,7 @@ win32_close (int x)
 
 /* Convert pid to printable format. */
 static char *
-cygwin_pid_to_str (ptid_t ptid)
+win32_pid_to_str (ptid_t ptid)
 {
   static char buf[80];
   int pid = PIDGET (ptid);
@@ -2055,7 +2077,7 @@ init_win32_ops (void)
   win32_ops.to_mourn_inferior = win32_mourn_inferior;
   win32_ops.to_can_run = win32_can_run;
   win32_ops.to_thread_alive = win32_win32_thread_alive;
-  win32_ops.to_pid_to_str = cygwin_pid_to_str;
+  win32_ops.to_pid_to_str = win32_pid_to_str;
   win32_ops.to_stop = win32_stop;
   win32_ops.to_stratum = process_stratum;
   win32_ops.to_has_all_memory = 1;
@@ -2086,12 +2108,14 @@ _initialize_win32_nat (void)
 
   add_com_alias ("sharedlibrary", "dll-symbols", class_alias, 1);
 
+#ifdef __CYGWIN__
   add_setshow_boolean_cmd ("shell", class_support, &useshell, _("\
 Set use of shell to start subprocess."), _("\
 Show use of shell to start subprocess."), NULL,
 			   NULL,
 			   NULL, /* FIXME: i18n: */
 			   &setlist, &showlist);
+#endif
 
   add_setshow_boolean_cmd ("cygwin-exceptions", class_support, &cygwin_exceptions, _("\
 Break when an exception is detected in the Cygwin DLL itself."), _("\

Re: Support of gdb for Windows 64 native systems

[Eli Zaretskii]

> Date: Sun, 14 Oct 2007 12:43:01 +0100
> From: Pedro Alves <pedro_alves@portugalmail.pt>
> CC: Eli Zaretskii <eliz@gnu.org>,  Kai.Tietz@onevision.com, 
>  gdb@sourceware.org
> 
> Nah, nah, na-na-naaah.  I've done it, and I said wouldn't  :)

Thanks!

> What's disabled:
> 	Use a shell to start the subprocess.

This will lose support for redirection.  Perhaps we could later add
something similar to what go32-nat.c did for DJGPP, to get that back.

Re: Support of gdb for Windows 64 native systems

[Pedro Alves]

Thinking only of the target part of the story, and assuming the
debug API is similar, the x64/i386 support in GDB
could be done similarly to how x86/arm is split in gdbserver.

It would probably be much easier to start with porting gdbserver
and cross-debugging.  gdbserver already supports x86 mingw.

Take a look at win32-low.c, win32-i386-low.c and win32-arm-low.c
in gdb/gdbserver/.  On the gdb side, you would need at
least a gdb/x64-(mingw|win32)-tdep.c and/or a i386-(mingw|win32)-tdep.c.

Cheers,
Pedro Alves