[RFA] nameless LOAD_DLL_DEBUG_EVENT causes ntdll.dll to be missing

[RFA] nameless LOAD_DLL_DEBUG_EVENT causes ntdll.dll to be missing

[Joel Brobecker]

Hello,

We observed on Windows 2012 that we were unable to unwind past
exception handlers. For instance, with any Ada program raising
an exception that does not get handled:

    % gnatmake -g a -bargs -shared
    % gdb a
    (gdb) start
    (gdb) catch exception unhandled
    Catchpoint 2: unhandled Ada exceptions
    (gdb) c
    Catchpoint 2, unhandled CONSTRAINT_ERROR at <__gnat_unhandled_exception> (
        e=0x645ff820 <constraint_error>) at s-excdeb.adb:53
    53      s-excdeb.adb: No such file or directory.

At this point, we can already see that something went wrong, since
the frame selected by the debugger corresponds to a runtime function
rather than the function in the user code that caused the exception
to be raised (in our case procedure A).

This is further confirmed by the fact that we are unable to unwind
all the way to procedure A:

    (gdb) bt
    #0  <__gnat_unhandled_exception> (e=0x645ff820 <constraint_error>)
        at s-excdeb.adb:53
    #1  0x000000006444e9a3 in <__gnat_notify_unhandled_exception> (excep=0x284d20)
        at a-exextr.adb:144
    #2  0x00000000645f106a in __gnat_personality_imp ()
       from C:\[...]\libgnat-7.3.dll
    #3  0x000000006144d1b7 in _GCC_specific_handler (ms_exc=0x242fab0,
        this_frame=0x242fe60, ms_orig_context=0x242f5c0, ms_disp=0x242ef70,
        gcc_per=0x645f0960 <__gnat_personality_imp>)
        at ../../../src/libgcc/unwind-seh.c:289
    #4  0x00000000645f1211 in __gnat_personality_seh0 ()
       from C:\[...]\libgnat-7.3.dll
    #5  0x000007fad3879f4d in ?? ()
    Backtrace stopped: previous frame inner to this frame (corrupt stack?)

It turns out that the unwinder has been doing its job flawlessly
up until frame #5. The address in frame #5 is correct, but GDB
is not able to associate it with any symbol or unwind record.

And this is because this address is inside ntdll.dll, and when
we received the LOAD_DLL_DEBUG_EVENT for that DLL, the system
was not able to tell us the name of the library, thus causing us
to silently ignoring the event. Because GDB does not know about
ntdll.dll, it is unable to access the unwind information from it.
And because the function at that address does not use a frame
pointer, the unwinding becomes impossible.

This patch helps recovering ntdll.dll at the end of the "run/attach"
phase, simply by trying to locate that specific DLL again.

In terms of our medium to long term planning, it seems to me that
we should be able to simplify the code by ignoring LOAD_DLL_DEBUG_EVENT
during the startup phase, and modify windows_ensure_ntdll_loaded
to then detect and report all shared libraries after we've finished
inferior creation.  But for a change just before 7.7 branch creation,
I thought it was safest to just handle ntdll.dll specifically. This
is less intrusive, and ntdll is the only DLL affected by the problem
I know so far.

gdb/ChangeLog:

	* windows-nat.c (windows_ensure_ntdll_loaded): New function.
	(do_initial_windows_stuff): Use windows_ensure_ntdll_loaded.

Tested on all versions of Windows at AdaCore (XP all the way to 2012),
both x86 and x86_64, but using the AdaCore gdb-testsuite. No regression.

OK to commit?

Thanks,
-- 
Joel

---
 gdb/windows-nat.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)

diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index e32c701..be5a132 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -1703,6 +1703,64 @@ windows_wait (struct target_ops *ops,
     }
 }
 
+/* On certain versions of Windows, the information about ntdll.dll
+   is not available yet at the time we get the LOAD_DLL_DEBUG_EVENT,
+   thus preventing us from reporting this DLL as an SO. This has been
+   witnessed on Windows 8.1, for instance.  A possible explanation
+   is that ntdll.dll might be mapped before the SO info gets created
+   by the Windows system -- ntdll.dll is the first DLL to be reported
+   via LOAD_DLL_DEBUG_EVENT and other DLLs do not seem to suffer from
+   that problem.
+
+   If we indeed are missing ntdll.dll, this function tries to recover
+   from this issue, after the fact.  Do nothing if we encounter any
+   issue trying to locate that DLL.  */
+
+static void
+windows_ensure_ntdll_loaded (void)
+{
+  struct so_list *so;
+  HMODULE dummy_hmodule;
+  DWORD cb_needed;
+  HMODULE *hmodules;
+  int i;
+
+  for (so = solib_start.next; so != NULL; so = so->next)
+    if (FILENAME_CMP (lbasename (so->so_name), "ntdll.dll") == 0)
+      return;  /* ntdll.dll already loaded, nothing to do.  */
+
+  if (EnumProcessModules (current_process_handle, &dummy_hmodule,
+			  sizeof (HMODULE), &cb_needed) == 0)
+    return;
+
+  if (cb_needed < 1)
+    return;
+
+  hmodules = (HMODULE *) alloca (cb_needed);
+  if (EnumProcessModules (current_process_handle, hmodules,
+			  cb_needed, &cb_needed) == 0)
+    return;
+
+  for (i = 0; i < (int) (cb_needed / sizeof (HMODULE)); i++)
+    {
+      MODULEINFO mi;
+      char dll_name[__PMAX];
+
+      if (GetModuleInformation (current_process_handle, hmodules[i],
+				&mi, sizeof (mi)) == 0)
+	continue;
+      if (GetModuleFileNameEx (current_process_handle, hmodules[i],
+			       dll_name, sizeof (dll_name)) == 0)
+	continue;
+      if (FILENAME_CMP (lbasename (dll_name), "ntdll.dll") == 0)
+	{
+	  solib_end->next = windows_make_so (dll_name, mi.lpBaseOfDll);
+	  solib_end = solib_end->next;
+	  return;
+	}
+    }
+}
+
 static void
 do_initial_windows_stuff (struct target_ops *ops, DWORD pid, int attaching)
 {
@@ -1756,6 +1814,8 @@ do_initial_windows_stuff (struct target_ops *ops, DWORD pid, int attaching)
 	break;
     }
 
+  windows_ensure_ntdll_loaded ();
+
   windows_initialization_done = 1;
   inf->control.stop_soon = NO_STOP_QUIETLY;
   stop_after_trap = 0;
-- 
1.8.1.2

Re: [RFA] nameless LOAD_DLL_DEBUG_EVENT causes ntdll.dll to be missing

[Pedro Alves]

On 12/03/2013 11:29 AM, Joel Brobecker wrote:
> Hello,
> 
> We observed on Windows 2012 that we were unable to unwind past
> exception handlers. For instance, with any Ada program raising
> an exception that does not get handled:
> 
>     % gnatmake -g a -bargs -shared
>     % gdb a
>     (gdb) start
>     (gdb) catch exception unhandled
>     Catchpoint 2: unhandled Ada exceptions
>     (gdb) c
>     Catchpoint 2, unhandled CONSTRAINT_ERROR at <__gnat_unhandled_exception> (
>         e=0x645ff820 <constraint_error>) at s-excdeb.adb:53
>     53      s-excdeb.adb: No such file or directory.
> 
> At this point, we can already see that something went wrong, since
> the frame selected by the debugger corresponds to a runtime function
> rather than the function in the user code that caused the exception
> to be raised (in our case procedure A).
> 
> This is further confirmed by the fact that we are unable to unwind
> all the way to procedure A:
> 
>     (gdb) bt
>     #0  <__gnat_unhandled_exception> (e=0x645ff820 <constraint_error>)
>         at s-excdeb.adb:53
>     #1  0x000000006444e9a3 in <__gnat_notify_unhandled_exception> (excep=0x284d20)
>         at a-exextr.adb:144
>     #2  0x00000000645f106a in __gnat_personality_imp ()
>        from C:\[...]\libgnat-7.3.dll
>     #3  0x000000006144d1b7 in _GCC_specific_handler (ms_exc=0x242fab0,
>         this_frame=0x242fe60, ms_orig_context=0x242f5c0, ms_disp=0x242ef70,
>         gcc_per=0x645f0960 <__gnat_personality_imp>)
>         at ../../../src/libgcc/unwind-seh.c:289
>     #4  0x00000000645f1211 in __gnat_personality_seh0 ()
>        from C:\[...]\libgnat-7.3.dll
>     #5  0x000007fad3879f4d in ?? ()
>     Backtrace stopped: previous frame inner to this frame (corrupt stack?)
> 
> It turns out that the unwinder has been doing its job flawlessly
> up until frame #5. The address in frame #5 is correct, but GDB
> is not able to associate it with any symbol or unwind record.
> 
> And this is because this address is inside ntdll.dll, and when
> we received the LOAD_DLL_DEBUG_EVENT for that DLL, the system
> was not able to tell us the name of the library, thus causing us
> to silently ignoring the event. Because GDB does not know about
> ntdll.dll, it is unable to access the unwind information from it.
> And because the function at that address does not use a frame
> pointer, the unwinding becomes impossible.
> 
> This patch helps recovering ntdll.dll at the end of the "run/attach"
> phase, simply by trying to locate that specific DLL again.

Does this happen only on "attach", or also with "run"?  I ask
because of this:

