[PATCH v2] wdscore: Implement CurrentIP.

[Mohamad Al-Jaf]

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=51850
Signed-off-by: Mohamad Al-Jaf 
---
v2: -Add architecture-specific assembly (Credit: Zebediah Figura)
    -Update copyright year
    -Add basic unit test
    -Explicitly declare function takes no parameters
Note: I compared the native wdscore to this one in both
Wine and Windows. The functions behaved identically.
---
 configure.ac                   |  1 +
 dlls/wdscore/Makefile.in       |  3 ++
 dlls/wdscore/main.c            | 55 ++++++++++++++++++++++++++++++++++
 dlls/wdscore/tests/Makefile.in |  5 ++++
 dlls/wdscore/tests/main.c      | 37 +++++++++++++++++++++++
 dlls/wdscore/wdscore.spec      |  2 +-
 6 files changed, 102 insertions(+), 1 deletion(-)
 create mode 100644 dlls/wdscore/main.c
 create mode 100644 dlls/wdscore/tests/Makefile.in
 create mode 100644 dlls/wdscore/tests/main.c

diff --git a/configure.ac b/configure.ac
index b42cb8a5346..a50881cc2fb 100644
--- a/configure.ac
+++ b/configure.ac
@@ -3347,6 +3347,7 @@ WINE_CONFIG_MAKEFILE(dlls/wbemdisp/tests)
 WINE_CONFIG_MAKEFILE(dlls/wbemprox)
 WINE_CONFIG_MAKEFILE(dlls/wbemprox/tests)
 WINE_CONFIG_MAKEFILE(dlls/wdscore)
+WINE_CONFIG_MAKEFILE(dlls/wdscore/tests)
 WINE_CONFIG_MAKEFILE(dlls/webservices)
 WINE_CONFIG_MAKEFILE(dlls/webservices/tests)
 WINE_CONFIG_MAKEFILE(dlls/websocket)
diff --git a/dlls/wdscore/Makefile.in b/dlls/wdscore/Makefile.in
index 20ba1d3b1c9..2020e72c7bb 100644
--- a/dlls/wdscore/Makefile.in
+++ b/dlls/wdscore/Makefile.in
@@ -1,3 +1,6 @@
 MODULE    = wdscore.dll

EXTRADLLFLAGS = -Wb,--prefer-native
+
+C_SRCS = \
+	main.c
diff --git a/dlls/wdscore/main.c b/dlls/wdscore/main.c
new file mode 100644
index 00000000000..1ed860e02ea
--- /dev/null
+++ b/dlls/wdscore/main.c
@@ -0,0 +1,55 @@
+/*
+ * Copyright 2022 Mohamad Al-Jaf
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA
+ */
+
+#include <stdarg.h>
+#include <stdint.h>
+
+#include "windef.h"
+#include "winbase.h"
+
+#include "wine/asm.h"
+#include "wine/debug.h"
+
+WINE_DEFAULT_DEBUG_CHANNEL(wdscore);
+
+/***********************************************************************
+ *           CurrentIP (wdscore.@)
+ */
+#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 lr, r0\n\t"
+    "bx lr" )
+#elif defined(__aarch64__)
+__ASM_STDCALL_FUNC(CurrentIP, 0,
+    "mov lr, x0\n\t"
+    "ret" )
+#else
+int WINAPI CurrentIP(void)
+{
+    FIXME( "not implemented\n" );
+    return 0;
+}
+#endif
diff --git a/dlls/wdscore/tests/Makefile.in b/dlls/wdscore/tests/Makefile.in
new file mode 100644
index 00000000000..f0606d902d5
--- /dev/null
+++ b/dlls/wdscore/tests/Makefile.in
@@ -0,0 +1,5 @@
+TESTDLL   = wdscore.dll
+IMPORTS   = wdscore
+
+C_SRCS = \
+	main.c
diff --git a/dlls/wdscore/tests/main.c b/dlls/wdscore/tests/main.c
new file mode 100644
index 00000000000..262c75641af
--- /dev/null
+++ b/dlls/wdscore/tests/main.c
@@ -0,0 +1,37 @@
+/*
+ * Unit test suite for wdscore
+ *
+ * Copyright 2022 Mohamad Al-Jaf
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA
+ *
+ */
+
+#include "wine/test.h"
+#include "winbase.h"
+
+static void test_CurrentIP(void)
+{
+    int ret;
+
+    ret = CurrentIP();
+    trace("CurrentIP() = %d\n", ret);
+    ok(ret != 0, "Unsupported architecture\n");
+}
+
+START_TEST(main)
+{
+    test_CurrentIP();
+}
diff --git a/dlls/wdscore/wdscore.spec b/dlls/wdscore/wdscore.spec
index 15958b86aba..8b6febe6b3b 100644
--- a/dlls/wdscore/wdscore.spec
+++ b/dlls/wdscore/wdscore.spec
@@ -71,7 +71,7 @@
 @ stub ConstructPartialMsgIfW
 @ stub ConstructPartialMsgVA
 @ stub ConstructPartialMsgVW
