[Bug 48941] New: Wine's implementation of IMalloc.DidAlloc relies on a spy mechanism, while Windows just calls "HeapValidate"

List overview All Threads

newer

older

[Bug 48729] New: Binary Domain has...

[Bug 6083] Heredis doesn't ...

WineHQ Bugzilla

16 Apr 2020 16 Apr '20

12:21 p.m.

https://bugs.winehq.org/show_bug.cgi?id=48941

Bug ID: 48941 Summary: Wine's implementation of IMalloc.DidAlloc relies on a spy mechanism, while Windows just calls "HeapValidate" Product: Wine Version: unspecified Hardware: x86 OS: other Status: UNCONFIRMED Severity: major Priority: P2 Component: -unknown Assignee: wine-bugs@winehq.org Reporter: contact@kcsoftwares.com

Created attachment 66916 --> https://bugs.winehq.org/attachment.cgi?id=66916 Executable allowing to raise the problem.

When using this test application : https://jira.reactos.org/secure/attachment/56514/ros.exe initially during ReactOS tests using latest development version, the problem described in https://jira.reactos.org/browse/CORE-15262 has been observed.

Investigation by the ReactOS dev team, pointed out that the root cause was on Wine side because Wine's implementation of IMalloc.DidAlloc relies on a spy mechanism, while Windows just calls "HeapValidate".

See comments on Wine's implementation of IMalloc.DidAlloc relies on a spy mechanism, while Windows just calls "HeapValidate" for further details.

Reproduce : Open ros.exe attachment Click on button "once", select a folder : Pop-up showing the folder path Click again on button => nothing happens due to the bug in Wine's IMalloc.DidAlloc while this works perfectly in Windows.

-- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.

Show replies by date

WineHQ Bugzilla

16 Apr 16 Apr

1:20 p.m.

New subject: [Bug 48941] Wine's implementation of IMalloc.DidAlloc relies on a spy mechanism, while Windows just calls "HeapValidate"

https://bugs.winehq.org/show_bug.cgi?id=48941

Nikolay Sivov bunglehead@gmail.com changed:

What |Removed |Added ---------------------------------------------------------------------------- Severity|major |normal

--- Comment #1 from Nikolay Sivov bunglehead@gmail.com --- It's true that what we have is incomplete, for example fSpyed is never set, but how do you that Windows is using HeapValidate and when? Also please attach source code for that test case.

-- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.

WineHQ Bugzilla

1:24 p.m.

New subject: [Bug 48941] Wine's implementation of IMalloc.DidAlloc relies on a spy mechanism, while Windows just calls "HeapValidate"

https://bugs.winehq.org/show_bug.cgi?id=48941

--- Comment #2 from Kyle_Katarn contact@kcsoftwares.com --- This is the code section from JCL library JclShell.pas causing the problem :

function PidlFree(var IdList: PItemIdList): Boolean; var Malloc: IMalloc; begin Result := False; if IdList = nil then Result := True else begin Malloc := nil; if Succeeded(SHGetMalloc(Malloc)) and (Malloc.DidAlloc(IdList) > 0) then begin Malloc.Free(IdList); IdList := nil; Result := True; end; end; end;

-- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.

WineHQ Bugzilla

2:15 p.m.

New subject: [Bug 48941] Wine's implementation of IMalloc.DidAlloc relies on a spy mechanism, while Windows just calls "HeapValidate"

https://bugs.winehq.org/show_bug.cgi?id=48941

--- Comment #3 from Kyle_Katarn contact@kcsoftwares.com --- (In reply to Nikolay Sivov from comment #1)

...

It's true that what we have is incomplete, for example fSpyed is never set, but how do you that Windows is using HeapValidate and when? Also please attach source code for that test case.

Do you need more details ?

-- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.

WineHQ Bugzilla

3:50 p.m.

New subject: [Bug 48941] Wine's implementation of IMalloc.DidAlloc relies on a spy mechanism, while Windows just calls "HeapValidate"

https://bugs.winehq.org/show_bug.cgi?id=48941

--- Comment #4 from Nikolay Sivov bunglehead@gmail.com --- (In reply to Kyle_Katarn from comment #3)

...