static char *
get_image_name (HANDLE h, void *address, int unicode)
{
...
  /* Attempt to read the name of the dll that was detected.
     This is documented to work only when actively debugging
     a program.  It will not work for attached processes.  */
  if (address == NULL)
    return NULL;

And it sounds like get_module_name somehow is failing
during the startup phase too?  Must be, because
get_module_name also uses EnumProcessModules.  Can
you share for the archives what exactly in get_module_name
fails (that doesn't fail for other dlls)?

> In terms of our medium to long term planning, it seems to me that
> we should be able to simplify the code by ignoring LOAD_DLL_DEBUG_EVENT
> during the startup phase, and modify windows_ensure_ntdll_loaded
> to then detect and report all shared libraries after we've finished
> inferior creation.  But for a change just before 7.7 branch creation,
> I thought it was safest to just handle ntdll.dll specifically. This
> is less intrusive, and ntdll is the only DLL affected by the problem
> I know so far.

EnumProcessModules used to be exported by psapi.dll in older Windows
versions (I think prior to Windows 7), I don't think we can assume
that API/dll is always around.

 > +  int i;
...
 > +  for (i = 0; i < (int) (cb_needed / sizeof (HMODULE)); i++)

Use size_t, and then you don't need the cast.


Does gdbserver need this too?  I'd almost bet it doesn't,
due to the extra fallback on toolhelp (despite
the 32 in the toolhelp API names, it works on 64-bit):

static void
handle_load_dll (void)
{
  LOAD_DLL_DEBUG_INFO *event = &current_event.u.LoadDll;
  char dll_buf[MAX_PATH + 1];
  char *dll_name = NULL;
  CORE_ADDR load_addr;

  dll_buf[0] = dll_buf[sizeof (dll_buf) - 1] = '\0';

  /* Windows does not report the image name of the dlls in the debug
     event on attaches.  We resort to iterating over the list of
     loaded dlls looking for a match by image base.  */
  if (!psapi_get_dll_name (event->lpBaseOfDll, dll_buf))
    {
      if (!server_waiting)
	/* On some versions of Windows and Windows CE, we can't create
	   toolhelp snapshots while the inferior is stopped in a
	   LOAD_DLL_DEBUG_EVENT due to a dll load, but we can while
	   Windows is reporting the already loaded dlls.  */
	toolhelp_get_dll_name (event->lpBaseOfDll, dll_buf);
    }

-- 
Pedro Alves

Re: [RFA] nameless LOAD_DLL_DEBUG_EVENT causes ntdll.dll to be missing

[Eli Zaretskii]

> Date: Tue, 03 Dec 2013 19:50:51 +0000
> From: Pedro Alves <palves@redhat.com>
> CC: gdb-patches@sourceware.org
> 
> EnumProcessModules used to be exported by psapi.dll in older Windows
> versions (I think prior to Windows 7), I don't think we can assume
> that API/dll is always around.

You need to load psapi.dll on Vista and XP, but you can assume that
DLL to be available.  Only Windows 2000 might be a problem.

Re: [RFA] nameless LOAD_DLL_DEBUG_EVENT causes ntdll.dll to be missing

[Joel Brobecker]

> >     % gnatmake -g a -bargs -shared
> >     % gdb a
> >     (gdb) start
[...]
> Does this happen only on "attach", or also with "run"?  I ask
> because of this:

It only happens on "run". I think I have a pretty good handle on what
is going on, now, thanks to your inquisitive questions.

First, something that was not obvious, and yet gave me an "aha!"
moment. At the time the LOAD_DLL_DEBUG_EVENT happens, it turns
out that the modules-array snapshot returned by EnumProcessModules
does not include the DLL being loaded yet. When I started instrumenting
the code in get_module_name, I looked at the result of cbNeeded by
sizeof (HMODULE), and on the first call (first DLL) it fails, then
return 2, 3, 4, etc, and the base address of the new element at
the end of the array always corresponds to the base address of
the _previous_ LOAD_DLL_DEBUG_EVENT, not the one being processed.
The reason why it did not catch the first time I looked at it is
because the first element at index 0 must be for the main executable.
Certainly the base address corresponds.

With that in mind, we can see that during the inferior startup
phase (when we "run"), calling "get_image_name" is hopeless.
But it is also unnecessary, since, most of the time, we get in
the event an address in inferior memory where the path to the
relevant DLL is stored (event->lpImageName).  All we have to do
is read that address, which we do but only after having tried
the first method of iterating over all process modules.

During "attach", on the other hand, the program has had time
to correctly set the entire array, so we get a full array
each time we process the LOAD_DLL_DEBUG_EVENT, allowing us
resolve the DLL name that way.

Now, back to the original problem, the difference between 2012
and older versions is the fact that, in older versions, the
LOAD_DLL_DEBUG_EVENT data for ntdll.dll provided the event->lpImageName
is set, and we can read the DLL's path that way. But with 2012,
it's null for this DLL. MSDN says it may happen:

    This member is strictly optional. Debuggers must be prepared to
    handle the case where lpImageName is NULL or *lpImageName (in the
    address space of the process being debugged) is NULL

And I just reacted to the following bit:

    ... and it will __NOT__ likely pass an image name for the __first__
    DLL event ...

Sounds familiar?

So, back to the suggestion I made for the future (post 7.7), I think
we should ignore the LOAD_DLL_DEBUG_EVENT events during
do_initial_windows_stuff, and just rely on EnumProcessModules
at the end of that function, once we're in control of the inferior.
But that's a major change, and given the number of versions of
Windows, multiplied by 2 for 32bits vs 64 bits - this is why I do
not suggest it for now. But I can work on that sometime next year,
to explore how well we would do.

This is modulo the issue of EnumProcessModules' availability.

> EnumProcessModules used to be exported by psapi.dll in older Windows
> versions (I think prior to Windows 7), I don't think we can assume
> that API/dll is always around.

We don't. We load the DLL explicitly in GDB, and then set things up
so that EnumProcessModules points either to the function in that DLL,
or else to bad_EnumProcessModules. If not available, the fallback
function returns a failure, which is correctly handled. In our case,
it will NOT be used, and in older versions of Windows, the issue
should not exist.

>  > +  int i;
> ...
>  > +  for (i = 0; i < (int) (cb_needed / sizeof (HMODULE)); i++)
> 
> Use size_t, and then you don't need the cast.

It's copy/paste of the existing code. I will fix this one, and
create a followup patch for the rest.

> Does gdbserver need this too?  I'd almost bet it doesn't,
> due to the extra fallback on toolhelp (despite
> the 32 in the toolhelp API names, it works on 64-bit):

I hadn't checked, but it looks like we have the very same issue.
I just build gdbserver (we don't use it on that platform), and
after connecting to GDBserver, I don't see "ntdll.dll" in the
output of "info shared" :-(. Even after setting a breakpoint inside
the program and continuing to that breakpoint, new DLLs appear,
but not ntdll.dll.

I think the same long-term treatment would apply to GDBserver,
and solve the problem as a result. For the immediate problem,
I could attempt a fix, but I am not sure how well I could test it.

In the meantime, does the GDB-side fix look OK to commit to you
(modulo the small issue you raised)?

-- 
Joel

Re: [RFA] nameless LOAD_DLL_DEBUG_EVENT causes ntdll.dll to be missing

[Pedro Alves]

Hi Joel,

Thanks for investigating further.

On 12/05/2013 10:54 AM, Joel Brobecker wrote:
>>>     % gnatmake -g a -bargs -shared
>>>     % gdb a
>>>     (gdb) start
> [...]
>> Does this happen only on "attach", or also with "run"?  I ask
>> because of this:
> 
> It only happens on "run". I think I have a pretty good handle on what
> is going on, now, thanks to your inquisitive questions.
> 
> First, something that was not obvious, and yet gave me an "aha!"
> moment. At the time the LOAD_DLL_DEBUG_EVENT happens, it turns
> out that the modules-array snapshot returned by EnumProcessModules
> does not include the DLL being loaded yet. When I started instrumenting
> the code in get_module_name, I looked at the result of cbNeeded by
> sizeof (HMODULE), and on the first call (first DLL) it fails, then
> return 2, 3, 4, etc, and the base address of the new element at
> the end of the array always corresponds to the base address of
> the _previous_ LOAD_DLL_DEBUG_EVENT, not the one being processed.
> The reason why it did not catch the first time I looked at it is
> because the first element at index 0 must be for the main executable.
> Certainly the base address corresponds.
> 
> With that in mind, we can see that during the inferior startup
> phase (when we "run"), calling "get_image_name" is hopeless.
> But it is also unnecessary, since, most of the time, we get in
> the event an address in inferior memory where the path to the
> relevant DLL is stored (event->lpImageName).  All we have to do
> is read that address, which we do but only after having tried
> the first method of iterating over all process modules.
> 
> During "attach", on the other hand, the program has had time
> to correctly set the entire array, so we get a full array
> each time we process the LOAD_DLL_DEBUG_EVENT, allowing us
> resolve the DLL name that way.
> 
> Now, back to the original problem, the difference between 2012
> and older versions is the fact that, in older versions, the
> LOAD_DLL_DEBUG_EVENT data for ntdll.dll provided the event->lpImageName
> is set, and we can read the DLL's path that way. But with 2012,
> it's null for this DLL. MSDN says it may happen:
> 
>     This member is strictly optional. Debuggers must be prepared to
>     handle the case where lpImageName is NULL or *lpImageName (in the
>     address space of the process being debugged) is NULL
> 
> And I just reacted to the following bit:
> 
>     ... and it will __NOT__ likely pass an image name for the __first__
>     DLL event ...
> 
> Sounds familiar?

:-)  I'd be good to have this new info distilled as a comment
in the code, IMO.

> So, back to the suggestion I made for the future (post 7.7), I think
> we should ignore the LOAD_DLL_DEBUG_EVENT events during
> do_initial_windows_stuff, and just rely on EnumProcessModules
> at the end of that function, once we're in control of the inferior.
> But that's a major change, and given the number of versions of
> Windows, multiplied by 2 for 32bits vs 64 bits - this is why I do
> not suggest it for now. But I can work on that sometime next year,
> to explore how well we would do.
> 
> This is modulo the issue of EnumProcessModules' availability.
> 

>> EnumProcessModules used to be exported by psapi.dll in older Windows
>> versions (I think prior to Windows 7), I don't think we can assume
>> that API/dll is always around.
> 
> We don't. We load the DLL explicitly in GDB, and then set things up
> so that EnumProcessModules points either to the function in that DLL,
> or else to bad_EnumProcessModules.  If not available, the fallback
> function returns a failure, which is correctly handled. In our case,
> it will NOT be used, and in older versions of Windows, the issue
> should not exist.

Right, I was aware of that LoadLibrary trick in use.  I do think
we do still use it on newer Windows, though in those, the functions
moved to kernel32.dll and psapi.dll is just a wrapper.

> I hadn't checked, but it looks like we have the very same issue.
> I just build gdbserver (we don't use it on that platform), and
> after connecting to GDBserver, I don't see "ntdll.dll" in the
> output of "info shared" :-(. Even after setting a breakpoint inside
> the program and continuing to that breakpoint, new DLLs appear,
> but not ntdll.dll.
> 
> I think the same long-term treatment would apply to GDBserver,
> and solve the problem as a result. 

Yes.

> For the immediate problem,
> I could attempt a fix, but I am not sure how well I could test it.

I think the fix is very low risk, and could go in with minimal
testing.

> In the meantime, does the GDB-side fix look OK to commit to you
> (modulo the small issue you raised)?

Yes.

One nit -- does ntdll.dll appears at the end of the
loaded list with this fix, or at the beginning?  I'd think
it should be the first dll in "info sharedlibrary", assuming
that that's what we see on older Windows versions.

-- 
Pedro Alves

Re: [RFA] nameless LOAD_DLL_DEBUG_EVENT causes ntdll.dll to be missing

[Joel Brobecker]

> :-)  I'd be good to have this new info distilled as a comment
> in the code, IMO.

Excellent suggestion, of course. I, with my incessant asking for
comments, should have thought of it too! ;-)

[regarding gdbserver]
> I think the fix is very low risk, and could go in with minimal
> testing.

OK - will work on that ASAP.

> > In the meantime, does the GDB-side fix look OK to commit to you
> > (modulo the small issue you raised)?
> 
> Yes.
> 
> One nit -- does ntdll.dll appears at the end of the
> loaded list with this fix, or at the beginning?  I'd think
> it should be the first dll in "info sharedlibrary", assuming
> that that's what we see on older Windows versions.

I think this is going to be difficult, unless we defer the notification
for the other shared libraries - in which case we're doing what I wanted
to do, but after 7.7 gets cut, which is to ignore LOAD_DLL_DEBUG_EVENTs
until the end of do_initial_windows_stuff, and then just iterate over
all modules to generate the SOs.

The reason why I think it would be difficult is because, when a new
shared object is loaded, we get a TARGET_WAITKIND_LOADED, which
I think eventually leads us to:

  -> handle_solib_event ()
     -> solib_add (NULL, 0, &current_target, auto_solib_add);
        -> update_solib_list (from_tty, target);

And looking at update_solib_list, it calls current_sos, and then
adds new SOs at the end (via the VEC_push). On top of that, an
easier issue that we'll have to handle in windows-nat the adding
of the new SO at the start of the SO list, rather than then end.

Since this is mostly cosmetic and only affects Windows 2012,
I would leave it alone until we have more leeway towards simplifying
the DLL handling.

-- 
Joel

Re: [RFA] nameless LOAD_DLL_DEBUG_EVENT causes ntdll.dll to be missing

[Pedro Alves]

On 12/09/2013 11:33 AM, Joel Brobecker wrote:
> The reason why I think it would be difficult is because, when a new
> shared object is loaded, we get a TARGET_WAITKIND_LOADED, which
> I think eventually leads us to:
> 
>   -> handle_solib_event ()
>      -> solib_add (NULL, 0, &current_target, auto_solib_add);
>         -> update_solib_list (from_tty, target);
> 
> And looking at update_solib_list, it calls current_sos, and then
> adds new SOs at the end (via the VEC_push). 

I see now.  You're right.

> On top of that, an
> easier issue that we'll have to handle in windows-nat the adding
> of the new SO at the start of the SO list, rather than then end.

Yeah, I naively thought that that would be all we'd have
to do.  :-)

> 
> Since this is mostly cosmetic and only affects Windows 2012,
> I would leave it alone until we have more leeway towards simplifying
> the DLL handling.

Agreed.

-- 
Pedro Alves

pushed: [RFA] nameless LOAD_DLL_DEBUG_EVENT causes ntdll.dll to be missing

[Joel Brobecker]

Hello,

Attached is the patch I ended up checking in. It's the same patch,
augmented with additional comments.

gdb/ChangeLog:

        * windows-nat.c (handle_load_dll): Add comments.
        (windows_ensure_ntdll_loaded): New function.
        (do_initial_windows_stuff): Use windows_ensure_ntdll_loaded.
        Add FIXME comment.

-- 
Joel

Re: pushed: [RFA] nameless LOAD_DLL_DEBUG_EVENT causes ntdll.dll to be missing

[Joel Brobecker]

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

> Attached is the patch I ended up checking in. It's the same patch,
> augmented with additional comments.
> 
> gdb/ChangeLog:
> 
>         * windows-nat.c (handle_load_dll): Add comments.
>         (windows_ensure_ntdll_loaded): New function.
>         (do_initial_windows_stuff): Use windows_ensure_ntdll_loaded.
>         Add FIXME comment.

Fixing ENOPATCH...

-- 
Joel

[-- Attachment #2: 0001-nameless-LOAD_DLL_DEBUG_EVENT-causes-ntdll.dll-to-be.patch --]
[-- Type: text/x-diff, Size: 8276 bytes --]

From 94481b8c8f63a2a956d18ac1e6d6134c5334cb24 Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Wed, 20 Nov 2013 12:43:20 -0500
Subject: [PATCH] nameless LOAD_DLL_DEBUG_EVENT causes ntdll.dll to be missing

We observed on Windows 2012 that we were unable to unwind past
exception handlers. For instance, with any Ada program raising
an exception that does not get handled:

    % gnatmake -g a -bargs -shared
    % gdb a
    (gdb) start
    (gdb) catch exception unhandled
    Catchpoint 2: unhandled Ada exceptions
    (gdb) c
    Catchpoint 2, unhandled CONSTRAINT_ERROR at <__gnat_unhandled_exception> (
        e=0x645ff820 <constraint_error>) at s-excdeb.adb:53
    53      s-excdeb.adb: No such file or directory.

At this point, we can already see that something went wrong, since
the frame selected by the debugger corresponds to a runtime function
rather than the function in the user code that caused the exception
to be raised (in our case procedure A).

This is further confirmed by the fact that we are unable to unwind
all the way to procedure A:

    (gdb) bt
    #0  <__gnat_unhandled_exception> (e=0x645ff820 <constraint_error>)
        at s-excdeb.adb:53
    #1  0x000000006444e9a3 in <__gnat_notify_unhandled_exception> (excep=0x284d2
+0)
        at a-exextr.adb:144
    #2  0x00000000645f106a in __gnat_personality_imp ()
       from C:\[...]\libgnat-7.3.dll
    #3  0x000000006144d1b7 in _GCC_specific_handler (ms_exc=0x242fab0,
        this_frame=0x242fe60, ms_orig_context=0x242f5c0, ms_disp=0x242ef70,
        gcc_per=0x645f0960 <__gnat_personality_imp>)
        at ../../../src/libgcc/unwind-seh.c:289
    #4  0x00000000645f1211 in __gnat_personality_seh0 ()
       from C:\[...]\libgnat-7.3.dll
    #5  0x000007fad3879f4d in ?? ()
    Backtrace stopped: previous frame inner to this frame (corrupt stack?)

It turns out that the unwinder has been doing its job flawlessly
up until frame #5. The address in frame #5 is correct, but GDB
is not able to associate it with any symbol or unwind record.

And this is because this address is inside ntdll.dll, and when
we received the LOAD_DLL_DEBUG_EVENT for that DLL, the system
was not able to tell us the name of the library, thus causing us
to silently ignoring the event. Because GDB does not know about
ntdll.dll, it is unable to access the unwind information from it.
And because the function at that address does not use a frame
pointer, the unwinding becomes impossible.

This patch helps recovering ntdll.dll at the end of the "run/attach"
phase, simply by trying to locate that specific DLL again.

In terms of our medium to long term planning, it seems to me that
we should be able to simplify the code by ignoring LOAD_DLL_DEBUG_EVENT
during the startup phase, and modify windows_ensure_ntdll_loaded
to then detect and report all shared libraries after we've finished
inferior creation.  But for a change just before 7.7 branch creation,
I thought it was safest to just handle ntdll.dll specifically. This
is less intrusive, and ntdll is the only DLL affected by the problem
I know so far.

gdb/ChangeLog:

	* windows-nat.c (handle_load_dll): Add comments.
        (windows_ensure_ntdll_loaded): New function.
	(do_initial_windows_stuff): Use windows_ensure_ntdll_loaded.
        Add FIXME comment.
---
 gdb/ChangeLog     |  7 +++++
 gdb/windows-nat.c | 86 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 93 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 788eaab..9401312 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,10 @@
+2013-12-10  Joel Brobecker  <brobecker@adacore.com>
+
+	* windows-nat.c (handle_load_dll): Add comments.
+	(windows_ensure_ntdll_loaded): New function.
+	(do_initial_windows_stuff): Use windows_ensure_ntdll_loaded.
+	Add FIXME comment.
+
 2013-12-08  Joel Brobecker  <brobecker@adacore.com>
 
 	GDB 7.6.2 released.
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index e32c701..f0545fc 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -847,11 +847,31 @@ handle_load_dll (void *dummy)
 
   dll_buf[0] = dll_buf[sizeof (dll_buf) - 1] = '\0';
 
+  /* Try getting the DLL name by searching the list of known modules
+     and matching their base address against this new DLL's base address.
+
+     FIXME: brobecker/2013-12-10:
+     It seems odd to be going through this search if the DLL name could
+     simply be extracted via "event->lpImageName".  Moreover, some
+     experimentation with various versions of Windows seem to indicate
+     that it might still be too early for this DLL to be listed when
+     querying the system about the current list of modules, thus making
+     this attempt pointless.
+
+     This code can therefore probably be removed.  But at the time of
+     this writing, we are too close to creating the GDB 7.7 branch
+     for us to make such a change.  We are therefore defering it.  */
+
   if (!get_module_name (event->lpBaseOfDll, dll_buf))
     dll_buf[0] = dll_buf[sizeof (dll_buf) - 1] = '\0';
 
   dll_name = dll_buf;
 
+  /* Try getting the DLL name via the lpImageName field of the event.
+     Note that Microsoft documents this fields as strictly optional,
+     in the sense that it might be NULL.  And the first DLL event in
+     particular is explicitly documented as "likely not pass[ed]"
+     (source: MSDN LOAD_DLL_DEBUG_INFO structure).  */
   if (*dll_name == '\0')
     dll_name = get_image_name (current_process_handle,
 			       event->lpImageName, event->fUnicode);
@@ -1703,6 +1723,64 @@ windows_wait (struct target_ops *ops,
     }
 }
 
+/* On certain versions of Windows, the information about ntdll.dll
+   is not available yet at the time we get the LOAD_DLL_DEBUG_EVENT,
+   thus preventing us from reporting this DLL as an SO. This has been
+   witnessed on Windows 8.1, for instance.  A possible explanation
+   is that ntdll.dll might be mapped before the SO info gets created
+   by the Windows system -- ntdll.dll is the first DLL to be reported
+   via LOAD_DLL_DEBUG_EVENT and other DLLs do not seem to suffer from
+   that problem.
+
+   If we indeed are missing ntdll.dll, this function tries to recover
+   from this issue, after the fact.  Do nothing if we encounter any
+   issue trying to locate that DLL.  */
+
+static void
+windows_ensure_ntdll_loaded (void)
+{
+  struct so_list *so;
+  HMODULE dummy_hmodule;
+  DWORD cb_needed;
+  HMODULE *hmodules;
+  int i;
+
+  for (so = solib_start.next; so != NULL; so = so->next)
+    if (FILENAME_CMP (lbasename (so->so_name), "ntdll.dll") == 0)
+      return;  /* ntdll.dll already loaded, nothing to do.  */
+
+  if (EnumProcessModules (current_process_handle, &dummy_hmodule,
+			  sizeof (HMODULE), &cb_needed) == 0)
+    return;
+
+  if (cb_needed < 1)
+    return;
+
+  hmodules = (HMODULE *) alloca (cb_needed);
+  if (EnumProcessModules (current_process_handle, hmodules,
+			  cb_needed, &cb_needed) == 0)
+    return;
+
+  for (i = 0; i < (int) (cb_needed / sizeof (HMODULE)); i++)
+    {
+      MODULEINFO mi;
+      char dll_name[__PMAX];
+
+      if (GetModuleInformation (current_process_handle, hmodules[i],
+				&mi, sizeof (mi)) == 0)
+	continue;
+      if (GetModuleFileNameEx (current_process_handle, hmodules[i],
+			       dll_name, sizeof (dll_name)) == 0)
+	continue;
+      if (FILENAME_CMP (lbasename (dll_name), "ntdll.dll") == 0)
+	{
+	  solib_end->next = windows_make_so (dll_name, mi.lpBaseOfDll);
+	  solib_end = solib_end->next;
+	  return;
+	}
+    }
+}
+
 static void
 do_initial_windows_stuff (struct target_ops *ops, DWORD pid, int attaching)
 {
@@ -1756,6 +1834,14 @@ do_initial_windows_stuff (struct target_ops *ops, DWORD pid, int attaching)
 	break;
     }
 
+  /* FIXME: brobecker/2013-12-10: We should try another approach where
+     we first ignore all DLL load/unload events up until this point,
+     and then iterate over all modules to create the associated shared
+     objects.  This is a fairly significant change, however, and we are
+     close to creating a release branch, so we are delaying it a bit,
+     after the branch is created.  */
+  windows_ensure_ntdll_loaded ();
+
   windows_initialization_done = 1;
   inf->control.stop_soon = NO_STOP_QUIETLY;
   stop_after_trap = 0;
-- 
1.8.1.2

Re: [RFA] nameless LOAD_DLL_DEBUG_EVENT causes ntdll.dll to be missing

[Joel Brobecker]

Hi Pedro,

> [regarding gdbserver]
> > I think the fix is very low risk, and could go in with minimal
> > testing.
> 
> OK - will work on that ASAP.

I just had a look, and unfortunately, by the time we get to the end of
do_initial_child_stuff, gdbserver has not done the wait/resume cycle
and so DLLs have not been mapped yet. This is because this part is
currently handled by the generic code, as opposed to the "create_inferior"
or "attach" methods.

It seems to me that, if we want to fix this issue in GDBserver,
we'll need to add a new method, something like inferior_created.
We'd then call this new method, if defined, at the end of both
start_inferior and attach_inferior.

Does this sound right to you?

Thanks,
-- 
Joel

Re: [RFA] nameless LOAD_DLL_DEBUG_EVENT causes ntdll.dll to be missing

[Pedro Alves]

On 12/10/2013 10:56 AM, Joel Brobecker wrote:
> Hi Pedro,
> 
>> [regarding gdbserver]
>>> I think the fix is very low risk, and could go in with minimal
>>> testing.
>>
>> OK - will work on that ASAP.
> 
> I just had a look, and unfortunately, by the time we get to the end of
> do_initial_child_stuff, gdbserver has not done the wait/resume cycle
> and so DLLs have not been mapped yet. This is because this part is
> currently handled by the generic code, as opposed to the "create_inferior"
> or "attach" methods.

Hmm, I had forgotten that.  I always though that gdbserver's
"create inferior" sequence of calling mywait after create_inferior
to be a little odd, leading to this issue (the FIXME):

  signal_pid = create_inferior (new_argv[0], new_argv);

  /* FIXME: we don't actually know at this point that the create
     actually succeeded.  We won't know that until we wait.  */
  fprintf (stderr, "Process %s created; pid = %ld\n", argv[0],
	   signal_pid);

Changing that would mean changing more than we're willing at
the moment.  We can still work in that direction, and actually
make gdbserver's win32 initial event handling more similar to
GDB's.

> It seems to me that, if we want to fix this issue in GDBserver,
> we'll need to add a new method, something like inferior_created.
> We'd then call this new method, if defined, at the end of both
> start_inferior and attach_inferior.
> 
> Does this sound right to you?

What about this alternative below as preparatory for your
patch?  It makes gdbserver closer to GDB here.

(Actually, I've though before of making gdb/windows-nat.c's initial
event handling in do_initial_windows_stuff call windows_wait directly
instead of wait_for_inferior, to get rid of stop_after_trap.
That would also go in the direction of your earlier suggestion
of fetching the dll list after the initial events,
as then the initial LOAD_DLL_DEBUG_EVENT events wouldn't cause
the solib_add calls in infrun.c -- infrun wouldn't see them at
all.)

"gdbserver.exe :9999 gdbserver.exe" under wine, and then
connecting with an --enable-targets=all GNU/Linux GDB works,
but I'm not really able to test beyond that.

2013-12-10  Pedro Alves  <palves@redhat.com>

	* target.c (mywait): Convert TARGET_WAITKIND_LOADED to
	TARGET_WAITKIND_STOPPED.
	* win32-low.c (stopped_at_initial_breakpoint): New global.
	(do_initial_child_stuff): Consume events up to the initial
	breakpoint here.
	(win32_wait): Return the last event if starting up.
	Don't ignore TARGET_WAITKIND_LOADED here.
---

 gdb/gdbserver/target.c    |    5 ++++
 gdb/gdbserver/win32-low.c |   62 ++++++++++++++++++++++++++++++++++-----------
 2 files changed, 52 insertions(+), 15 deletions(-)

diff --git a/gdb/gdbserver/target.c b/gdb/gdbserver/target.c
index d4a2a98..d229933 100644
--- a/gdb/gdbserver/target.c
+++ b/gdb/gdbserver/target.c
@@ -82,6 +82,11 @@ mywait (ptid_t ptid, struct target_waitstatus *ourstatus, int options,

   ret = (*the_target->wait) (ptid, ourstatus, options);

+  /* We don't expose _LOADED events to gdbserver core.  See the
+     `dlls_changed' global.  */
+  if (ourstatus->kind == TARGET_WAITKIND_LOADED)
+    ourstatus->kind = TARGET_WAITKIND_STOPPED;
+
   /* If GDB is connected through TCP/serial, then GDBserver will most
      probably be running on its own terminal/console, so it's nice to
      print there why is GDBserver exiting.  If however, GDB is
diff --git a/gdb/gdbserver/win32-low.c b/gdb/gdbserver/win32-low.c
index 979eedd..17dd5bc 100644
--- a/gdb/gdbserver/win32-low.c
+++ b/gdb/gdbserver/win32-low.c
@@ -80,6 +80,11 @@ static enum gdb_signal last_sig = GDB_SIGNAL_0;
 /* The current debug event from WaitForDebugEvent.  */
 static DEBUG_EVENT current_event;

+/* A status that hasn't been reported to the core yet, and so
+   win32_wait should return it next, instead of fetching the next
+   debug event off the win32 API.  */
+static struct target_waitstatus cached_status;
+
 /* Non zero if an interrupt request is to be satisfied by suspending
    all threads.  */
 static int soft_interrupt_requested = 0;
@@ -97,6 +102,8 @@ typedef BOOL (WINAPI *winapi_DebugSetProcessKillOnExit) (BOOL KillOnExit);
 typedef BOOL (WINAPI *winapi_DebugBreakProcess) (HANDLE);
 typedef BOOL (WINAPI *winapi_GenerateConsoleCtrlEvent) (DWORD, DWORD);

+static ptid_t win32_wait (ptid_t ptid, struct target_waitstatus *ourstatus,
+			  int options);
 static void win32_resume (struct thread_resume *resume_info, size_t n);

 /* Get the thread ID from the current selected inferior (the current
@@ -336,6 +343,34 @@ do_initial_child_stuff (HANDLE proch, DWORD pid, int attached)

   if (the_low_target.initial_stuff != NULL)
     (*the_low_target.initial_stuff) ();
+
+  cached_status.kind = TARGET_WAITKIND_IGNORE;
+
+  /* Flush all currently pending debug events (thread and dll list) up
+     to the initial breakpoint.  */
+  while (1)
+    {
+      struct target_waitstatus status;
+
+      win32_wait (minus_one_ptid, &status, 0);
+
+      /* Note win32_wait doesn't return thread events.  */
+      if (status.kind != TARGET_WAITKIND_LOADED)
+	{
+	  cached_status = status;
+	  break;
+	}
+
+      {
+	struct thread_resume resume;
+
+	resume.thread = minus_one_ptid;
+	resume.kind = resume_continue;
+	resume.sig = 0;
+
+	win32_resume (&resume, 1);
+      }
+    }
 }

 /* Resume all artificially suspended threads if we are continuing
@@ -1593,6 +1628,18 @@ win32_wait (ptid_t ptid, struct target_waitstatus *ourstatus, int options)
 {
   struct regcache *regcache;

+  if (cached_status.kind != TARGET_WAITKIND_IGNORE)
+    {
+      /* The core always does a wait after creating the inferior, and
+	 do_initial_child_stuff already ran the inferior to the
+	 initial breakpoint (or an exit, if creating the process
+	 fails).  Report it now.  */
+      cached_status.kind = TARGET_WAITKIND_IGNORE;
+
+      *ourstatus = cached_status;
+      return debug_event_ptid (&current_event);
+    }
+
   while (1)
     {
       if (!get_child_debug_event (ourstatus))
@@ -1612,21 +1659,6 @@ win32_wait (ptid_t ptid, struct target_waitstatus *ourstatus, int options)

 	  regcache = get_thread_regcache (current_inferior, 1);
 	  child_fetch_inferior_registers (regcache, -1);
-
-	  if (ourstatus->kind == TARGET_WAITKIND_LOADED
-	      && !server_waiting)
-	    {
-	      /* When gdb connects, we want to be stopped at the
-		 initial breakpoint, not in some dll load event.  */
-	      child_continue (DBG_CONTINUE, -1);
-	      break;
-	    }
-
-	  /* We don't expose _LOADED events to gdbserver core.  See
-	     the `dlls_changed' global.  */
-	  if (ourstatus->kind == TARGET_WAITKIND_LOADED)
-	    ourstatus->kind = TARGET_WAITKIND_STOPPED;
-
 	  return debug_event_ptid (&current_event);
 	default:
 	  OUTMSG (("Ignoring unknown internal event, %d\n", ourstatus->kind));

Re: [RFA] nameless LOAD_DLL_DEBUG_EVENT causes ntdll.dll to be missing

[Pedro Alves]

On 12/10/2013 01:41 PM, Pedro Alves wrote:
> +  if (cached_status.kind != TARGET_WAITKIND_IGNORE)
> +    {
> +      /* The core always does a wait after creating the inferior, and
> +	 do_initial_child_stuff already ran the inferior to the
> +	 initial breakpoint (or an exit, if creating the process
> +	 fails).  Report it now.  */
> +      cached_status.kind = TARGET_WAITKIND_IGNORE;
> +
> +      *ourstatus = cached_status;

Well, that's silly.  Need to copy the status before
clearing it...

"target remote" doesn't currently use that status for anything,
so I didn't notice it.  Fixed patch below.

2013-12-10  Pedro Alves  <palves@redhat.com>

	* target.c (mywait): Convert TARGET_WAITKIND_LOADED to
	TARGET_WAITKIND_STOPPED.
	* win32-low.c (stopped_at_initial_breakpoint): New global.
	(do_initial_child_stuff): Consume events up to the initial
	breakpoint here.
	(win32_wait): Return the last event if starting up.
	Don't ignore TARGET_WAITKIND_LOADED here.
---

 gdb/gdbserver/target.c    |    5 ++++
 gdb/gdbserver/win32-low.c |   61 ++++++++++++++++++++++++++++++++++-----------
 2 files changed, 51 insertions(+), 15 deletions(-)

diff --git a/gdb/gdbserver/target.c b/gdb/gdbserver/target.c
index d4a2a98..d229933 100644
--- a/gdb/gdbserver/target.c
+++ b/gdb/gdbserver/target.c
@@ -82,6 +82,11 @@ mywait (ptid_t ptid, struct target_waitstatus *ourstatus, int options,

   ret = (*the_target->wait) (ptid, ourstatus, options);

+  /* We don't expose _LOADED events to gdbserver core.  See the
+     `dlls_changed' global.  */
+  if (ourstatus->kind == TARGET_WAITKIND_LOADED)
+    ourstatus->kind = TARGET_WAITKIND_STOPPED;
+
   /* If GDB is connected through TCP/serial, then GDBserver will most
      probably be running on its own terminal/console, so it's nice to
      print there why is GDBserver exiting.  If however, GDB is
diff --git a/gdb/gdbserver/win32-low.c b/gdb/gdbserver/win32-low.c
index 979eedd..a4c9e77 100644
--- a/gdb/gdbserver/win32-low.c
+++ b/gdb/gdbserver/win32-low.c
@@ -80,6 +80,11 @@ static enum gdb_signal last_sig = GDB_SIGNAL_0;
 /* The current debug event from WaitForDebugEvent.  */
 static DEBUG_EVENT current_event;

+/* A status that hasn't been reported to the core yet, and so
+   win32_wait should return it next, instead of fetching the next
+   debug event off the win32 API.  */
+static struct target_waitstatus cached_status;
+
 /* Non zero if an interrupt request is to be satisfied by suspending
    all threads.  */
 static int soft_interrupt_requested = 0;
@@ -97,6 +102,8 @@ typedef BOOL (WINAPI *winapi_DebugSetProcessKillOnExit) (BOOL KillOnExit);
 typedef BOOL (WINAPI *winapi_DebugBreakProcess) (HANDLE);
 typedef BOOL (WINAPI *winapi_GenerateConsoleCtrlEvent) (DWORD, DWORD);

+static ptid_t win32_wait (ptid_t ptid, struct target_waitstatus *ourstatus,
+			  int options);
 static void win32_resume (struct thread_resume *resume_info, size_t n);

 /* Get the thread ID from the current selected inferior (the current
@@ -336,6 +343,34 @@ do_initial_child_stuff (HANDLE proch, DWORD pid, int attached)

   if (the_low_target.initial_stuff != NULL)
     (*the_low_target.initial_stuff) ();
+
+  cached_status.kind = TARGET_WAITKIND_IGNORE;
+
+  /* Flush all currently pending debug events (thread and dll list) up
+     to the initial breakpoint.  */
+  while (1)
+    {
+      struct target_waitstatus status;
+
+      win32_wait (minus_one_ptid, &status, 0);
+
+      /* Note win32_wait doesn't return thread events.  */
+      if (status.kind != TARGET_WAITKIND_LOADED)
+	{
+	  cached_status = status;
+	  break;
+	}
+
+      {
+	struct thread_resume resume;
+
+	resume.thread = minus_one_ptid;
+	resume.kind = resume_continue;
+	resume.sig = 0;
+
+	win32_resume (&resume, 1);
+      }
+    }
 }

 /* Resume all artificially suspended threads if we are continuing
@@ -1593,6 +1628,17 @@ win32_wait (ptid_t ptid, struct target_waitstatus *ourstatus, int options)
 {
   struct regcache *regcache;

+  if (cached_status.kind != TARGET_WAITKIND_IGNORE)
+    {
+      /* The core always does a wait after creating the inferior, and
+	 do_initial_child_stuff already ran the inferior to the
+	 initial breakpoint (or an exit, if creating the process
+	 fails).  Report it now.  */
+      *ourstatus = cached_status;
+      cached_status.kind = TARGET_WAITKIND_IGNORE;
+      return debug_event_ptid (&current_event);
+    }
+
   while (1)
     {
       if (!get_child_debug_event (ourstatus))
@@ -1612,21 +1658,6 @@ win32_wait (ptid_t ptid, struct target_waitstatus *ourstatus, int options)

 	  regcache = get_thread_regcache (current_inferior, 1);
 	  child_fetch_inferior_registers (regcache, -1);
-
-	  if (ourstatus->kind == TARGET_WAITKIND_LOADED
-	      && !server_waiting)
-	    {
-	      /* When gdb connects, we want to be stopped at the
-		 initial breakpoint, not in some dll load event.  */
-	      child_continue (DBG_CONTINUE, -1);
-	      break;
-	    }
-
-	  /* We don't expose _LOADED events to gdbserver core.  See
-	     the `dlls_changed' global.  */
-	  if (ourstatus->kind == TARGET_WAITKIND_LOADED)
-	    ourstatus->kind = TARGET_WAITKIND_STOPPED;
-
 	  return debug_event_ptid (&current_event);
 	default:
 	  OUTMSG (("Ignoring unknown internal event, %d\n", ourstatus->kind));

Re: [RFA] nameless LOAD_DLL_DEBUG_EVENT causes ntdll.dll to be missing

[Joel Brobecker]

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

> Hmm, I had forgotten that.  I always though that gdbserver's
> "create inferior" sequence of calling mywait after create_inferior
> to be a little odd, leading to this issue (the FIXME):
[...]
> Changing that would mean changing more than we're willing at
> the moment.  We can still work in that direction, and actually
> make gdbserver's win32 initial event handling more similar to
> GDB's.
[...]
> What about this alternative below as preparatory for your
> patch?  It makes gdbserver closer to GDB here.
> 2013-12-10  Pedro Alves  <palves@redhat.com>
[updated patch]
> 2013-12-10  Pedro Alves  <palves@redhat.com>
> 
> 	* target.c (mywait): Convert TARGET_WAITKIND_LOADED to
> 	TARGET_WAITKIND_STOPPED.
> 	* win32-low.c (stopped_at_initial_breakpoint): New global.
> 	(do_initial_child_stuff): Consume events up to the initial
> 	breakpoint here.
> 	(win32_wait): Return the last event if starting up.
> 	Don't ignore TARGET_WAITKIND_LOADED here.

I tested the patch on x86-windows, with no regression.

On top of that patch, I was able to implement the same post-init
trick of looking for ntdll.dll, but not without a couple of surprises:
we don't have FILENAME_CMP nor a "basename" function in gdbserver
at the moment. I worked around the first issue by using strcasecmp,
which is good enough for our purposes. But for the second issue,
I only had a handful of bad options for our current situation:
  1. Import the module from gnulib; but that's never an innocent
     change, and also the documentation says that it does not work
     for Windows paths;
  2. Import the libiberty module by hand, which itself depends on
     their safe-ctype.h module.
  3. Write a quick ad hoc function that implements basename.

(1) is a non-starter, and I didn't like either of (2) or (3).
In the end, I went for (2) as the quickest option towards testing
the change and sending an RFC patch. Should we go with this approach,
we'll probably want to add the libiberty dependencies through
configure.srv instead of inside OBS. That addition should be
temporary, as the minute we stop looking specifically for ntdll,
and load all mapped dlls through that loop, we'll stop needing
lbasename, and will be able to remove the dependency.

But the good news is that the patch does fix the problem and adds
the missing ntdll.dll. I've tested the resulting gdbserver through
our testsuite as best as I could, and the results are more than
decent, so I think the change should be relatively good.

I'll also add comments and documentation, if we decide to move forward.

gdb/gdbserver/ChangeLog:

        * Makefile.in (OBS): Add safe-ctype.o and lbasename.o.
        (safe-ctype.o, lbasename.o): New rules.
        * win32-low.c (win32_ensure_ntdll_loaded): New function.
        (do_initial_child_stuff): Add call to win32_ensure_ntdll_loaded.

WDYT? It almost makes you want to take the risk of moving forward
with the post-branch proposal now rather than waiting for the branch
;-).

Thanks,
-- 
Joel

[-- Attachment #2: 0001-gdbserver-nameless-LOAD_DLL_DEBUG_EVENT-causes-ntdll.patch --]
[-- Type: text/x-diff, Size: 4847 bytes --]

From 11887cf93aa8f046610136e2ce3afd3100efc9a5 Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Thu, 12 Dec 2013 12:53:45 -0500
Subject: [PATCH] [gdbserver] nameless LOAD_DLL_DEBUG_EVENT causes ntdll.dll
 to be missing

This is the gdbserver-equivalent of the change made in GDB to handle
the case, in x64 windows version 2012, where the kernel produces
a LOAD_DLL_DEBUG_EVENT where the name of the associated DLL cannot
be determined at that time, and thus has to be processed later.

The visible symptom is that ntdll.dll is missing from the list of
shared libraries known to be mapped by the inferior, with other
side-effects such as failure to unwind through code provided by
that DLL (such as exception handling routines).

gdb/gdbserver/ChangeLog:

	* Makefile.in (OBS): Add safe-ctype.o and lbasename.o.
	(safe-ctype.o, lbasename.o): New rules.
	* win32-low.c (win32_ensure_ntdll_loaded): New function.
	(do_initial_child_stuff): Add call to win32_ensure_ntdll_loaded.
---
 gdb/gdbserver/Makefile.in |    8 ++++-
 gdb/gdbserver/win32-low.c |   68 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 75 insertions(+), 1 deletions(-)

diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
index 641ea17..67d3a09 100644
--- a/gdb/gdbserver/Makefile.in
+++ b/gdb/gdbserver/Makefile.in
@@ -176,7 +176,7 @@ OBS = agent.o ax.o inferiors.o regcache.o remote-utils.o server.o signals.o \
       target.o waitstatus.o utils.o version.o vec.o gdb_vecs.o \
       mem-break.o hostio.o event-loop.o tracepoint.o xml-utils.o \
       common-utils.o ptid.o buffer.o format.o filestuff.o dll.o notif.o \
-      tdesc.o $(XML_BUILTIN) $(DEPFILES) $(LIBOBJS)
+      tdesc.o safe-ctype.o lbasename.o $(XML_BUILTIN) $(DEPFILES) $(LIBOBJS)
 GDBREPLAY_OBS = gdbreplay.o version.o
 GDBSERVER_LIBS = @GDBSERVER_LIBS@
 XM_CLIBS = @LIBS@
@@ -543,6 +543,12 @@ vasprintf.o: $(srcdir)/../../libiberty/vasprintf.c
 vsnprintf.o: $(srcdir)/../../libiberty/vsnprintf.c
 	$(COMPILE) $<
 	$(POSTCOMPILE)
+safe-ctype.o: $(srcdir)/../../libiberty/safe-ctype.c
+	$(COMPILE) $<
+	$(POSTCOMPILE)
+lbasename.o: $(srcdir)/../../libiberty/lbasename.c
+	$(COMPILE) $<
+	$(POSTCOMPILE)
 
 aarch64.c : $(srcdir)/../regformats/aarch64.dat $(regdat_sh)
 	$(SHELL) $(regdat_sh) $(srcdir)/../regformats/aarch64.dat aarch64.c
diff --git a/gdb/gdbserver/win32-low.c b/gdb/gdbserver/win32-low.c
index a4c9e77..a5f9b9d 100644
--- a/gdb/gdbserver/win32-low.c
+++ b/gdb/gdbserver/win32-low.c
@@ -105,6 +105,7 @@ typedef BOOL (WINAPI *winapi_GenerateConsoleCtrlEvent) (DWORD, DWORD);
 static ptid_t win32_wait (ptid_t ptid, struct target_waitstatus *ourstatus,
 			  int options);
 static void win32_resume (struct thread_resume *resume_info, size_t n);
+static void win32_ensure_ntdll_loaded (void);
 
 /* Get the thread ID from the current selected inferior (the current
    thread).  */
@@ -371,6 +372,8 @@ do_initial_child_stuff (HANDLE proch, DWORD pid, int attached)
 	win32_resume (&resume, 1);
       }
     }