-@ stub CurrentIP
+@ stdcall CurrentIP()
 @ stub EndMajorTask
 @ stub EndMinorTask
 @ stub GetMajorTask

[Giovanni Mascellani]

Hi,

Il 14/01/22 12:28, Mohamad Al-Jaf ha scritto:
> +/***********************************************************************
> -
> -
>       CurrentIP (wdscore.@)
> - */
>
> +#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 lr, r0\n\t"
> - "bx lr" )
>
> +#elif defined(__aarch64__)
> +__ASM_STDCALL_FUNC(CurrentIP, 0,
> - "mov lr, x0\n\t"
> - "ret" )
>
> +#else
> +int WINAPI CurrentIP(void)
> +{
> - FIXME( "not implemented\n" );
> - return 0;
>
> +}
> +#endif

Given your implementation, it seems more likely that CurrentIP returns 
void *, instead of int (which would get the pointer truncated, e.g., on 
i386). This is also hinted by this snippet that was mentioned in the bug:

https://github.com/seven-mile/CallCbsCore/blob/6465d9c6801768c56c7ca1faebc65...
> +static void test_CurrentIP(void)
> +{
> - int ret;
> -
> - ret = CurrentIP();
> - trace("CurrentIP() = %d\n", ret);
> - ok(ret != 0, "Unsupported architecture\n");
>
> +}

Same here. Notice that CurrentIP is not declared here, so the compiler 
defaults to the terrible fallback of believing it returns int, though I 
don't think this is appropriate here. And you should declare it anyway. 
I don't know if there is a header in which it is sensible to put the 
declaration.

But just tracing the result is not very useful for testing. Given that 
your hypothesis seems to be that CurrentIP returns the return address of 
the call, you could try to check that such address is in the caller 
function. There is no portable way to do that, but a reasonable attempt 
is to check that ret is higher than &test_CurrentIP and lower than, say, 
&test_CurrentIP + 0x100. I did a quick test and this seems indeed to be 
the case:

https://testbot.winehq.org/JobDetails.pl?Key=105255

Giovanni.

[Jinoh Kang]

On 1/14/22 20:28, Mohamad Al-Jaf wrote:
> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=51850
> Signed-off-by: Mohamad Al-Jaf
>
> v2: -Add architecture-specific assembly (Credit: Zebediah Figura)
>     -Update copyright year
>     -Add basic unit test
>     -Explicitly declare function takes no parameters
> Note: I compared the native wdscore to this one in both
> Wine and Windows. The functions behaved identically.
>
> configure.ac                   |  1 +
>  dlls/wdscore/Makefile.in       |  3 ++
>  dlls/wdscore/main.c            | 55 ++++++++++++++++++++++++++++++++++
>  dlls/wdscore/tests/Makefile.in |  5 ++++
>  dlls/wdscore/tests/main.c      | 37 +++++++++++++++++++++++
>  dlls/wdscore/wdscore.spec      |  2 +-
>  6 files changed, 102 insertions(+), 1 deletion(-)
>  create mode 100644 dlls/wdscore/main.c
>  create mode 100644 dlls/wdscore/tests/Makefile.in
>  create mode 100644 dlls/wdscore/tests/main.c
>
> diff --git a/configure.ac b/configure.ac
> index b42cb8a5346..a50881cc2fb 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -3347,6 +3347,7 @@ WINE_CONFIG_MAKEFILE(dlls/wbemdisp/tests)
>  WINE_CONFIG_MAKEFILE(dlls/wbemprox)
>  WINE_CONFIG_MAKEFILE(dlls/wbemprox/tests)
>  WINE_CONFIG_MAKEFILE(dlls/wdscore)
> +WINE_CONFIG_MAKEFILE(dlls/wdscore/tests)
>  WINE_CONFIG_MAKEFILE(dlls/webservices)
>  WINE_CONFIG_MAKEFILE(dlls/webservices/tests)
>  WINE_CONFIG_MAKEFILE(dlls/websocket)
> diff --git a/dlls/wdscore/Makefile.in b/dlls/wdscore/Makefile.in
> index 20ba1d3b1c9..2020e72c7bb 100644
> --- a/dlls/wdscore/Makefile.in
> +++ b/dlls/wdscore/Makefile.in
> @@ -1,3 +1,6 @@
>  MODULE    = wdscore.dll
>
> EXTRADLLFLAGS = -Wb,--prefer-native
> -
>
> +C_SRCS = \
> - main.c
>
> diff --git a/dlls/wdscore/main.c b/dlls/wdscore/main.c
> new file mode 100644
> index 00000000000..1ed860e02ea
> --- /dev/null
> +++ b/dlls/wdscore/main.c
> @@ -0,0 +1,55 @@
> +/*
> -
> - Copyright 2022 Mohamad Al-Jaf
> -
> -
> -
> - This library is free software; you can redistribute it and/or
> -
> - modify it under the terms of the GNU Lesser General Public
> -
> - License as published by the Free Software Foundation; either
> -
> - version 2.1 of the License, or (at your option) any later version.
> -
> -
> -
> - This library is distributed in the hope that it will be useful,
> -
> - but WITHOUT ANY WARRANTY; without even the implied warranty of
> -
> - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> -
> - Lesser General Public License for more details.
> -
> -
> -
> - You should have received a copy of the GNU Lesser General Public
> -
> - License along with this library; if not, write to the Free Software
> -
> - Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA
> - */
> -
>
> +#include <stdarg.h>
> +#include <stdint.h>
> -
>
> +#include "windef.h"
> +#include "winbase.h"
> -
>
> +#include "wine/asm.h"
> +#include "wine/debug.h"
> -
>
> +WINE_DEFAULT_DEBUG_CHANNEL(wdscore);
> -
>
> +/***********************************************************************
> -
> -
>       CurrentIP (wdscore.@)
> - */
>
> +#ifdef __i386__
> +__ASM_STDCALL_FUNC(CurrentIP, 0,
> - "movl 0(%esp), %eax\n\t"

You can get rid of "0".
> - "ret" )
>
> +#elif defined(__x86_64__)
> +__ASM_STDCALL_FUNC(CurrentIP, 0,
> - "movq 0(%rsp), %rax\n\t"

Ditto.
> - "ret" )
>
> +#elif defined(__arm__)
> +__ASM_STDCALL_FUNC(CurrentIP, 0,
> - "mov lr, r0\n\t"

Operands are swapped. This is not AT&T syntax. Should be "mov r0, lr".
> - "bx lr" )
>
> +#elif defined(__aarch64__)
> +__ASM_STDCALL_FUNC(CurrentIP, 0,
> - "mov lr, x0\n\t"

Also swapped.
> - "ret" )
>
> +#else
> +int WINAPI CurrentIP(void)

