[PATCH 0/1] MR1229: msvcrt: Fix crash due to global symbol coverage of dynamic libraries

[Wu Liande (＠wuliande)]

_Condition_variable_notify_one and DEFINE_THISCALL_WRAPPER(_Condition_variable_notify_one, 4) are redefined in other dynamic libraries.
Therefore, it caused the problem of global symbol coverage, which caused crash.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/1229

[PATCH 1/1] msvcrt: Fix crash due to global symbol coverage of dynamic libraries

[wuliande]

From: wuliande <m13552573542(a)163.com>

_Condition_variable_notify_one and DEFINE_THISCALL_WRAPPER(_Condition_variable_notify_one, 4) are redefined in other dynamic libraries.
Therefore, it caused the problem of global symbol coverage, which caused crash.
---
 dlls/msvcp90/msvcp_main.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/dlls/msvcp90/msvcp_main.c b/dlls/msvcp90/msvcp_main.c
index 4c9faec9e26..8717384c2c6 100644
--- a/dlls/msvcp90/msvcp_main.c
+++ b/dlls/msvcp90/msvcp_main.c
@@ -88,10 +88,10 @@ static bool (__thiscall *critical_section_trylock)(critical_section*);
 static _Condition_variable* (__thiscall *_Condition_variable_ctor)(_Condition_variable*);
 static void (__thiscall *_Condition_variable_dtor)(_Condition_variable*);
 static void (__thiscall *_Condition_variable_wait)(_Condition_variable*, critical_section*);
-bool (__thiscall *_Condition_variable_wait_for)(_Condition_variable*,
+static bool (__thiscall *_Condition_variable_wait_for)(_Condition_variable*,
         critical_section*, unsigned int);
-void (__thiscall *_Condition_variable_notify_one)(_Condition_variable*);
-void (__thiscall *_Condition_variable_notify_all)(_Condition_variable*);
+static void (__thiscall *_Condition_variable_notify_one)(_Condition_variable*);
+static void (__thiscall *_Condition_variable_notify_all)(_Condition_variable*);
 
 void cs_init(cs *cs)
 {
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/1229

[Wu Liande (＠wuliande)]

@piotr sorry i need Resubmit this commit because of personal reasons。
please install the app(https://dldir1.qq.com/wework/work_weixin/WeCom_4.0.19.6020.exe),after setup it.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/1229#note_12798

[Piotr Caban (＠piotr)]

I think that the problem in this application is completely different. I don't know why your patch makes a difference for you but it doesn't help in my case (as expected). Please note that I would like to get your patch committed to wine anyway. But the commit message doesn't make sense (please change it to `msvcp90: Make _Condition_variable_* function pointers static.`).

I have quickly debugged the issue and it's related to internal representation of SRWLock. The application does following thing:
```
SRWLock *lock;
lock = malloc(sizeof(*lock));
lock->Ptr = (void*)1;
ReleaseSRWLockExclusive(lock);
```
On Windows lock->Ptr=1 is used to mark that the exclusive lock is being held with no waiters. In this case ReleaseSRWLockExclusive should probably set lock->Ptr=NULL and return. See attached hack that lets the application start on my machine ([0001-tmp.txt](/uploads/506068c59882aaf35362845fa9b82fea/0001-tmp.txt)).

@zfigura - you may be interested in looking into it :smile:

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/1229#note_12825

[Zebediah Figura (＠zfigura)]

Yes, I'll take a look.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/1229#note_12826

[Wu Liande (＠wuliande)]

@piotr @zfigura I solved the problem of deadlock by the following method
+ if (old.s.owners != -1){
+            ERR("Lock %p is not owned exclusive!\n", lock);
+            RtlInitializeSRWLock(lock);
+            return ;
+ }

but This deadlock problem happens after crash, it's two problems.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/1229#note_12907

[PATCH 0/1] MR1229: msvcp90: Make _Condition_variable_* function pointers static

[Wu Liande (＠wuliande)]

On Thu Nov  3 01:20:08 2022 +0000, Piotr Caban wrote:
> I think that the problem in this application is completely different. I
> don't know why your patch makes a difference for you but it doesn't help
> in my case (as expected). Please note that I would like to get your
> patch committed to wine anyway. But the commit message doesn't make
> sense (please change it to `msvcp90: Make _Condition_variable_* function
> pointers static.`).
> I have quickly debugged the issue and it's related to internal
> representation of SRWLock. The application does following thing:
> ```
> SRWLock *lock;
> lock = malloc(sizeof(*lock));
> lock->Ptr = (void*)1;
> ReleaseSRWLockExclusive(lock);
> ```
> On Windows lock->Ptr=1 is used to mark that the exclusive lock is being
> held with no waiters. In this case ReleaseSRWLockExclusive should
> probably set lock->Ptr=NULL and return. See attached hack that lets the
> application start on my machine ([0001-tmp.txt](/uploads/506068c59882aaf35362845fa9b82fea/0001-tmp.txt)).
> @zfigura - you may be interested in looking into it :smile:
> @piotr I changed the commit instructions as you said。But I'm wondering why you didn't reproduce that。
> my env：
> debain 11
> wine i386

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/1229#note_12911

[PATCH 0/1] MR1229: msvcp90: Make _Condition_variable_* function pointers static

[Piotr Caban (＠piotr)]

On Thu Nov  3 01:24:28 2022 +0000, Wu Liande wrote:
> @piotr I changed the commit instructions as you said。But I'm wondering
> why you didn't reproduce that。
> my env：
> debain 11
> wine i386
> my log:
> ` ./wine '/home/makesi/.wine/drive_c/Program Files
> (x86)/WXWork/WXWork.exe'                             
> 007c:fixme:hid:handle_IRP_MN_QUERY_ID Unhandled type 00000005
> 007c:fixme:hid:handle_IRP_MN_QUERY_ID Unhandled type 00000005
> 007c:fixme:hid:handle_IRP_MN_QUERY_ID Unhandled type 00000005
> 007c:fixme:hid:handle_IRP_MN_QUERY_ID Unhandled type 00000005
> 0024:err:module:LdrInitializeThunk "MSVCP120.dll" failed to initialize, aborting
> 0024:err:module:LdrInitializeThunk Initializing dlls for L"C:\\Program
> Files (x86)\\WXWork\\WXWork.exe" failed, status c0000005
> `
> I don't know why it crashes on startup on your machine but it's not caused by _Condition_variable_notify_one being redefined in multiple .so files (I guess you're using no-mingw build of Wine). Maybe there's e.g. some kind of memory corruption that happens to be hidden by that change.

If you don't believe me that it's not the problem - the application loads both msvcp120 and msvcp140, there are much more global symbols with the same names and you don't see any problem caused by it.

Please change the commit message (patch title you set with git while committing the patch, not only merge request title).

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/1229#note_12946