[PATCH] wdscore: Implement CurrentIP.

[Nikolay Sivov]

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?