(In reply to Nikolay Sivov from comment #1)

...
It's true that what we have is incomplete, for example fSpyed is never set, but how do you that Windows is using HeapValidate and when? Also please attach source code for that test case.

Do you need more details ?

Question regarding HeapValidate() still stands. For use case, no, it's clear what breaks - we simply don't return correct flags, with spy or without. Of course there is not reason to do DidAlloc, or SHGetMalloc() in this code, it should simply call CoTaskMemFree(), or ILFree/SHFree() which is the same thing.

-- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.

WineHQ Bugzilla

4:21 p.m.

New subject: [Bug 48941] Wine's implementation of IMalloc.DidAlloc relies on a spy mechanism, while Windows just calls "HeapValidate"

https://bugs.winehq.org/show_bug.cgi?id=48941

--- Comment #5 from Kyle_Katarn contact@kcsoftwares.com --- Thanks ! I opened a ticket on JCL side. Would you have a patch to propose to change the code according to your suggestions ?

Regarding Wine, do you imply that the fix in order to correctly handle such code (which runs perfectly in Windows) will not be investigated ? Or do you have a potential fix in Wine to suggest ?

this is blocking all apps using this JCL code section in Wine (+ potentially others). Thanks !

-- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.

WineHQ Bugzilla

4:22 p.m.

New subject: [Bug 48941] Wine's implementation of IMalloc.DidAlloc relies on a spy mechanism, while Windows just calls "HeapValidate"

https://bugs.winehq.org/show_bug.cgi?id=48941

--- Comment #6 from Kyle_Katarn contact@kcsoftwares.com --- https://issuetracker.delphi-jedi.org/view.php?id=6688

-- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.

WineHQ Bugzilla

4:34 p.m.

New subject: [Bug 48941] Wine's implementation of IMalloc.DidAlloc relies on a spy mechanism, while Windows just calls "HeapValidate"

https://bugs.winehq.org/show_bug.cgi?id=48941

--- Comment #7 from Nikolay Sivov bunglehead@gmail.com --- (In reply to Kyle_Katarn from comment #5)

...

Thanks ! I opened a ticket on JCL side. Would you have a patch to propose to change the code according to your suggestions ?

I'm not going to patch it, no, and I don't know what was authors motivation too. If it was a workaround for some garbage pointer passed in, that could cause damage. Ideally callers should not pass invalid input, and if it's some internal function, callers should be fixed, and function could then be simplified.

...

Regarding Wine, do you imply that the fix in order to correctly handle such code (which runs perfectly in Windows) will not be investigated ? Or do you have a potential fix in Wine to suggest ?

Sure, it should be fixed in Wine too. I'll probably take a look.

...

this is blocking all apps using this JCL code section in Wine (+ potentially others). Thanks !

-- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.

WineHQ Bugzilla

4:34 p.m.

New subject: [Bug 48941] Wine's implementation of IMalloc.DidAlloc relies on a spy mechanism, while Windows just calls "HeapValidate"

https://bugs.winehq.org/show_bug.cgi?id=48941

Nikolay Sivov bunglehead@gmail.com changed:

What |Removed |Added ---------------------------------------------------------------------------- Component|-unknown |ole32

-- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.

WineHQ Bugzilla

4:40 p.m.

New subject: [Bug 48941] Wine's implementation of IMalloc.DidAlloc relies on a spy mechanism, while Windows just calls "HeapValidate"

https://bugs.winehq.org/show_bug.cgi?id=48941

--- Comment #8 from Kyle_Katarn contact@kcsoftwares.com --- OK. Issue reported to JCL dev team. Very unlikely to see a modification coming as the project seems "on hold" and no longer as active as it used to be.

Regarding issue / impact, the problem impacting execution in Wine is of course happening even on fully nominal cases.

Thank you for giving a look at how Wine could be improved with regard to this. Once implemented, please let me know and i'll pass the information to the ReactOS dev team too !

-- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.

WineHQ Bugzilla

7:03 p.m.

New subject: [Bug 48941] IMalloc::DidAlloc() return value is inaccurate

https://bugs.winehq.org/show_bug.cgi?id=48941

Nikolay Sivov bunglehead@gmail.com changed:

-- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.

WineHQ Bugzilla

17 Apr 17 Apr

9:33 p.m.

New subject: [Bug 48941] IMalloc::DidAlloc() return value is inaccurate

https://bugs.winehq.org/show_bug.cgi?id=48941

--- Comment #9 from Nikolay Sivov bunglehead@gmail.com --- This patch seems enough https://source.winehq.org/git/wine.git/?a=commit;h=08f4b6ee0a05fe27e56cb9777.... Please retest with current wine.

-- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.

WineHQ Bugzilla

9:33 p.m.

New subject: [Bug 48941] IMalloc::DidAlloc() return value is inaccurate

https://bugs.winehq.org/show_bug.cgi?id=48941

Nikolay Sivov bunglehead@gmail.com changed:

What |Removed |Added ---------------------------------------------------------------------------- Version|unspecified |5.6

-- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.

WineHQ Bugzilla

18 Apr 18 Apr

6:45 a.m.

New subject: [Bug 48941] IMalloc::DidAlloc() return value is inaccurate

https://bugs.winehq.org/show_bug.cgi?id=48941

--- Comment #10 from Kyle_Katarn contact@kcsoftwares.com --- Thanks ! I'll check with ReactOS team !

-- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.

WineHQ Bugzilla

20 Apr 20 Apr

5:54 a.m.

New subject: [Bug 48941] IMalloc::DidAlloc() return value is inaccurate

https://bugs.winehq.org/show_bug.cgi?id=48941

--- Comment #11 from Kyle_Katarn contact@kcsoftwares.com --- (In reply to Nikolay Sivov from comment #9)

...

This patch seems enough https://source.winehq.org/git/wine.git/?a=commit; h=08f4b6ee0a05fe27e56cb9777dce845dcf1072f8. Please retest with current wine.

Hello,

Thank you very much. Positive results from the preliminary test done by the ReactOS team on the test case + more complex cases (based SUMo). We'll continue tests, but this looks very good.

In which Wine version do you plan to integrate this patch ?

-- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.

WineHQ Bugzilla

8:03 a.m.

New subject: [Bug 48941] IMalloc::DidAlloc() return value is inaccurate

https://bugs.winehq.org/show_bug.cgi?id=48941

--- Comment #12 from Nikolay Sivov bunglehead@gmail.com --- Wine 5.7 will be first release to include it.

-- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.

WineHQ Bugzilla

3:41 p.m.

New subject: [Bug 48941] IMalloc::DidAlloc() return value is inaccurate

https://bugs.winehq.org/show_bug.cgi?id=48941

--- Comment #13 from Kyle_Katarn contact@kcsoftwares.com --- (In reply to Nikolay Sivov from comment #12)

...

Wine 5.7 will be first release to include it.

Perfect, thanks !

-- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.

WineHQ Bugzilla

9:21 p.m.

New subject: [Bug 48941] IMalloc::DidAlloc() return value is inaccurate

https://bugs.winehq.org/show_bug.cgi?id=48941

Nikolay Sivov bunglehead@gmail.com changed:

--- Comment #14 from Nikolay Sivov bunglehead@gmail.com --- Marking fixed as test application works now, commit 08f4b6ee0a05fe27e56cb9777dce845dcf1072f8.

-- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.

WineHQ Bugzilla

24 Apr 24 Apr

8:14 p.m.

New subject: [Bug 48941] IMalloc::DidAlloc() return value is inaccurate

https://bugs.winehq.org/show_bug.cgi?id=48941

Alexandre Julliard julliard@winehq.org changed:

What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED

--- Comment #15 from Alexandre Julliard julliard@winehq.org --- Closing bugs fixed in 5.7.

-- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.

WineHQ Bugzilla

8:15 p.m.

New subject: [Bug 48941] IMalloc::DidAlloc() return value is inaccurate

https://bugs.winehq.org/show_bug.cgi?id=48941

--- Comment #16 from Kyle_Katarn contact@kcsoftwares.com --- (In reply to Alexandre Julliard from comment #15)

...

Closing bugs fixed in 5.7.

Thank you

-- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.

WineHQ Bugzilla

30 Apr 30 Apr

3:16 p.m.

New subject: [Bug 48941] IMalloc::DidAlloc() return value is inaccurate

https://bugs.winehq.org/show_bug.cgi?id=48941

Anastasius Focht focht@gmx.net changed:

-- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.

1714

Age (days ago)

1728

Last active (days ago)

wine-bugs@winehq.org

20 comments

1 participants

tags (0)

participants (1)

WineHQ Bugzilla