As the other reviewer pointed out, use void * as the return address.
> +{
> - FIXME( "not implemented\n" );
> - return 0;

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.
> +}
> +#endif
> diff --git a/dlls/wdscore/tests/Makefile.in b/dlls/wdscore/tests/Makefile.in
> new file mode 100644
> index 00000000000..f0606d902d5
> --- /dev/null
> +++ b/dlls/wdscore/tests/Makefile.in
> @@ -0,0 +1,5 @@
> +TESTDLL   = wdscore.dll
> +IMPORTS   = wdscore
> -
>
> +C_SRCS = \
> - main.c
>
> diff --git a/dlls/wdscore/tests/main.c b/dlls/wdscore/tests/main.c
> new file mode 100644
> index 00000000000..262c75641af
> --- /dev/null
> +++ b/dlls/wdscore/tests/main.c
> @@ -0,0 +1,37 @@
> +/*
> -
> - Unit test suite for wdscore
> -
> -
> -
> - Copyright 2022 Mohamad Al-Jaf
> -
> -
> -
> - This library is free software; you can redistribute it and/or
> -
> - modify it under the terms of the GNU Lesser General Public
> -
> - License as published by the Free Software Foundation; either
> -
> - version 2.1 of the License, or (at your option) any later version.
> -
> -
> -
> - This library is distributed in the hope that it will be useful,
> -
> - but WITHOUT ANY WARRANTY; without even the implied warranty of
> -
> - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> -
> - Lesser General Public License for more details.
> -
> -
> -
> - You should have received a copy of the GNU Lesser General Public
> -
> - License along with this library; if not, write to the Free Software
> -
> - Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA
> -
> -
> - */
> -
>
> +#include "wine/test.h"
> +#include "winbase.h"
> -
>
> +static void test_CurrentIP(void)
> +{
> - int ret;

Use void *.
> -
> - ret = CurrentIP();
> - trace("CurrentIP() = %d\n", ret);

s/%d/%p/
> - ok(ret != 0, "Unsupported architecture\n");
>
> +}
> -
>
> +START_TEST(main)
> +{
> - test_CurrentIP();
>
> +}
> diff --git a/dlls/wdscore/wdscore.spec b/dlls/wdscore/wdscore.spec
> index 15958b86aba..8b6febe6b3b 100644
> --- a/dlls/wdscore/wdscore.spec
> +++ b/dlls/wdscore/wdscore.spec
> @@ -71,7 +71,7 @@
>  @ stub ConstructPartialMsgIfW
>  @ stub ConstructPartialMsgVA
>  @ stub ConstructPartialMsgVW
> -@ stub CurrentIP
> +@ stdcall CurrentIP()
>  @ stub EndMajorTask
>  @ stub EndMinorTask
>  @ stub GetMajorTask