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

[Pedro Alves <palves at redhat dot com>]

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