+
+  win32_ensure_ntdll_loaded ();
 }
 
 /* Resume all artificially suspended threads if we are continuing
@@ -1134,6 +1137,71 @@ failed:
   return 0;
 }
 
+static void
+win32_ensure_ntdll_loaded (void)
+{
+  struct inferior_list_entry *dll_e;
+  size_t i;
+  HMODULE dh_buf[1];
+  HMODULE *DllHandle = dh_buf;
+  DWORD cbNeeded;
+  BOOL ok;
+
+  for (dll_e = all_dlls.head; dll_e != NULL; dll_e = dll_e->next)
+    {
+      struct dll_info *dll = (struct dll_info *) dll_e;
+
+      if (strcasecmp (lbasename (dll->name), "ntdll.dll") == 0)
+	return;
+    }
+
+  if (!load_psapi ())
+    return;
+
+  cbNeeded = 0;
+  ok = (*win32_EnumProcessModules) (current_process_handle,
+				    DllHandle,
+				    sizeof (HMODULE),
+				    &cbNeeded);
+
+  if (!ok || !cbNeeded)
+    return;
+
+  DllHandle = (HMODULE *) alloca (cbNeeded);
+  if (!DllHandle)
+    return;
+
+  ok = (*win32_EnumProcessModules) (current_process_handle,
+				    DllHandle,
+				    cbNeeded,
+				    &cbNeeded);
+  if (!ok)
+    return;
+
+  for (i = 0; i < ((size_t) cbNeeded / sizeof (HMODULE)); i++)
+    {
+      MODULEINFO mi;
+      char dll_name[MAX_PATH];
+
+      if (!(*win32_GetModuleInformation) (current_process_handle,
+					  DllHandle[i],
+					  &mi,
+					  sizeof (mi)))
+	continue;
+      if ((*win32_GetModuleFileNameExA) (current_process_handle,
+					 DllHandle[i],
+					 dll_name,
+					 MAX_PATH) == 0)
+	continue;
+      if (strcasecmp (lbasename (dll_name), "ntdll.dll") == 0)
+	{
+	  win32_add_one_solib (dll_name,
+			       (CORE_ADDR) (uintptr_t) mi.lpBaseOfDll);
+	  return;
+	}
+    }
+}
+
 typedef HANDLE (WINAPI *winapi_CreateToolhelp32Snapshot) (DWORD, DWORD);
 typedef BOOL (WINAPI *winapi_Module32First) (HANDLE, LPMODULEENTRY32);
 typedef BOOL (WINAPI *winapi_Module32Next) (HANDLE, LPMODULEENTRY32);
-- 
1.7.9

Re: [RFA] nameless LOAD_DLL_DEBUG_EVENT causes ntdll.dll to be missing

[Eli Zaretskii]

> Date: Thu, 12 Dec 2013 19:18:43 +0100
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: gdb-patches@sourceware.org
> 
> On top of that patch, I was able to implement the same post-init
> trick of looking for ntdll.dll, but not without a couple of surprises:
> we don't have FILENAME_CMP nor a "basename" function in gdbserver
> at the moment. I worked around the first issue by using strcasecmp,
> which is good enough for our purposes. But for the second issue,
> I only had a handful of bad options for our current situation:
>   1. Import the module from gnulib; but that's never an innocent
>      change, and also the documentation says that it does not work
>      for Windows paths;
>   2. Import the libiberty module by hand, which itself depends on
>      their safe-ctype.h module.
>   3. Write a quick ad hoc function that implements basename.
> 
> (1) is a non-starter, and I didn't like either of (2) or (3).
> In the end, I went for (2) as the quickest option towards testing
> the change and sending an RFC patch. Should we go with this approach,
> we'll probably want to add the libiberty dependencies through
> configure.srv instead of inside OBS. That addition should be
> temporary, as the minute we stop looking specifically for ntdll,
> and load all mapped dlls through that loop, we'll stop needing
> lbasename, and will be able to remove the dependency.

I don't understand why you didn't like (2), I think that's the obvious
choice here.

Re: [RFA] nameless LOAD_DLL_DEBUG_EVENT causes ntdll.dll to be missing

[Pedro Alves]

On 12/12/2013 06:18 PM, Joel Brobecker wrote:
>> Hmm, I had forgotten that.  I always though that gdbserver's
>> "create inferior" sequence of calling mywait after create_inferior
>> to be a little odd, leading to this issue (the FIXME):
> [...]
>> Changing that would mean changing more than we're willing at
>> the moment.  We can still work in that direction, and actually
>> make gdbserver's win32 initial event handling more similar to
>> GDB's.
> [...]
>> What about this alternative below as preparatory for your
>> patch?  It makes gdbserver closer to GDB here.
>> 2013-12-10  Pedro Alves  <palves@redhat.com>
> [updated patch]
>> 2013-12-10  Pedro Alves  <palves@redhat.com>
>>
>> 	* target.c (mywait): Convert TARGET_WAITKIND_LOADED to
>> 	TARGET_WAITKIND_STOPPED.
>> 	* win32-low.c (stopped_at_initial_breakpoint): New global.
>> 	(do_initial_child_stuff): Consume events up to the initial
>> 	breakpoint here.
>> 	(win32_wait): Return the last event if starting up.
>> 	Don't ignore TARGET_WAITKIND_LOADED here.
> 
> I tested the patch on x86-windows, with no regression.
> 
> On top of that patch, I was able to implement the same post-init
> trick of looking for ntdll.dll, but not without a couple of surprises:
> we don't have FILENAME_CMP nor a "basename" function in gdbserver
> at the moment. I worked around the first issue by using strcasecmp,
> which is good enough for our purposes. But for the second issue,
> I only had a handful of bad options for our current situation:
>   1. Import the module from gnulib; but that's never an innocent
>      change, and also the documentation says that it does not work
>      for Windows paths;
>   2. Import the libiberty module by hand, which itself depends on
>      their safe-ctype.h module.
>   3. Write a quick ad hoc function that implements basename.
> 
> (1) is a non-starter, and I didn't like either of (2) or (3).
> In the end, I went for (2) as the quickest option towards testing
> the change and sending an RFC patch. Should we go with this approach,
> we'll probably want to add the libiberty dependencies through
> configure.srv instead of inside OBS. That addition should be
> temporary, as the minute we stop looking specifically for ntdll,
> and load all mapped dlls through that loop, we'll stop needing
> lbasename, and will be able to remove the dependency.
> 
> But the good news is that the patch does fix the problem and adds
> the missing ntdll.dll. I've tested the resulting gdbserver through
> our testsuite as best as I could, and the results are more than
> decent, so I think the change should be relatively good.
> 
> I'll also add comments and documentation, if we decide to move forward.

Looks fine to me.  Feel free to push my patch, or let me know if
you prefer that I push it.  It's all the same in a git world.

> gdb/gdbserver/ChangeLog:
> 
>         * Makefile.in (OBS): Add safe-ctype.o and lbasename.o.
>         (safe-ctype.o, lbasename.o): New rules.
>         * win32-low.c (win32_ensure_ntdll_loaded): New function.
>         (do_initial_child_stuff): Add call to win32_ensure_ntdll_loaded.
> 
> WDYT? It almost makes you want to take the risk of moving forward
> with the post-branch proposal now rather than waiting for the branch
> ;-).

:-)  I'd be OK with that.

(We can actually leave both methods in place,
for Windows versions that don't have psapi.dll.  E.g., leave most things
as is, and if ntdll.dll is not detected (or generically/alternatively, if
any dll was seen without a dll name), and psapi works, discard the
dll list, and refetch it after the initial breakpoint is hit.  We'd need
to make gdb's windows-nat.c not use wait_for_inferior, like I mentioned
before.)

-- 
Pedro Alves

Re: [RFA] nameless LOAD_DLL_DEBUG_EVENT causes ntdll.dll to be missing

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Joel> which is good enough for our purposes. But for the second issue,
Joel> I only had a handful of bad options for our current situation:
Joel>   1. Import the module from gnulib; but that's never an innocent
Joel>      change, and also the documentation says that it does not work
Joel>      for Windows paths;
Joel>   2. Import the libiberty module by hand, which itself depends on
Joel>      their safe-ctype.h module.
Joel>   3. Write a quick ad hoc function that implements basename.

Now that we are on git and can move stuff around without much hassle, I
think it's a good time for someone to tackle rearranging the tree a bit
so that this artificial "gdbserver cannot use libiberty" restriction can
be lifted.

I think this means turning common into a real library (itself a bit
tricky due to the #ifs) and moving the gnulib, common, target and
gdbserver directories to top-level.

Tom

Re: [RFA] nameless LOAD_DLL_DEBUG_EVENT causes ntdll.dll to be missing

[Pedro Alves]

On 12/12/2013 10:06 PM, Tom Tromey wrote:
>>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:
> 
> Joel> which is good enough for our purposes. But for the second issue,
> Joel> I only had a handful of bad options for our current situation:
> Joel>   1. Import the module from gnulib; but that's never an innocent
> Joel>      change, and also the documentation says that it does not work
> Joel>      for Windows paths;
> Joel>   2. Import the libiberty module by hand, which itself depends on
> Joel>      their safe-ctype.h module.
> Joel>   3. Write a quick ad hoc function that implements basename.
> 
> Now that we are on git and can move stuff around without much hassle, I
> think it's a good time for someone to tackle rearranging the tree a bit
> so that this artificial "gdbserver cannot use libiberty" restriction can
> be lifted.

Not speaking for or against the plan, but I don't recall that the current
restriction was based on directory structure.  With ACX_CONFIGURE_DIR, we
can build libiberty within gdbserver's build dir, like we do gnulib (see below),
so it seems that's a non-issue (even if that was is not the cleanest).
Rather, the main issue was that libiberty won't build out of the box on some
of the gdbserver targets, like Windows CE and Lynx, IIRC.  CE is probably
already broken for a while, so it's fine with me to not have it
block such a change.

---
 gdb/gdbserver/configure    | 97 ++++++++++++++++++++++++++++++++++++++++++++++
 gdb/gdbserver/configure.ac |  2 +
 2 files changed, 99 insertions(+)

diff --git a/gdb/gdbserver/configure b/gdb/gdbserver/configure
index fac7fb3..9c49ae2 100755
--- a/gdb/gdbserver/configure
+++ b/gdb/gdbserver/configure
@@ -4714,6 +4714,103 @@ $as_echo "$as_me: running $SHELL $ac_sub_configure $ac_sub_configure_args --cach
   cd "$ac_popdir"
 
 
+
+  in_src="../../libiberty"
+  in_build="build-libiberty-gdbserver"
+
+  # Remove --cache-file, --srcdir, and --disable-option-checking arguments
+  # so they do not pile up.
+  ac_sub_configure_args=
+  ac_prev=
+  eval "set x $ac_configure_args"
+  shift
+  for ac_arg
+  do
+    if test -n "$ac_prev"; then
+      ac_prev=
+      continue
+    fi
+    case $ac_arg in
+    -cache-file | --cache-file | --cache-fil | --cache-fi \
+    | --cache-f | --cache- | --cache | --cach | --cac | --ca | --c)
+      ac_prev=cache_file ;;
+    -cache-file=* | --cache-file=* | --cache-fil=* | --cache-fi=* \
+    | --cache-f=* | --cache-=* | --cache=* | --cach=* | --cac=* | --ca=* \
+    | --c=*)
+      ;;
+    --config-cache | -C)
+      ;;
+    -srcdir | --srcdir | --srcdi | --srcd | --src | --sr)
+      ac_prev=srcdir ;;
+    -srcdir=* | --srcdir=* | --srcdi=* | --srcd=* | --src=* | --sr=*)
+      ;;
+    -prefix | --prefix | --prefi | --pref | --pre | --pr | --p)
+      ac_prev=prefix ;;
+    -prefix=* | --prefix=* | --prefi=* | --pref=* | --pre=* | --pr=* | --p=*)
+      ;;
+    --disable-option-checking)
+      ;;
+    *)
+      case $ac_arg in
+      *\'*) ac_arg=`$as_echo "$ac_arg" | sed "s/'/'\\\\\\\\''/g"` ;;
+      esac
+      as_fn_append ac_sub_configure_args " '$ac_arg'" ;;
+    esac
+  done
+
+  # Always prepend --prefix to ensure using the same prefix
+  # in subdir configurations.
+  ac_arg="--prefix=$prefix"
+  case $ac_arg in
+  *\'*) ac_arg=`$as_echo "$ac_arg" | sed "s/'/'\\\\\\\\''/g"` ;;
+  esac
+  ac_sub_configure_args="'$ac_arg' $ac_sub_configure_args"
+
+  # Pass --silent
+  if test "$silent" = yes; then
+    ac_sub_configure_args="--silent $ac_sub_configure_args"
+  fi
+
+  # Always prepend --disable-option-checking to silence warnings, since
+  # different subdirs can have different --enable and --with options.
+  ac_sub_configure_args="--disable-option-checking $ac_sub_configure_args"
+
+  ac_popdir=`pwd`
+  ac_dir=$in_build
+
+  ac_msg="=== configuring in $ac_dir (`pwd`/$ac_dir)"
+  $as_echo "$as_me:${as_lineno-$LINENO}: $ac_msg" >&5
+  $as_echo "$ac_msg" >&6
+  as_dir="$ac_dir"; as_fn_mkdir_p
+
+  case $srcdir in
+  [\\/]* | ?:[\\/]* )
+    ac_srcdir=$srcdir/$in_src ;;
+  *) # Relative name.
+    ac_srcdir=../$srcdir/$in_src ;;
+  esac
+
+  cd "$ac_dir"
+
+  ac_sub_configure=$ac_srcdir/configure
+
+  # Make the cache file name correct relative to the subdirectory.
+  case $cache_file in
+  [\\/]* | ?:[\\/]* ) ac_sub_cache_file=$cache_file ;;
+  *) # Relative name.
+    ac_sub_cache_file=$ac_top_build_prefix$cache_file ;;
+  esac
+
+  { $as_echo "$as_me:${as_lineno-$LINENO}: running $SHELL $ac_sub_configure $ac_sub_configure_args --cache-file=$ac_sub_cache_file --srcdir=$ac_srcdir" >&5
+$as_echo "$as_me: running $SHELL $ac_sub_configure $ac_sub_configure_args --cache-file=$ac_sub_cache_file --srcdir=$ac_srcdir" >&6;}
+  # The eval makes quoting arguments work.
+  eval "\$SHELL \"\$ac_sub_configure\" $ac_sub_configure_args \
+       --cache-file=\"\$ac_sub_cache_file\" --srcdir=\"\$ac_srcdir\"" ||
+    as_fn_error "$ac_sub_configure failed for $ac_dir" "$LINENO" 5
+
+  cd "$ac_popdir"
+
+
 for ac_header in sgtty.h termio.h termios.h sys/reg.h string.h 		 proc_service.h sys/procfs.h linux/elf.h 		 errno.h fcntl.h signal.h sys/file.h malloc.h 		 sys/ioctl.h netinet/in.h sys/socket.h netdb.h 		 netinet/tcp.h arpa/inet.h
 do :
   as_ac_Header=`$as_echo "ac_cv_header_$ac_header" | $as_tr_sh`
diff --git a/gdb/gdbserver/configure.ac b/gdb/gdbserver/configure.ac
index 1c61843..5b39acc 100644
--- a/gdb/gdbserver/configure.ac
+++ b/gdb/gdbserver/configure.ac
@@ -77,6 +77,8 @@ AC_PROG_MAKE_SET
 # build it in the same directory, when building in the source dir.
 ACX_CONFIGURE_DIR(["../gnulib"], ["build-gnulib-gdbserver"])
 
+ACX_CONFIGURE_DIR(["../../libiberty"], ["build-libiberty-gdbserver"])
+
 AC_CHECK_HEADERS(sgtty.h termio.h termios.h sys/reg.h string.h dnl
 		 proc_service.h sys/procfs.h linux/elf.h dnl
 		 errno.h fcntl.h signal.h sys/file.h malloc.h dnl
-- 
1.7.11.7

Re: [RFA] nameless LOAD_DLL_DEBUG_EVENT causes ntdll.dll to be missing

[Joel Brobecker]

> Not speaking for or against the plan, but I don't recall that the
> current restriction was based on directory structure.  With
> ACX_CONFIGURE_DIR, we can build libiberty within gdbserver's build
> dir, like we do gnulib (see below), so it seems that's a non-issue
> (even if that was is not the cleanest).  Rather, the main issue was
> that libiberty won't build out of the box on some of the gdbserver
> targets, like Windows CE and Lynx, IIRC.  CE is probably already
> broken for a while, so it's fine with me to not have it block such a
> change.

The patch only configures libiberty without building it, but going into
that build subdirectory and doing "make", I get a libiberty.a archive
for both ppc-lynx178 and x86-lynx5. So, if CE is not a blocker,
I think we could go ahead.

But this would really only be used by Windows, though, so delay
for after the branch?

-- 
Joel

Re: [RFA] nameless LOAD_DLL_DEBUG_EVENT causes ntdll.dll to be missing

[Pedro Alves]

On 12/13/2013 11:04 AM, Joel Brobecker wrote:

> delay for after the branch?

IMO, yes.

-- 
Pedro Alves

Re: [RFA] nameless LOAD_DLL_DEBUG_EVENT causes ntdll.dll to be missing

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> Not speaking for or against the plan, but I don't recall that the
Pedro> current restriction was based on directory structure.  With
Pedro> ACX_CONFIGURE_DIR, we can build libiberty within gdbserver's
Pedro> build dir, like we do gnulib (see below), so it seems that's a
Pedro> non-issue (even if that was is not the cleanest).

Yeah, I was pushing for an increase in cleanliness as well.

Pedro> Rather, the
Pedro> main issue was that libiberty won't build out of the box on some
Pedro> of the gdbserver targets, like Windows CE and Lynx, IIRC.

That has to be fixable with one of the N portability technologies
already in place :)

Tom

Re: [RFA] nameless LOAD_DLL_DEBUG_EVENT causes ntdll.dll to be missing

[Joel Brobecker]

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

> I'll also add comments and documentation, if we decide to move forward.

Here is an updated patch. Compared to the last one, I moved the
linking of safe-ctype.o and lbasename.o to configure.srv instead
of Makefile.in::OBS, so only Windows builds are affected. I also
copy/pasted the comment from windows-nat.c to document the new function.

gdb/gdbserver/ChangeLog:

	* Makefile.in (safe-ctype.o, lbasename.o): New rules.
	* configure.srv: Add safe-ctype.o and lbasename.o to srv_tgtobj
	for all targets that use win32-low.c.
	* win32-low.c (win32_ensure_ntdll_loaded): New function.
	(do_initial_child_stuff): Add call to win32_ensure_ntdll_loaded.

I tested the patch on both x86_64-windows and also x86-windows.
OK to commit this version? (I can take care of checking in your patch)

Thank you,
-- 
Joel

[-- Attachment #2: 0001-gdbserver-nameless-LOAD_DLL_DEBUG_EVENT-causes-ntdll.patch --]
[-- Type: text/x-diff, Size: 7077 bytes --]

From 362d97dbeca5ad34e7cea715c3a67e8bfc9ce819 Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Thu, 12 Dec 2013 12:53:45 -0500
Subject: [PATCH] [gdbserver] nameless LOAD_DLL_DEBUG_EVENT causes ntdll.dll
 to be missing

This is the gdbserver-equivalent of the change made in GDB to handle
the case, in x64 windows version 2012, where the kernel produces
a LOAD_DLL_DEBUG_EVENT where the name of the associated DLL cannot
be determined at that time, and thus has to be processed later.

The visible symptom is that ntdll.dll is missing from the list of
shared libraries known to be mapped by the inferior, with other
side-effects such as failure to unwind through code provided by
that DLL (such as exception handling routines).

gdb/gdbserver/ChangeLog:

	* Makefile.in (safe-ctype.o, lbasename.o): New rules.
	* configure.srv: Add safe-ctype.o and lbasename.o to srv_tgtobj
	for all targets that use win32-low.c.
	* win32-low.c (win32_ensure_ntdll_loaded): New function.
	(do_initial_child_stuff): Add call to win32_ensure_ntdll_loaded.
---
 gdb/gdbserver/Makefile.in   |    6 +++
 gdb/gdbserver/configure.srv |    6 +++
 gdb/gdbserver/win32-low.c   |   81 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 93 insertions(+), 0 deletions(-)

diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
index 641ea17..c8d971b 100644
--- a/gdb/gdbserver/Makefile.in
+++ b/gdb/gdbserver/Makefile.in
@@ -543,6 +543,12 @@ vasprintf.o: $(srcdir)/../../libiberty/vasprintf.c
 vsnprintf.o: $(srcdir)/../../libiberty/vsnprintf.c
 	$(COMPILE) $<
 	$(POSTCOMPILE)
+safe-ctype.o: $(srcdir)/../../libiberty/safe-ctype.c
+	$(COMPILE) $<
+	$(POSTCOMPILE)
+lbasename.o: $(srcdir)/../../libiberty/lbasename.c
+	$(COMPILE) $<
+	$(POSTCOMPILE)
 
 aarch64.c : $(srcdir)/../regformats/aarch64.dat $(regdat_sh)
 	$(SHELL) $(regdat_sh) $(srcdir)/../regformats/aarch64.dat aarch64.c
diff --git a/gdb/gdbserver/configure.srv b/gdb/gdbserver/configure.srv
index 32d935a..f4e6154 100644
--- a/gdb/gdbserver/configure.srv
+++ b/gdb/gdbserver/configure.srv
@@ -77,6 +77,7 @@ case "${target}" in
 			;;
   arm*-*-mingw32ce*)	srv_regobj=reg-arm.o
 			srv_tgtobj="win32-low.o win32-arm-low.o"
+			srv_tgtobj="${srv_tgtobj} safe-ctype.o lbasename.o"
 			srv_tgtobj="${srv_tgtobj} wincecompat.o"
 			# hostio_last_error implementation is in win32-low.c
 			srv_hostio_err_objs=""
@@ -100,6 +101,7 @@ case "${target}" in
 			;;
   i[34567]86-*-cygwin*)	srv_regobj="$srv_i386_regobj"
 			srv_tgtobj="i386-low.o win32-low.o win32-i386-low.o"
+			srv_tgtobj="${srv_tgtobj} safe-ctype.o lbasename.o"
 			srv_xmlfiles="$srv_i386_xmlfiles"
 			;;
   i[34567]86-*-linux*)	srv_regobj="$srv_i386_linux_regobj"
@@ -126,6 +128,7 @@ case "${target}" in
   i[34567]86-*-mingw32ce*)
 			srv_regobj="$srv_i386_regobj"
 			srv_tgtobj="i386-low.o win32-low.o win32-i386-low.o"
+			srv_tgtobj="${srv_tgtobj} safe-ctype.o lbasename.o"
 			srv_tgtobj="${srv_tgtobj} wincecompat.o"
 			srv_xmlfiles="$srv_i386_xmlfiles"
 			# hostio_last_error implementation is in win32-low.c
@@ -135,6 +138,7 @@ case "${target}" in
 			;;
   i[34567]86-*-mingw*)	srv_regobj="$srv_i386_regobj"
 			srv_tgtobj="i386-low.o win32-low.o win32-i386-low.o"
+			srv_tgtobj="${srv_tgtobj} safe-ctype.o lbasename.o"
 			srv_xmlfiles="$srv_i386_xmlfiles"
 			srv_mingw=yes
 			;;
@@ -326,11 +330,13 @@ case "${target}" in
 			;;
   x86_64-*-mingw*)	srv_regobj="$srv_amd64_regobj"
 			srv_tgtobj="i386-low.o i387-fp.o win32-low.o win32-i386-low.o"
+			srv_tgtobj="${srv_tgtobj} safe-ctype.o lbasename.o"
 			srv_xmlfiles="$srv_i386_xmlfiles $srv_amd64_xmlfiles"
 			srv_mingw=yes
 			;;
   x86_64-*-cygwin*)	srv_regobj="$srv_amd64_regobj"
 			srv_tgtobj="i386-low.o i387-fp.o win32-low.o win32-i386-low.o"
+			srv_tgtobj="${srv_tgtobj} safe-ctype.o lbasename.o"
 			srv_xmlfiles="$srv_i386_xmlfiles"
 			;;
 
diff --git a/gdb/gdbserver/win32-low.c b/gdb/gdbserver/win32-low.c
index a4c9e77..91e24a0 100644
--- a/gdb/gdbserver/win32-low.c
+++ b/gdb/gdbserver/win32-low.c
@@ -105,6 +105,7 @@ typedef BOOL (WINAPI *winapi_GenerateConsoleCtrlEvent) (DWORD, DWORD);
 static ptid_t win32_wait (ptid_t ptid, struct target_waitstatus *ourstatus,
 			  int options);
 static void win32_resume (struct thread_resume *resume_info, size_t n);
+static void win32_ensure_ntdll_loaded (void);
 
 /* Get the thread ID from the current selected inferior (the current
    thread).  */
