Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=51850 Signed-off-by: Mohamad Al-Jaf mohamadaljaf@gmail.com --- v4: - Remove WINAPI in the function in main.c and tests/main.c.
Since WINAPI cleans the stack, it doesn't make sense to use it in this context. --- configure.ac | 1 + dlls/wdscore/Makefile.in | 3 ++ dlls/wdscore/main.c | 54 ++++++++++++++++++++++++++++ dlls/wdscore/tests/Makefile.in | 4 +++ dlls/wdscore/tests/main.c | 65 ++++++++++++++++++++++++++++++++++ dlls/wdscore/wdscore.spec | 2 +- 6 files changed, 128 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 0b1a53f3ff9..1923c95f4b4 100644 --- a/configure.ac +++ b/configure.ac @@ -3345,6 +3345,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..9001838431d --- /dev/null +++ b/dlls/wdscore/main.c @@ -0,0 +1,54 @@ +/* + * 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 "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 (%esp), %eax\n\t" + "ret" ) +#elif defined(__x86_64__) +__ASM_STDCALL_FUNC(CurrentIP, 0, + "movq (%rsp), %rax\n\t" + "ret" ) +#elif defined(__arm__) +__ASM_STDCALL_FUNC(CurrentIP, 0, + "mov r0, lr\n\t" + "bx lr" ) +#elif defined(__aarch64__) +__ASM_STDCALL_FUNC(CurrentIP, 0, + "mov x0, lr\n\t" + "ret" ) +#else +void *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..baf4adf23a1 --- /dev/null +++ b/dlls/wdscore/tests/Makefile.in @@ -0,0 +1,4 @@ +TESTDLL = wdscore.dll + +C_SRCS = \ + main.c diff --git a/dlls/wdscore/tests/main.c b/dlls/wdscore/tests/main.c new file mode 100644 index 00000000000..f17abbbdb3f --- /dev/null +++ b/dlls/wdscore/tests/main.c @@ -0,0 +1,65 @@ +/* + * 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 <stdarg.h> + +#include "windef.h" +#include "winbase.h" + +#include "wine/test.h" + +static HMODULE dll; +static void* (*pCurrentIP)(void); + +static BOOL init_function_pointers(void) +{ + dll = LoadLibraryA("wdscore.dll"); + + if (dll) + { + pCurrentIP = (void*)GetProcAddress(dll, "CurrentIP"); + return TRUE; + } + + return FALSE; +} + +static void test_CurrentIP(void) +{ + char *cur; + char *ret; + + cur = (char*)&test_CurrentIP; + ret = (char*)pCurrentIP(); + + ok(cur <= ret && ret < (cur + 0x100), "Address %p not in function starting at %p.\n", ret, cur); +} + +START_TEST(main) +{ + if (init_function_pointers()) + { + test_CurrentIP(); + FreeLibrary(dll); + } + else + skip("could not load wdscore.dll\n"); +} 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
Signed-off-by: Mohamad Al-Jaf mohamadaljaf@gmail.com --- dlls/wdscore/wdscore.spec | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/dlls/wdscore/wdscore.spec b/dlls/wdscore/wdscore.spec index 8b6febe6b3b..282b6301688 100644 --- a/dlls/wdscore/wdscore.spec +++ b/dlls/wdscore/wdscore.spec @@ -116,7 +116,7 @@ @ stub WdsGetBlackboardValue @ stub WdsGetCurrentExecutionGroup @ stub WdsGetSetupLog -@ stub WdsTempDir +@ stub WdsGetTempDir @ stub WdsInitialize @ stub WdsInitializeCallbackArray @ stub WdsInitializeDataBinary @@ -139,7 +139,7 @@ @ stub WdsPackCollection @ stub WdsPublish @ stub WdsPublishEx -@ stub WdsPublishImmediateAsynch +@ stub WdsPublishImmediateAsync @ stub WdsPublishImmediateEx @ stub WdsPublishOffline @ stub WdsSeqAlloc @@ -150,8 +150,8 @@ @ stub WdsSetUILanguage @ stub WdsSetupLogDestroy @ stub WdsSetupLogInit -@ stub WdsSetLogMessageA -@ stub WdsSetLogMessageW +@ stub WdsSetupLogMessageA +@ stub WdsSetupLogMessageW @ stub WdsSubscribeEx @ stub WdsTerminate @ stub WdsUnlockExecutionGroup
Enables the Windows MediaCreationTool21H2 to run.
Signed-off-by: Mohamad Al-Jaf mohamadaljaf@gmail.com --- The debug channel emits a rather long string for msg: fixme:wdscore:ConstructPartialMsgVW 67108864 L"\5343\7465\7075\614c\676e\6e49\4c69\616f\6564\3a72\ 493a\696e\6974\6c61\7a69\3a65\4c20\4e41\2e47\4e49\2049\ 6944\6572\7463\726f\2079\203d\7325" - stub
Should I get rid of it? --- dlls/wdscore/main.c | 22 ++++++++++++++++++++++ dlls/wdscore/wdscore.spec | 4 ++-- dlls/wdscore/wdscore_internal.h | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 57 insertions(+), 2 deletions(-) create mode 100644 dlls/wdscore/wdscore_internal.h
diff --git a/dlls/wdscore/main.c b/dlls/wdscore/main.c index 9001838431d..6f054cdd63b 100644 --- a/dlls/wdscore/main.c +++ b/dlls/wdscore/main.c @@ -24,6 +24,8 @@ #include "wine/asm.h" #include "wine/debug.h"
+#include "wdscore_internal.h" + WINE_DEFAULT_DEBUG_CHANNEL(wdscore);
/*********************************************************************** @@ -52,3 +54,23 @@ void *CurrentIP(void) return 0; } #endif + + +/*********************************************************************** + * ConstructPartialMsgVA (wdscore.@) + */ +LPVOID WINAPI ConstructPartialMsgVA( WdsLogLevel level, LPCSTR msg, va_list args ) +{ + FIXME( "%u %s - stub\n", level, debugstr_a(msg) ); + return NULL; +} + + +/*********************************************************************** + * ConstructPartialMsgVW (wdscore.@) + */ +LPVOID WINAPI ConstructPartialMsgVW( WdsLogLevel level, LPCWSTR msg, va_list args ) +{ + FIXME( "%u %s - stub\n", level, debugstr_w(msg) ); + return NULL; +} diff --git a/dlls/wdscore/wdscore.spec b/dlls/wdscore/wdscore.spec index 282b6301688..b14778001a8 100644 --- a/dlls/wdscore/wdscore.spec +++ b/dlls/wdscore/wdscore.spec @@ -69,8 +69,8 @@ #@ extern g_bEnableDiagnosticMode @ stub ConstructPartialMsgIfA @ stub ConstructPartialMsgIfW -@ stub ConstructPartialMsgVA -@ stub ConstructPartialMsgVW +@ stdcall ConstructPartialMsgVA(long str ptr) +@ stdcall ConstructPartialMsgVW(long wstr ptr) @ stdcall CurrentIP() @ stub EndMajorTask @ stub EndMinorTask diff --git a/dlls/wdscore/wdscore_internal.h b/dlls/wdscore/wdscore_internal.h new file mode 100644 index 00000000000..b5cbe3c8c25 --- /dev/null +++ b/dlls/wdscore/wdscore_internal.h @@ -0,0 +1,33 @@ +/* + * 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 + */ + +#ifndef __WDSCORE_INTERNAL_H +#define __WDSCORE_INTERNAL_H + +typedef enum _WdsLogLevel { + WdsLogLevelAssert = 0x0000000, + WdsLogLevelFatalError = 0x1000000, + WdsLogLevelError = 0x2000000, + WdsLogLevelWarning = 0x3000000, + WdsLogLevelInfo = 0x4000000, + WdsLogLevelStatus = 0x5000000, + WdsLogLevelVerbose = 0x6800000, + WdsLogLevelTrace = 0x7000000 +} WdsLogLevel; + +#endif /* __WDSCORE_INTERNAL_H */
Hi,
While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check?
Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=106260
Your paranoid android.
=== debian11 (build log) ===
error: patch failed: configure.ac:3347 error: patch failed: dlls/wdscore/Makefile.in:1 error: patch failed: dlls/wdscore/wdscore.spec:71 Task: Patch failed to apply
=== debian11 (build log) ===
error: patch failed: configure.ac:3347 error: patch failed: dlls/wdscore/Makefile.in:1 error: patch failed: dlls/wdscore/wdscore.spec:71 Task: Patch failed to apply
Required by the Windows MediaCreationTool21H2.
Signed-off-by: Mohamad Al-Jaf mohamadaljaf@gmail.com --- v4: - Add missing argument - Rename one more unknown --- dlls/wdscore/main.c | 28 ++++++++++++++++++++++++++++ dlls/wdscore/wdscore.spec | 4 ++-- dlls/wdscore/wdscore_internal.h | 27 +++++++++++++++++++++++++++ 3 files changed, 57 insertions(+), 2 deletions(-)
diff --git a/dlls/wdscore/main.c b/dlls/wdscore/main.c index 6f054cdd63b..e4d28b9301a 100644 --- a/dlls/wdscore/main.c +++ b/dlls/wdscore/main.c @@ -74,3 +74,31 @@ LPVOID WINAPI ConstructPartialMsgVW( WdsLogLevel level, LPCWSTR msg, va_list arg FIXME( "%u %s - stub\n", level, debugstr_w(msg) ); return NULL; } + + +/*********************************************************************** + * WdsSetupLogMessageA (wdscore.@) + */ +HRESULT WINAPI WdsSetupLogMessageA( LPVOID msg, WdsLogSource src, LPCSTR unknown1, LPCSTR unknown2, + ULONG unknown3, LPCSTR file, LPCSTR func, void *ip, + ULONG unknown4, void *unknown5, UINT unknown6 ) +{ + FIXME( "%p, %u, %s, %s, %u, %s, %s, %p, %u, %p, %u - stub\n", msg, src, debugstr_a(unknown1), + debugstr_a(unknown2), unknown3, debugstr_a(file), debugstr_a(func), ip, unknown4, + unknown5, unknown6 ); + return S_OK; +} + + +/*********************************************************************** + * WdsSetupLogMessageW (wdscore.@) + */ +HRESULT WINAPI WdsSetupLogMessageW( LPVOID msg, WdsLogSource src, LPCWSTR unknown1, LPCWSTR unknown2, + ULONG unknown3, LPCWSTR file, LPCWSTR func, void *ip, + ULONG unknown4, void *unknown5, UINT unknown6 ) +{ + FIXME( "%p, %u, %s, %s, %u, %s, %s, %p, %u, %p, %u - stub\n", msg, src, debugstr_w(unknown1), + debugstr_w(unknown2), unknown3, debugstr_w(file), debugstr_w(func), ip, unknown4, + unknown5, unknown6 ); + return S_OK; +} diff --git a/dlls/wdscore/wdscore.spec b/dlls/wdscore/wdscore.spec index b14778001a8..7ec8025ddee 100644 --- a/dlls/wdscore/wdscore.spec +++ b/dlls/wdscore/wdscore.spec @@ -150,8 +150,8 @@ @ stub WdsSetUILanguage @ stub WdsSetupLogDestroy @ stub WdsSetupLogInit -@ stub WdsSetupLogMessageA -@ stub WdsSetupLogMessageW +@ stdcall WdsSetupLogMessageA(ptr long str str long str str ptr long ptr long) +@ stdcall WdsSetupLogMessageW(ptr long wstr wstr long wstr wstr ptr long ptr long) @ stub WdsSubscribeEx @ stub WdsTerminate @ stub WdsUnlockExecutionGroup diff --git a/dlls/wdscore/wdscore_internal.h b/dlls/wdscore/wdscore_internal.h index b5cbe3c8c25..84a17d5fbc0 100644 --- a/dlls/wdscore/wdscore_internal.h +++ b/dlls/wdscore/wdscore_internal.h @@ -30,4 +30,31 @@ typedef enum _WdsLogLevel { WdsLogLevelTrace = 0x7000000 } WdsLogLevel;
+typedef enum _WdsLogSource { + WdsLogSourceDPX = 0x1000000, + WdsLogSourceCBS = 0x2000000, + WdsLogSourceCSI = 0x1800000, + WdsLogSourceSXS = 0x2800000, + WdsLogSourceCMI = 0x3000000, + WdsLogSourceDEPLOY = 0x4000000, + WdsLogSourceDU = 0x5000000, + WdsLogSourceIBS = 0x6000000, + WdsLogSourceIBSLIB = 0x6400000, + WdsLogSourceDIAG = 0x7000000, + WdsLogSourceDIAGER = 0x7400000, + WdsLogSourceMIG = 0x8000000, + WdsLogSourceHWARE = 0x8400000, + WdsLogSourceMIGUI = 0x8800000, + WdsLogSourceUI = 0xA000000, + WdsLogSourceCONX = 0xA400000, + WdsLogSourceMOUPG = 0xA800000, + WdsLogSourceWDS = 0xB000000, + WdsLogSourceDISM = 0xB800000, + WdsLogSourcePANTHR = 0x9000000, + WdsLogSourceWINPE = 0xC000000, + WdsLogSourceSP = 0xC800000, + WdsLogSourceLIB = 0xD000000, + WdsLogSourceTOOL = 0xE000000 +} WdsLogSource; + #endif /* __WDSCORE_INTERNAL_H */
Hi,
While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check?
Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=106261
Your paranoid android.
=== debian11 (build log) ===
error: patch failed: configure.ac:3347 error: patch failed: dlls/wdscore/Makefile.in:1 error: patch failed: dlls/wdscore/wdscore.spec:71 error: patch failed: dlls/wdscore/wdscore.spec:150 Task: Patch failed to apply
=== debian11 (build log) ===
error: patch failed: configure.ac:3347 error: patch failed: dlls/wdscore/Makefile.in:1 error: patch failed: dlls/wdscore/wdscore.spec:71 error: patch failed: dlls/wdscore/wdscore.spec:150 Task: Patch failed to apply
This is supposed to be v5, not v4. I always check the patches on my machine and send the patches to my email to check again. This time I waited an hour before submitting and checked it again before submitting. And yet I still made a mistake, it's very frustrating. I guess I got confused with the previous v3 revision of wdscore: Add stubs for WdsSetupLogMessage[AW] and mistakenly updated all of them to v4.
Terribly sorry about that. I don't know why I keep making mistakes like these.
Hi,
Sorry to bother you all. Just wanted to get your input on this.
Because CurrentIP isn't WINAPI, shouldn't __ASM_GLOBAL_FUNC be used instead of __ASM_STDCALL_FUNC? Since stdcall cleans the stack, wouldn't the stack pointer change? Or is my understanding incorrect?
Hi,
Il 28/01/22 00:20, Mohamad Al-Jaf ha scritto:
Since WINAPI cleans the stack, it doesn't make sense to use it in this context.
Calling conventions are not just about cleaning the stack, though it's true that for a function that takes no argument and just return a pointer most of the differences make little sense. Name mangling is something that (on i386) changes between WINAPI and CDECL, but I don't know how that appears in DLL linking, so I don't know how to help you.
BTW, you mention bug https://bugs.winehq.org/show_bug.cgi?id=51850 in your patch description. Does the patch solve the bug? Is there any different if you use STDCALL/WINAPI or CDECL? If there is, choose the alternative that fixes the application. If they are the same, I guess I'd choose WINAPI.
Giovanni.
Hi Giovanni,
I see, thanks for the clarification.
Yes, the patch solves the bug. Well, for the tests removing WINAPI seems to change the pointer a bit, but it's still within the range so it doesn't seem to affect it.
I haven't tried it with __ASM_GLOBAL_FUNC, but it seems to work fine as is. Along with the stubs in this patchset, wdscore functions similarly to the native version, at least for the media creation tool.
Still, I'm just wondering if using __ASM_STDCALL_FUNC isn't accurate? In the CallCbsCore headers, CurrentIP is defined without WINAPI.[1]
I don't know what to do now. If it's okay to use WINAPI, should I resubmit the older version of this patch?
[1] https://github.com/seven-mile/CallCbsCore/blob/master/StackManager.h
* On 2022-01-29 06:35, Mohamad Al-Jaf wrote:
Yes, the patch solves the bug. Well, for the tests removing WINAPI seems to change the pointer a bit, but it's still within the range so it doesn't seem to affect it.
...
I don't know what to do now. If it's okay to use WINAPI, should I resubmit the older version of this patch?
My 2c€ worth idea is to adhere with the K.I.S.S. principle: just keep the current version and let the function naturally be tested as much as possible.
In case there occurs some bug eg. one containing stack overflow in the app (or something else) related to possibly incorrect use of CurrentIP(), this would be good stimuli for enhancing the implementation (and the tests). I continue below.
* On 2022-01-21 01:03, you also asked Dmitry:
the test doesn't verify that returned
value belongs to the dll address range
Could you explain this?
As Dmitry seemingly haven't responded to the list, I remind about this by sharing my understanding.
Every process has its own virtual address space (VAS) [1].
The .exe file and each linked dll is mapped into a VAS and is given its own address (so-called ImageBase) and its size here (during the load time). Image base and size can be seen in the output of programs like ListDlls [2], CurrProcess [3].
The same pair or numbers can be calculated for every section of a dll (green boxes in the "Memory Map" pic [4]) or at least for executable sections only (red boxes in Figure 2 in [5]).
It also can be done for every function exported by a dll [6].
Dmitry probably wanted to see whether the returned address belong:
- to any of "Dynamic-base DLLs"; [1] - to the .text section of them; - to the the whole .exe file; - to the .text section of it; - to specifically the wdscore.dll; - to the .text section of it; - to the specific function of specific image; - to the stack of some (main) thread of the process; [1]
This would require enumerating dlls mapped into the test process and extracting their image bases and sizes at least.
I might perfectly be wrong with my guess. But at least it's a start (for the next step into advancing the test).
S.
[1] https://github.com/LordNoteworthy/windows-internals/blob/master/windows-inte... [2] https://docs.microsoft.com/en-us/gaming/playfab/features/multiplayer/servers... [3] https://www.nirsoft.net/utils/cprocess.html [4] https://blog.malwarebytes.com/threat-analysis/2013/06/my-memory-isnt-what-it... [5] https://www.cyberark.com/resources/threat-research-blog/masking-malicious-me... [6] https://www.codeproject.com/Articles/86215/Remote-Threads-Basics-Part-1#:~:t...
On 29/01/2022 15:02, Saulius Krasuckas wrote:
- On 2022-01-29 06:35, Mohamad Al-Jaf wrote:
Yes, the patch solves the bug. Well, for the tests removing WINAPI seems to change the pointer a bit, but it's still within the range so it doesn't seem to affect it.
...
I don't know what to do now. If it's okay to use WINAPI, should I resubmit the older version of this patch?
My 2c€ worth idea is to adhere with the K.I.S.S. principle: just keep the current version and let the function naturally be tested as much as possible.
In case there occurs some bug eg. one containing stack overflow in the app (or something else) related to possibly incorrect use of CurrentIP(), this would be good stimuli for enhancing the implementation (and the tests). I continue below.
- On 2022-01-21 01:03, you also asked Dmitry:
the test doesn't verify that returned
value belongs to the dll address range
Could you explain this?
As Dmitry seemingly haven't responded to the list, I remind about this by sharing my understanding.
Every process has its own virtual address space (VAS) [1].
The .exe file and each linked dll is mapped into a VAS and is given its own address (so-called ImageBase) and its size here (during the load time). Image base and size can be seen in the output of programs like ListDlls [2], CurrProcess [3].
The same pair or numbers can be calculated for every section of a dll (green boxes in the "Memory Map" pic [4]) or at least for executable sections only (red boxes in Figure 2 in [5]).
It also can be done for every function exported by a dll [6].
Dmitry probably wanted to see whether the returned address belong:
- to any of "Dynamic-base DLLs"; [1]
- to the .text section of them;
- to the the whole .exe file;
- to the .text section of it;
- to specifically the wdscore.dll;
- to the .text section of it;
- to the specific function of specific image;
- to the stack of some (main) thread of the process; [1]
This would require enumerating dlls mapped into the test process and extracting their image bases and sizes at least.
I might perfectly be wrong with my guess. But at least it's a start (for the next step into advancing the test).
S.
[1] https://github.com/LordNoteworthy/windows-internals/blob/master/windows-inte...
[2] https://docs.microsoft.com/en-us/gaming/playfab/features/multiplayer/servers...
[3] https://www.nirsoft.net/utils/cprocess.html [4] https://blog.malwarebytes.com/threat-analysis/2013/06/my-memory-isnt-what-it...
[5] https://www.cyberark.com/resources/threat-research-blog/masking-malicious-me...
[6] https://www.codeproject.com/Articles/86215/Remote-Threads-Basics-Part-1#:~:t...
This is overly complicating things. The current test checks if it's within 256 bytes to the test function's address, so it's already testing everything you listed (almost). Only downside is that an arbitrary range (256) since AFAIK we can't use assembly code in tests. If we could, we could easily test it exactly.
On 2022-01-29 16:53, Gabriel Ivăncescu wrote:
This is overly complicating things. The current test checks if it's within 256 bytes to the test function's address, so it's already testing everything you listed (almost). Only downside is that an arbitrary range (256) since AFAIK we can't use assembly code in tests. If we could, we could easily test it exactly.
I mostly agree and tried here to interpret Dmitry's statement.
S.
Hi,
Every process has its own virtual address space (VAS) [1].
Thanks for the detailed explanation and also for the references. I didn't know about Windows Internals, seems like a good book to learn about how Windows works, much appreciated.
Since it has no arguments and is not a "public" exported function it probably does not matter. But yes, using __ASM_STDCALL_FUNC without declaring it as WINAPI is a mismatch. You should be using either __ASM_GLOBAL_FUNC and no WINAPI, or __ASM_STDCALL_FUNC with WINAPI.
I guess if the header doesn't use WINAPI, might as well make it without.
Yeah, I don't think it really matters. But I just want it to be as accurate as possible to the Windows implementation. Thank you for the clarification, I wasn't sure what to do.
I mostly agree and tried here to interpret Dmitry's statement.
I see, yeah, I also agree that it's not necessary.
I've submitted a revision.
On 31/01/2022 11:18, Mohamad Al-Jaf wrote:
Hi,
Every process has its own virtual address space (VAS) [1].
Thanks for the detailed explanation and also for the references. I didn't know about Windows Internals, seems like a good book to learn about how Windows works, much appreciated.
Since it has no arguments and is not a "public" exported function it probably does not matter. But yes, using __ASM_STDCALL_FUNC without declaring it as WINAPI is a mismatch. You should be using either __ASM_GLOBAL_FUNC and no WINAPI, or __ASM_STDCALL_FUNC with WINAPI.
I guess if the header doesn't use WINAPI, might as well make it without.
Yeah, I don't think it really matters. But I just want it to be as accurate as possible to the Windows implementation. Thank you for the clarification, I wasn't sure what to do.
I share your sentiment, but in this case it is impossible to know, unless we can find it documented somewhere. From a binary signature standpoint, a function with 0 args is identical in STDCALL and CDECL, because it's not mangled in the DLL either (unlike lib), and the only source we have is that header that uses it.
So it doesn't really matter, just that the mismatch looked wrong (either both STDCALL or both CDECL is fine in my eyes, but not an authoritative opinion).
Hi,
I share your sentiment, but in this case it is impossible to know, unless we can find it documented somewhere. From a binary signature standpoint, a function with 0 args is identical in STDCALL and CDECL, because it's not mangled in the DLL either (unlike lib), and the only source we have is that header that uses it.
So it doesn't really matter, just that the mismatch looked wrong (either both STDCALL or both CDECL is fine in my eyes, but not an authoritative opinion).
Oh, I see, perhaps I shouldn't have changed it. I'm not sure if I did the right thing here, especially since it's undocumented.
Giovanni had already signed off on the WINAPI version of this but I saw that it didn't get committed so I thought maybe Alexandre saw something wrong that neither one of us saw. Maybe I was being too impatient, but I wanted to also submit the stubs and those depend on this patch.
I made a decision I thought was right based on my understanding of cdecl and stdcall, but I'm still inexperienced in Wine development. I apologize if I came off as disrespectful towards Giovanni.
On 29/01/2022 06:35, Mohamad Al-Jaf wrote:
Hi Giovanni,
I see, thanks for the clarification.
Yes, the patch solves the bug. Well, for the tests removing WINAPI seems to change the pointer a bit, but it's still within the range so it doesn't seem to affect it.
I haven't tried it with __ASM_GLOBAL_FUNC, but it seems to work fine as is. Along with the stubs in this patchset, wdscore functions similarly to the native version, at least for the media creation tool.
Still, I'm just wondering if using __ASM_STDCALL_FUNC isn't accurate? In the CallCbsCore headers, CurrentIP is defined without WINAPI.[1]
I don't know what to do now. If it's okay to use WINAPI, should I resubmit the older version of this patch?
[1] https://github.com/seven-mile/CallCbsCore/blob/master/StackManager.h
Since it has no arguments and is not a "public" exported function it probably does not matter. But yes, using __ASM_STDCALL_FUNC without declaring it as WINAPI is a mismatch. You should be using either __ASM_GLOBAL_FUNC and no WINAPI, or __ASM_STDCALL_FUNC with WINAPI.
I guess if the header doesn't use WINAPI, might as well make it without.
-
Gabriel Ivăncescu -
Giovanni Mascellani -
Marvin -
Mohamad Al-Jaf -
Saulius Krasuckas