[PATCH] wdscore: Implement CurrentIP.

[Zebediah Figura (she/her)]

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.