@@ -371,6 +372,8 @@ do_initial_child_stuff (HANDLE proch, DWORD pid, int attached)
 	win32_resume (&resume, 1);
       }
     }
+
+  win32_ensure_ntdll_loaded ();
 }
 
 /* Resume all artificially suspended threads if we are continuing
@@ -1134,6 +1137,84 @@ failed:
   return 0;
 }
 
+/* On certain versions of Windows, the information about ntdll.dll
+   is not available yet at the time we get the LOAD_DLL_DEBUG_EVENT,
+   thus preventing us from reporting this DLL as an SO. This has been
+   witnessed on Windows 8.1, for instance.  A possible explanation
+   is that ntdll.dll might be mapped before the SO info gets created
+   by the Windows system -- ntdll.dll is the first DLL to be reported
+   via LOAD_DLL_DEBUG_EVENT and other DLLs do not seem to suffer from
+   that problem.
+
+   If we indeed are missing ntdll.dll, this function tries to recover
+   from this issue, after the fact.  Do nothing if we encounter any
+   issue trying to locate that DLL.  */
+
+static void
+win32_ensure_ntdll_loaded (void)
+{
+  struct inferior_list_entry *dll_e;
+  size_t i;
+  HMODULE dh_buf[1];
+  HMODULE *DllHandle = dh_buf;
+  DWORD cbNeeded;
+  BOOL ok;
+
+  for (dll_e = all_dlls.head; dll_e != NULL; dll_e = dll_e->next)
+    {
+      struct dll_info *dll = (struct dll_info *) dll_e;
+
+      if (strcasecmp (lbasename (dll->name), "ntdll.dll") == 0)
+	return;
+    }
+
+  if (!load_psapi ())
+    return;
+
+  cbNeeded = 0;
+  ok = (*win32_EnumProcessModules) (current_process_handle,
+				    DllHandle,
+				    sizeof (HMODULE),
+				    &cbNeeded);
+
+  if (!ok || !cbNeeded)
+    return;
+
+  DllHandle = (HMODULE *) alloca (cbNeeded);
+  if (!DllHandle)
+    return;
+
+  ok = (*win32_EnumProcessModules) (current_process_handle,
+				    DllHandle,
+				    cbNeeded,
+				    &cbNeeded);
+  if (!ok)
+    return;
+
+  for (i = 0; i < ((size_t) cbNeeded / sizeof (HMODULE)); i++)
+    {
+      MODULEINFO mi;
+      char dll_name[MAX_PATH];
+
+      if (!(*win32_GetModuleInformation) (current_process_handle,
+					  DllHandle[i],
+					  &mi,
+					  sizeof (mi)))
+	continue;
+      if ((*win32_GetModuleFileNameExA) (current_process_handle,
+					 DllHandle[i],
+					 dll_name,
+					 MAX_PATH) == 0)
+	continue;
+      if (strcasecmp (lbasename (dll_name), "ntdll.dll") == 0)
+	{
+	  win32_add_one_solib (dll_name,
+			       (CORE_ADDR) (uintptr_t) mi.lpBaseOfDll);
+	  return;
+	}
+    }
+}
+
 typedef HANDLE (WINAPI *winapi_CreateToolhelp32Snapshot) (DWORD, DWORD);
 typedef BOOL (WINAPI *winapi_Module32First) (HANDLE, LPMODULEENTRY32);
 typedef BOOL (WINAPI *winapi_Module32Next) (HANDLE, LPMODULEENTRY32);
