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

[Pedro Alves <palves at redhat dot com>]

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