Re: [RFA/commit 1/3] Ignore DLL load/unload events during inferior initialization.

[Joel Brobecker <brobecker at adacore dot com>]

> The series looked fine to me, FWIW.  Thanks!

Thanks for reviewing them too!

> On 02/10/2014 10:50 AM, Joel Brobecker wrote:
> > -/* 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.
> 
> I think it's a little unfortunate that we lose this comment (in
> some form), because it explains precisely _why_ we defer reading
> the dll list to afterwards.  I think someone reading the code
> not being familiar with the history will naturally wonder
> about this.

Agreed. In my mind, I had preserved that information, but in looking
at it again, it is, hum, a fairly significantly stripped-down version
of the comment.

Attached is the patch I just checked in.

gdb/ChangeLog:

        * windows-nat.c (handle_unload_dll): Add function documentation.
        (do_initial_windows_stuff): Add comment explaining why we wait
        until after inferior initialization has finished before
        processing all DLLs.

The old comment is resurected at the location where we do the
batch processing of all DLLs, and there are references to it
from both handle_load_dll and handle_unload_dll. Anyone wondering
why we ignore those events when looking at get_windows_debug_event...

    case LOAD_DLL_DEBUG_EVENT:
      [...]
      if (saw_create != 1 || ! windows_initialization_done)
        break;
      catch_errors (handle_load_dll, NULL, (char *) "", RETURN_MASK_ALL);

... can probably search for windows_initialization_done, and
will immediately find do_initial_windows_stuff, where the comment
was added.

Let me know if there are other ways you think I could improve
the documentation further!

-- 
Joel

>From 3be75f87b9a0e5b06175dadedb268c609609c821 Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Thu, 20 Feb 2014 17:18:47 +0100
Subject: [PATCH] windows-nat.c: Bring comment back regarding handling of DLL
 load events.

This patch brings back a comment that got stripped down a bit too much
during a recent change.

gdb/ChangeLog:

        * windows-nat.c (handle_unload_dll): Add function documentation.
        (do_initial_windows_stuff): Add comment explaining why we wait
        until after inferior initialization has finished before
        processing all DLLs.
---
 gdb/ChangeLog     |  7 +++++++
 gdb/windows-nat.c | 23 ++++++++++++++++++++++-
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 2745653..bbb1039 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,12 @@
 2014-02-20  Joel Brobecker  <brobecker@adacore.com>
 
+	* windows-nat.c (handle_unload_dll): Add function documentation.
+	(do_initial_windows_stuff): Add comment explaining why we wait
+	until after inferior initialization has finished before
+	processing all DLLs.
+
+2014-02-20  Joel Brobecker  <brobecker@adacore.com>
+
 	* windows-nat.c (get_module_name): Delete.
 	(windows_get_exec_module_filename): New function, mostly
 	inspired from get_module_name.
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index a570a1a..4366aab 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -802,6 +802,14 @@ windows_free_so (struct so_list *so)
   xfree (so);
 }
 
+/* Handle a DLL unload event.
+   Return 1 if successful, or zero otherwise.
+
+   This function assumes that this event did not occur during inferior
+   initialization, where their event info may be incomplete (see
+   do_initial_windows_stuff and windows_add_all_dlls for more info
+   on how we handle DLL loading during that phase).  */
+
 static int
 handle_unload_dll (void *dummy)
 {
@@ -1735,7 +1743,20 @@ do_initial_windows_stuff (struct target_ops *ops, DWORD pid, int attaching)
     }
 
   /* Now that the inferior has been started and all DLLs have been mapped,
-     we can iterate over all DLLs and load them in.  */
+     we can iterate over all DLLs and load them in.
+
+     We avoid doing it any earlier because, on certain versions of Windows,
+     LOAD_DLL_DEBUG_EVENTs are sometimes not complete.  In particular,
+     we have seen on Windows 8.1 that the ntdll.dll load event does not
+     include the DLL name, preventing us from creating an associated SO.
+     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.
+
+     Rather than try to work around this sort of issue, it is much
+     simpler to just ignore DLL load/unload events during the startup
+     phase, and then process them all in one batch now.  */
   windows_add_all_dlls ();
 
   windows_initialization_done = 1;
-- 
1.8.3.2