-- 
1.7.9

Re: [RFA] nameless LOAD_DLL_DEBUG_EVENT causes ntdll.dll to be missing

[Pedro Alves]

Looks OK to me.  Thanks.

On 12/13/2013 02:17 PM, Joel Brobecker wrote:
> +
> +  win32_ensure_ntdll_loaded ();
>  }

(
Though it's harmless as is, there's no ntdll.dll on CE, only on
Windows versions with NT based kernels.  To be consistent
with what we do elsewhere on the file already when
treating ntdll.dll especially, this could be:

#ifndef _WIN32_WCE
  win32_ensure_ntdll_loaded ();
#endif
)

-- 
Pedro Alves

Re: [RFA] nameless LOAD_DLL_DEBUG_EVENT causes ntdll.dll to be missing

[Joel Brobecker]

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

> Looks OK to me.  Thanks.

Thanks!

> Though it's harmless as is, there's no ntdll.dll on CE, only on
> Windows versions with NT based kernels.  To be consistent
> with what we do elsewhere on the file already when
> treating ntdll.dll especially, this could be:
> 
> #ifndef _WIN32_WCE
>   win32_ensure_ntdll_loaded ();
> #endif

That's a good suggestion. Attached at the two patches I just pushed.
The second shows the added #ifndef's....

