[PATCH] wdscore: Implement CurrentIP.

[Mohamad Al-Jaf]

> Adding the debug channel declaration should be deferred until the first
> patch that uses it.

I was going to remove it but since the #ifdef guards ends with an unknown
architecture, it would require the debug channel to display the fixme
message, no?

> I am sort of led to wonder, though, is there really a point in trying to
> use this program with builtin wdscore? It sounds like the DLL is rather
> private to the program in question. As long as you're copying the Media
> Creation tool from a windows 10 installation (since I don't see any
> other way to acquire it?) I'd imagine that one should copy native
> wdscore.dll as well.

You can download the tool directly from Microsoft.com and also the archive
link in the wine bug report. But you're right, there's no real point to
using the builtin wdscore. Personally, I'd use the native dll for something
like this. I figured since it was already in the wine tree and the bug
report was accepted, it would be a good learning exercise.

> No, you pretty much have to guess just from figuring out what the
> arguments look like. If you can't tell I'd recommend using "void *" or
> "DWORD_PTR" so that the entire value is captured.

I see, thank you.

>Yes, that looks correct. Note that you don't need the \n\t after the
> last instruction.

Thanks, I figured as much and removed the \n\t afterwards.

> A missing newline at the end of a file is considered a whitespace error;
> I believe Git should warn you about those.

That's odd, I don't think it warned me. Perhaps I need to configure it for
that?

> I'm not an expert in ARM assembly, but I believe you want:
>
>     mov lr, r0
>     bx lr
>
> for ARM, and
>
>     mov lr, x0
>     ret
>
> for ARM64.
>
> The rest looks correct to me.

Thank you so much, I really appreciate all the help you've given me. You're
a great teacher!

> Is there any indication this function should be doing something like
> this at all?

I'm testing it out in Windows to get a sense of what it does. Here's my
code in C++, let me know if it's incorrect:

    typedef int (*CurrentIP)();
    HMODULE hDLL = LoadLibraryA("wdscore.dll");
    CurrentIP ip = (CurrentIP)GetProcAddress(hDLL, "CurrentIP");
    std::cout << "CurrentIP() = " << ip() << "\n";
    FreeLibrary(hDLL);

It returns random numbers, e.g. 10424371, 1249331, 6033459. So doesn't this
mean it's returning the instruction pointer?

If so, I don't know how to write a conformance test for it. Would it be
necessary in this case?

On Thu, Jan 13, 2022 at 11:20 PM Nikolay Sivov <nsivov at codeweavers.com>
wrote:

>
>
> On 1/14/22 02:23, Zebediah Figura (she/her) wrote:
> > On 1/12/22 20:25, Mohamad Al-Jaf wrote:
> >> Hi Zebediah,
> >>
> >> Thanks for the feedback.
> >>
> >> Not sure how to quote in Gmail but I'll add > for each line I reply to.
> >>
> >>> This debug channel is unused, and should generate a warning.
> >>
> >> I can remove it but I plan on adding stub functions to the dll.
> >> CurrentIP
> >> is the first function the Microsoft Media Creation tool calls upon,
> >> ConstructPartialMsgVW after. Not sure how many others it will try to
> >> call.
> >> Based on your method, I think ConstructPartialMsgVW takes 3
> >> arguments, but
> >> I checked it a few weeks ago so I don't remember if this is the exact
> >> number.
> >
> > Adding the debug channel declaration should be deferred until the
> > first patch that uses it.
> >
> > I am sort of led to wonder, though, is there really a point in trying
> > to use this program with builtin wdscore? It sounds like the DLL is
> > rather private to the program in question. As long as you're copying
> > the Media Creation tool from a windows 10 installation (since I don't
> > see any other way to acquire it?) I'd imagine that one should copy
> > native wdscore.dll as well.
> >
> >>
> >> In any case, the debug channel would be used for those arguments. While
> >> we're on this subject, is there a method to determine the function
> >> return
> >> type, argument types and possibly names? I see DWORD and sometimes
> >> void is
> >> used for undocumented functions, would this be appropriate?
> >
> > No, you pretty much have to guess just from figuring out what the
> > arguments look like. If you can't tell I'd recommend using "void *" or
> > "DWORD_PTR" so that the entire value is captured.
> >
> >>
> >>> No reason to make this a separate function; you can use
> >> __ASM_STDCALL_FUNC() to declare it.
> >>
> >> Thanks, is this correct so far?
> >>
> >> __ASM_STDCALL_FUNC(CurrentIP, 0,
> >>      "mov 0(%esp), %eax\n\t"
> >>      "ret\n\t" )
> >
> > Yes, that looks correct. Note that you don't need the \n\t after the
> > last instruction.
> >
> >>
> >>> Please try to avoid whitespace errors.
> >>
> >> I see a lot of files have a newline at the end, I'm assuming a newline
> >> avoids whitespace errors?
> >
> > A missing newline at the end of a file is considered a whitespace
> > error; I believe Git should warn you about those.
> >
> >>
> >>> Giovanni points out that this also won't compile on anything other than
> >> i386. You'll need to use #ifdef guards to handle separate architectures.
> >>
> >> I see, I'm not really sure what to do for the other architectures or
> >> which
> >> ones are appropriate in this context. I'm not familiar with arm
> >> assembly. I
> >> looked at the assembly code in kernelbase/thread.c for reference and
> >> this
> >> is what I have so far:
> >>
> >> #ifdef __i386__
> >> __ASM_STDCALL_FUNC(CurrentIP, 0,
> >>      "movl 0(%esp), %eax\n\t"
> >>      "ret" )
> >> #elif defined(__x86_64__)
> >> __ASM_STDCALL_FUNC(CurrentIP, 0,
> >>      "movq 0(%rsp), %rax\n\t"
> >>      "ret" )
> >> #elif defined(__arm__)
> >> __ASM_STDCALL_FUNC(CurrentIP, 0,
> >>      "mov r13, r0\n\t"
> >>      "ret" )
> >> #elif defined(__aarch64__)
> >> __ASM_STDCALL_FUNC(CurrentIP, 0,
> >>      "mov sp, x0\n\t"
> >>      "ret" )
> >> #else
> >> int WINAPI CurrentIP()
> >> {
> >>      FIXME( "not implemented\n" );
> >>      return 0;
> >> }
> >> #endif
> >>
> >> Is this a good start? What parts are incorrect?
> >
> > I'm not an expert in ARM assembly, but I believe you want:
> >
> >     mov lr, r0
> >     bx lr
> >
> > for ARM, and
> >
> >     mov lr, x0
> >     ret
> >
> > for ARM64.
> >
> > The rest looks correct to me.
> >
> Is there any indication this function should be doing something like
> this at all?
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20220114/5a0f045d/attachment.htm>