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

[Joel Brobecker <brobecker at adacore dot com>]

> 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

>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