Thanks again!
-- 
Joel

[-- Attachment #2: 0001-Do-the-target-waiting-within-do_initial_child_stuff-.patch --]
[-- Type: text/x-diff, Size: 6765 bytes --]

From 4210d83ee607bffaf27a235f0475bf1e5ea8266d Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Thu, 12 Dec 2013 11:12:30 -0500
Subject: [PATCH 1/2] Do the target-waiting within do_initial_child_stuff on
 Windows.

This is a preparatory patch that achieves two goals:

  . Makes the initial event handling more similar to GDB's;
  . Opens the door for implementing post-inititial-handling
    operations.

At the moment, this is only done on Windows, where the
post-initial-handling is going to be needed (in the context of
Windows 2012). And because we're close to creating the gdb 7.7
branch, making that change for all platforms is a little more
risk that we'd like. So the change is currently implemented
on Windows.

gdb/gdbserver/ChangeLog:

        * target.c (mywait): Set OURSTATUS->KIND to TARGET_WAITKIND_STOPPED
        if equal to TARGET_WAITKIND_LOADED.
        * win32-low.c (cached_status): New static global.
        (win32_wait): Add declaration.
        (do_initial_child_stuff): Flush all initial pending debug events
        up to the initial breakpoint.
        (win32_wait): If CACHED_STATUS was set, return that instead
        of doing a real wait.  Remove the code resuming the execution
        of the inferior after receiving a TARGET_WAITKIND_LOADED event
        during the initial phase.  Also remove the code changing
        OURSTATUS->KIND from TARGET_WAITKIND_LOADED to
        TARGET_WAITKIND_STOPPED.
---
 gdb/gdbserver/ChangeLog   | 15 ++++++++++++
 gdb/gdbserver/target.c    |  5 ++++
 gdb/gdbserver/win32-low.c | 61 +++++++++++++++++++++++++++++++++++------------
 3 files changed, 66 insertions(+), 15 deletions(-)

diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog
index 26d305e..9f8eb9e 100644
--- a/gdb/gdbserver/ChangeLog
+++ b/gdb/gdbserver/ChangeLog
@@ -1,3 +1,18 @@
+2013-12-13  Pedro Alves  <palves@redhat.com>
+
+	* target.c (mywait): Set OURSTATUS->KIND to TARGET_WAITKIND_STOPPED
+	if equal to TARGET_WAITKIND_LOADED.
+	* win32-low.c (cached_status): New static global.
+	(win32_wait): Add declaration.
+	(do_initial_child_stuff): Flush all initial pending debug events
+	up to the initial breakpoint.
+	(win32_wait): If CACHED_STATUS was set, return that instead
+	of doing a real wait.  Remove the code resuming the execution
+	of the inferior after receiving a TARGET_WAITKIND_LOADED event
+	during the initial phase.  Also remove the code changing
+	OURSTATUS->KIND from TARGET_WAITKIND_LOADED to
+	TARGET_WAITKIND_STOPPED.
+
 2013-12-11  Yao Qi  <yao@codesourcery.com>
 
 	* notif.c (handle_notif_ack): Return 0 if no notification
diff --git a/gdb/gdbserver/target.c b/gdb/gdbserver/target.c
index d4a2a98..d229933 100644
--- a/gdb/gdbserver/target.c
+++ b/gdb/gdbserver/target.c
@@ -82,6 +82,11 @@ mywait (ptid_t ptid, struct target_waitstatus *ourstatus, int options,
 
   ret = (*the_target->wait) (ptid, ourstatus, options);
 
+  /* We don't expose _LOADED events to gdbserver core.  See the
+     `dlls_changed' global.  */
+  if (ourstatus->kind == TARGET_WAITKIND_LOADED)
+    ourstatus->kind = TARGET_WAITKIND_STOPPED;
+
   /* If GDB is connected through TCP/serial, then GDBserver will most
      probably be running on its own terminal/console, so it's nice to
      print there why is GDBserver exiting.  If however, GDB is
diff --git a/gdb/gdbserver/win32-low.c b/gdb/gdbserver/win32-low.c
index 979eedd..a4c9e77 100644
--- a/gdb/gdbserver/win32-low.c
+++ b/gdb/gdbserver/win32-low.c
@@ -80,6 +80,11 @@ static enum gdb_signal last_sig = GDB_SIGNAL_0;
 /* The current debug event from WaitForDebugEvent.  */
 static DEBUG_EVENT current_event;
 
+/* A status that hasn't been reported to the core yet, and so
+   win32_wait should return it next, instead of fetching the next
+   debug event off the win32 API.  */
+static struct target_waitstatus cached_status;
+
 /* Non zero if an interrupt request is to be satisfied by suspending
    all threads.  */
 static int soft_interrupt_requested = 0;
@@ -97,6 +102,8 @@ typedef BOOL (WINAPI *winapi_DebugSetProcessKillOnExit) (BOOL KillOnExit);
 typedef BOOL (WINAPI *winapi_DebugBreakProcess) (HANDLE);
 typedef BOOL (WINAPI *winapi_GenerateConsoleCtrlEvent) (DWORD, DWORD);
 
+static ptid_t win32_wait (ptid_t ptid, struct target_waitstatus *ourstatus,
+			  int options);
 static void win32_resume (struct thread_resume *resume_info, size_t n);
 
 /* Get the thread ID from the current selected inferior (the current
@@ -336,6 +343,34 @@ do_initial_child_stuff (HANDLE proch, DWORD pid, int attached)
 
   if (the_low_target.initial_stuff != NULL)
     (*the_low_target.initial_stuff) ();
+
+  cached_status.kind = TARGET_WAITKIND_IGNORE;
+
+  /* Flush all currently pending debug events (thread and dll list) up
+     to the initial breakpoint.  */
+  while (1)
+    {
+      struct target_waitstatus status;
+
+      win32_wait (minus_one_ptid, &status, 0);
+
+      /* Note win32_wait doesn't return thread events.  */
+      if (status.kind != TARGET_WAITKIND_LOADED)
+	{
+	  cached_status = status;
+	  break;
+	}
+
+      {
+	struct thread_resume resume;
+
+	resume.thread = minus_one_ptid;
+	resume.kind = resume_continue;
+	resume.sig = 0;
+
+	win32_resume (&resume, 1);
+      }
+    }
 }
 
 /* Resume all artificially suspended threads if we are continuing
@@ -1593,6 +1628,17 @@ win32_wait (ptid_t ptid, struct target_waitstatus *ourstatus, int options)
 {
   struct regcache *regcache;
 
+  if (cached_status.kind != TARGET_WAITKIND_IGNORE)
+    {
+      /* The core always does a wait after creating the inferior, and
+	 do_initial_child_stuff already ran the inferior to the
+	 initial breakpoint (or an exit, if creating the process
+	 fails).  Report it now.  */
+      *ourstatus = cached_status;
+      cached_status.kind = TARGET_WAITKIND_IGNORE;
+      return debug_event_ptid (&current_event);
+    }
+
   while (1)
     {
       if (!get_child_debug_event (ourstatus))
@@ -1612,21 +1658,6 @@ win32_wait (ptid_t ptid, struct target_waitstatus *ourstatus, int options)
 
 	  regcache = get_thread_regcache (current_inferior, 1);
 	  child_fetch_inferior_registers (regcache, -1);
-
-	  if (ourstatus->kind == TARGET_WAITKIND_LOADED
-	      && !server_waiting)
-	    {
-	      /* When gdb connects, we want to be stopped at the
-		 initial breakpoint, not in some dll load event.  */
-	      child_continue (DBG_CONTINUE, -1);
-	      break;
-	    }
-
-	  /* We don't expose _LOADED events to gdbserver core.  See
-	     the `dlls_changed' global.  */
-	  if (ourstatus->kind == TARGET_WAITKIND_LOADED)
-	    ourstatus->kind = TARGET_WAITKIND_STOPPED;
-
 	  return debug_event_ptid (&current_event);
 	default:
 	  OUTMSG (("Ignoring unknown internal event, %d\n", ourstatus->kind));
-- 
1.8.1.2


[-- Attachment #3: 0002-nameless-LOAD_DLL_DEBUG_EVENT-causes-ntdll.dll-to-be.patch --]
[-- Type: text/x-diff, Size: 7812 bytes --]

From 379a5e2d36e4323702d48aeb794a8e42bf5bff5b Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Thu, 12 Dec 2013 12:53:45 -0500
Subject: [PATCH 2/2] nameless LOAD_DLL_DEBUG_EVENT causes ntdll.dll to be
 missing

This is the gdbserver-equivalent of the change made in GDB to handle
the case, in x64 windows version 2012, where the kernel produces
a LOAD_DLL_DEBUG_EVENT where the name of the associated DLL cannot
be determined at that time, and thus has to be processed later.

The visible symptom is that ntdll.dll is missing from the list of
shared libraries known to be mapped by the inferior, with other
side-effects such as failure to unwind through code provided by
that DLL (such as exception handling routines).

gdb/gdbserver/ChangeLog:

	* Makefile.in (safe-ctype.o, lbasename.o): New rules.
	* configure.srv: Add safe-ctype.o and lbasename.o to srv_tgtobj
	for all targets that use win32-low.c.
	* win32-low.c (win32_ensure_ntdll_loaded): New function.
	(do_initial_child_stuff): Add call to win32_ensure_ntdll_loaded.
---
 gdb/gdbserver/ChangeLog     |  8 +++++
 gdb/gdbserver/Makefile.in   |  6 ++++
 gdb/gdbserver/configure.srv |  6 ++++
 gdb/gdbserver/win32-low.c   | 87 +++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 107 insertions(+)

diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog
index 9f8eb9e..c73840f 100644
--- a/gdb/gdbserver/ChangeLog
+++ b/gdb/gdbserver/ChangeLog
@@ -1,3 +1,11 @@
+2013-12-13  Joel Brobecker  <brobecker@adacore.com>
+
+	* Makefile.in (safe-ctype.o, lbasename.o): New rules.
+	* configure.srv: Add safe-ctype.o and lbasename.o to srv_tgtobj
+	for all targets that use win32-low.c.
+	* win32-low.c (win32_ensure_ntdll_loaded): New function.
+	(do_initial_child_stuff): Add call to win32_ensure_ntdll_loaded.
+
 2013-12-13  Pedro Alves  <palves@redhat.com>
 
 	* target.c (mywait): Set OURSTATUS->KIND to TARGET_WAITKIND_STOPPED
diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
index 641ea17..c8d971b 100644
--- a/gdb/gdbserver/Makefile.in
+++ b/gdb/gdbserver/Makefile.in
@@ -543,6 +543,12 @@ vasprintf.o: $(srcdir)/../../libiberty/vasprintf.c
 vsnprintf.o: $(srcdir)/../../libiberty/vsnprintf.c
 	$(COMPILE) $<
 	$(POSTCOMPILE)
+safe-ctype.o: $(srcdir)/../../libiberty/safe-ctype.c
+	$(COMPILE) $<
+	$(POSTCOMPILE)
+lbasename.o: $(srcdir)/../../libiberty/lbasename.c
+	$(COMPILE) $<
+	$(POSTCOMPILE)
 
 aarch64.c : $(srcdir)/../regformats/aarch64.dat $(regdat_sh)
 	$(SHELL) $(regdat_sh) $(srcdir)/../regformats/aarch64.dat aarch64.c
diff --git a/gdb/gdbserver/configure.srv b/gdb/gdbserver/configure.srv
index 32d935a..f4e6154 100644
--- a/gdb/gdbserver/configure.srv
+++ b/gdb/gdbserver/configure.srv
@@ -77,6 +77,7 @@ case "${target}" in
 			;;
   arm*-*-mingw32ce*)	srv_regobj=reg-arm.o
 			srv_tgtobj="win32-low.o win32-arm-low.o"
+			srv_tgtobj="${srv_tgtobj} safe-ctype.o lbasename.o"
 			srv_tgtobj="${srv_tgtobj} wincecompat.o"
 			# hostio_last_error implementation is in win32-low.c
 			srv_hostio_err_objs=""
@@ -100,6 +101,7 @@ case "${target}" in
 			;;
   i[34567]86-*-cygwin*)	srv_regobj="$srv_i386_regobj"
 			srv_tgtobj="i386-low.o win32-low.o win32-i386-low.o"
