[PATCH] wdscore: Implement CurrentIP.

[Giovanni Mascellani]

Hi,

Il 17/01/22 03:55, Mohamad Al-Jaf ha scritto:
> Thank you, that's a great way to check for it. Here's what I have in the 
> test, are these error messages good?
> 
> static void test_CurrentIP(void)
> {
>      void *cur;
>      void *ret;
> 
>      cur = &test_CurrentIP;
>      ret = CurrentIP();
> 
>      ok(ret != 0, "Unsupported architecture\n");

This is redundant, given the following two.

>      ok(ret > cur, "&test_CurrentIP=%p CurrentIP()=%p\n", cur, ret);
>      ok(ret < (cur + 0x100), "&test_CurrentIP + 0x100=%p 
> CurrentIP()=%p\n", cur + 0x100, ret);
> }

Usually messages are a bit more descriptive. I would go for something like:

ok(cur <= ret && ret < (cur + 0x100), "Address %p not in function 
starting at %p.\n", ret, cur);

Though notice that in C it is illegal to add a number to a void*, so 
this line will raise some warnings. Convert pointers to char* or to 
uintptr_t to avoid the warnings.

>  > Let me suggest an alternative: instead of assembly, we can just use 
> GCC's built-in functions here for all architectures.
>  >
>  > #ifdef __has_builtin
>  > #if __has_builtin(__builtin_extract_return_addr)
>  > #define extract_retaddr(x) __builtin_extract_return_addr(x)
>  > #endif
>  > #elif defined(__GNUC__) && __GNUC__ >= 5
>  > #define extract_retaddr(x) __builtin_extract_return_addr(x)
>  > #else
>  > #define extract_retaddr(x) (x)
>  > #endif
>  >
>  > void *WINAPI CurrentIP(void)
>  > {
>  >     return extract_retaddr( __builtin_return_address( 0 ));
>  > }
>  >
>  > Also this means the debug channel is (again) no longer needed.
> 
> Thank you, this looks great. But according to the GNU documentation 
> __builtin_return_address is processor specific and might return 0 or 
> worse, a random value: 
> https://gcc.gnu.org/onlinedocs/gcc-4.1.2/gcc/Return-Address.html 
> <https://gcc.gnu.org/onlinedocs/gcc-4.1.2/gcc/Return-Address.html>

My interpretation of that page is that calling 
__builtin_return_address(0) is always fine, it is for higher frames that 
it is not necessarily reliable. Still, it is a GCC-specific thing, so 
alternatives should be provided for other compilers.

Giovanni.