[PATCH 4/4] ntdll: Print a warning when LdrGetProcedureAddress fails

[Alex Henrie]

On Sun, Jun 7, 2020 at 11:17 PM Zebediah Figura <z.figura12 at gmail.com> wrote:
>
> On 6/7/20 11:41 PM, Alex Henrie wrote:
> > Signed-off-by: Alex Henrie <alexhenrie24 at gmail.com>
> > ---
> > This will help tremendously with debugging null pointer segfaults.
>
> Seconded wholeheartedly. I've kept a similar patch in my local tree for
> quite a while.
>
> Even worse, applications will detect that a function is missing but
> silently fail...

Glad you approve!

> > diff --git a/dlls/ntdll/loader.c b/dlls/ntdll/loader.c
> > index d1b71efc07..2c37135105 100644
> > --- a/dlls/ntdll/loader.c
> > +++ b/dlls/ntdll/loader.c
> > @@ -1865,6 +1865,17 @@ NTSTATUS WINAPI LdrGetProcedureAddress(HMODULE module, const ANSI_STRING *name,
> >       }
> >
> >       RtlLeaveCriticalSection( &loader_section );
> > +
> > +    if (__WINE_GET_DEBUGGING_WARN( __wine_dbch___default ) && module && (name || ord) && !*address)
>
> Why not just WARN_ON(module)?

I didn't know that you could do that. Thanks for the tip!

> Couldn't we just check for STATUS_PROCEDURE_NOT_FOUND? (I also wouldn't
> think it's worth checking that the parameters are valid.)

Yes, I realized after I sent v1 that the function should be checking
ret instead of address. I put in some argument checks because I didn't
want to see noise from this function when the failure is not a bug in
Wine. Do you see any strong reason to drop the argument checks?

> > +    {
> > +        WCHAR path_buffer[MAX_PATH];
> > +        UNICODE_STRING module_path = { 0, sizeof(path_buffer), path_buffer };
> > +        path_buffer[0] = 0;
> > +        LdrGetDllFullName( module, &module_path );
> > +        WARN( "function %s (ordinal %d) not found in module %s\n",
> > +              wine_dbgstr_a(name ? name->Buffer : NULL), ord, wine_dbgstr_w(path_buffer) );
>
> As long as we're calling get_modref() in this function anyway, if we're
> concerned about printing the module name, I think it makes sense to just
> access the WINE_MODREF directly. That way you wouldn't have to possibly
> truncate the buffer (not that it probably matters).

You're right, I really didn't need to use LdrGetDllFullName. I will
send a new patch that does not depend on it.

-Alex