+			srv_tgtobj="${srv_tgtobj} safe-ctype.o lbasename.o"
 			srv_xmlfiles="$srv_i386_xmlfiles"
 			;;
   i[34567]86-*-linux*)	srv_regobj="$srv_i386_linux_regobj"
@@ -126,6 +128,7 @@ case "${target}" in
   i[34567]86-*-mingw32ce*)
 			srv_regobj="$srv_i386_regobj"
 			srv_tgtobj="i386-low.o win32-low.o win32-i386-low.o"
+			srv_tgtobj="${srv_tgtobj} safe-ctype.o lbasename.o"
 			srv_tgtobj="${srv_tgtobj} wincecompat.o"
 			srv_xmlfiles="$srv_i386_xmlfiles"
 			# hostio_last_error implementation is in win32-low.c
@@ -135,6 +138,7 @@ case "${target}" in
 			;;
   i[34567]86-*-mingw*)	srv_regobj="$srv_i386_regobj"
 			srv_tgtobj="i386-low.o win32-low.o win32-i386-low.o"
+			srv_tgtobj="${srv_tgtobj} safe-ctype.o lbasename.o"
 			srv_xmlfiles="$srv_i386_xmlfiles"
 			srv_mingw=yes
 			;;
@@ -326,11 +330,13 @@ case "${target}" in
 			;;
   x86_64-*-mingw*)	srv_regobj="$srv_amd64_regobj"
 			srv_tgtobj="i386-low.o i387-fp.o win32-low.o win32-i386-low.o"
+			srv_tgtobj="${srv_tgtobj} safe-ctype.o lbasename.o"
 			srv_xmlfiles="$srv_i386_xmlfiles $srv_amd64_xmlfiles"
 			srv_mingw=yes
 			;;
   x86_64-*-cygwin*)	srv_regobj="$srv_amd64_regobj"
 			srv_tgtobj="i386-low.o i387-fp.o win32-low.o win32-i386-low.o"
+			srv_tgtobj="${srv_tgtobj} safe-ctype.o lbasename.o"
 			srv_xmlfiles="$srv_i386_xmlfiles"
 			;;
 
diff --git a/gdb/gdbserver/win32-low.c b/gdb/gdbserver/win32-low.c
index a4c9e77..bcc77fb 100644
--- a/gdb/gdbserver/win32-low.c
+++ b/gdb/gdbserver/win32-low.c
@@ -105,6 +105,9 @@ typedef BOOL (WINAPI *winapi_GenerateConsoleCtrlEvent) (DWORD, DWORD);
 static ptid_t win32_wait (ptid_t ptid, struct target_waitstatus *ourstatus,
 			  int options);
 static void win32_resume (struct thread_resume *resume_info, size_t n);
+#ifndef _WIN32_WCE
+static void win32_ensure_ntdll_loaded (void);
+#endif
 
 /* Get the thread ID from the current selected inferior (the current
    thread).  */
@@ -371,6 +374,10 @@ do_initial_child_stuff (HANDLE proch, DWORD pid, int attached)
 	win32_resume (&resume, 1);
       }
     }
+
+#ifndef _WIN32_WCE
+  win32_ensure_ntdll_loaded ();
+#endif
 }
 
 /* Resume all artificially suspended threads if we are continuing
@@ -1134,6 +1141,86 @@ failed:
   return 0;
 }
 
+#ifndef _WIN32_WCE
+/* On certain versions of Windows, the information about ntdll.dll
+   is not available yet at the time we get the LOAD_DLL_DEBUG_EVENT,
+   thus preventing us from reporting this DLL as an SO. This has been
+   witnessed on Windows 8.1, for instance.  A possible explanation
+   is that ntdll.dll might be mapped before the SO info gets created
+   by the Windows system -- ntdll.dll is the first DLL to be reported
+   via LOAD_DLL_DEBUG_EVENT and other DLLs do not seem to suffer from
+   that problem.
+
+   If we indeed are missing ntdll.dll, this function tries to recover
+   from this issue, after the fact.  Do nothing if we encounter any
+   issue trying to locate that DLL.  */
+
+static void
+win32_ensure_ntdll_loaded (void)
+{
+  struct inferior_list_entry *dll_e;
+  size_t i;
+  HMODULE dh_buf[1];
+  HMODULE *DllHandle = dh_buf;
+  DWORD cbNeeded;
+  BOOL ok;
+
+  for (dll_e = all_dlls.head; dll_e != NULL; dll_e = dll_e->next)
+    {
+      struct dll_info *dll = (struct dll_info *) dll_e;
+
+      if (strcasecmp (lbasename (dll->name), "ntdll.dll") == 0)
+	return;
+    }
+
+  if (!load_psapi ())
+    return;
+
+  cbNeeded = 0;
+  ok = (*win32_EnumProcessModules) (current_process_handle,
+				    DllHandle,
+				    sizeof (HMODULE),
+				    &cbNeeded);
+
+  if (!ok || !cbNeeded)
+    return;
+
+  DllHandle = (HMODULE *) alloca (cbNeeded);
+  if (!DllHandle)
+    return;
+
+  ok = (*win32_EnumProcessModules) (current_process_handle,
+				    DllHandle,
+				    cbNeeded,
+				    &cbNeeded);
+  if (!ok)
+    return;
+
+  for (i = 0; i < ((size_t) cbNeeded / sizeof (HMODULE)); i++)
+    {
+      MODULEINFO mi;
+      char dll_name[MAX_PATH];
+
+      if (!(*win32_GetModuleInformation) (current_process_handle,
+					  DllHandle[i],
+					  &mi,
+					  sizeof (mi)))
+	continue;
+      if ((*win32_GetModuleFileNameExA) (current_process_handle,
+					 DllHandle[i],
+					 dll_name,
+					 MAX_PATH) == 0)
+	continue;
+      if (strcasecmp (lbasename (dll_name), "ntdll.dll") == 0)
+	{
+	  win32_add_one_solib (dll_name,
+			       (CORE_ADDR) (uintptr_t) mi.lpBaseOfDll);
+	  return;
+	}
+    }
+}
+#endif
+
 typedef HANDLE (WINAPI *winapi_CreateToolhelp32Snapshot) (DWORD, DWORD);
 typedef BOOL (WINAPI *winapi_Module32First) (HANDLE, LPMODULEENTRY32);
 typedef BOOL (WINAPI *winapi_Module32Next) (HANDLE, LPMODULEENTRY32);
-- 
1.8.1.2