http://bugs.winehq.org/show_bug.cgi?id=14078
Summary: Rewrite typelib marshaller on top of NDR functions Product: Wine Version: CVS/GIT Platform: Other OS/Version: other Status: NEW Severity: normal Priority: P2 Component: ole AssignedTo: wine-bugs@winehq.org ReportedBy: dank@kegel.com
[Copied from wine-devel.] Dan K. wrote in http://www.winehq.org/pipermail/wine-devel/2008-June/066540.html --- snip --- While looking at the valgrind warning in http://kegel.com/wine/valgrind/logs-2008-06-20/vg-oleaut32_tmarshal.txt
Conditional jump or move depends on uninitialised value(s) at serialize_param (tmarshal.c:736) by serialize_param (tmarshal.c:744) by xCall (tmarshal.c:1414) by ??? by func_tmarshal (tmarshal.c:1179) by run_test (test.h:449) by main (test.h:498) Uninitialised value was created by a stack allocation at test_typelibmarshal (tmarshal.c:762)
The problem happens during a call to this method where widget is a pointer to an uninitialized pointer which will receive the pointer to the widget:
interface IKindaEnumWidget : IUnknown { HRESULT Next( [out] IWidget **widget);
I discovered that the attached patch prevented the problem. I don't quite understand why; at first glance, widget is an out parameter from the function, why would it be dereferenced while serializing the call?
--- snip ---
Rob Shearman wrote: http://www.winehq.org/pipermail/wine-devel/2008-June/066571.html
--- snip --- It's a bug in the typelib marshaller. It doesn't check whether a VT_PTR type is actually an interface pointer and not access it on input when the parameter is an [out] parameter. Note that because of the memory re-use semantics it is legal to access memory passed in to a remote function, even when the parameter is [out].
I think it's getting close to the time to reimplement the typelib marshaller on top of NDR functions so that we don't have to implement these subtleties twice, would improve performance and would reduce the amount of code. --- snip ---
http://bugs.winehq.org/show_bug.cgi?id=14078
Rob Shearman robertshearman@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Component|ole |oleaut32
http://bugs.winehq.org/show_bug.cgi?id=14078
Austin English austinenglish@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Version|CVS/GIT |unspecified
--- Comment #1 from Austin English austinenglish@gmail.com 2009-01-20 02:39:18 --- Removing deprecated CVS/GIT version tag. Please retest in current git. If the bug is still present in today's wine, but was not present in some earlier version of wine, please update version field to earliest known version of wine that had the bug. Thanks!
http://bugs.winehq.org/show_bug.cgi?id=14078
Dan Kegel dank@kegel.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Version|unspecified |0.9
--- Comment #2 from Dan Kegel dank@kegel.com 2009-07-04 09:54:31 --- In WINEDEBUG=warn+heap make test, the tmarshal test sometimes fails with: tmarshal.c:604: Test failed: Struct parameter passed by value corrupted so I tried it under valgrind again.
Running valgrind --track-origins=yes --trace-children=yes wine oleaut32_test.exe.so tmarshal.c still shows the warning Conditional jump or move depends on uninitialised value(s) at serialize_param (tmarshal.c:785) by serialize_param (tmarshal.c:789) by xCall (tmarshal.c:1461) by 0x7F53003B: ??? by func_tmarshal (tmarshal.c:1455) by run_test (test.h:454) Uninitialised value was created by a stack allocation at test_typelibmarshal (tmarshal.c:969)
So it's still present in git, and it's been there forever, as far as I know. Setting original version to 0.9 as a rough approximation.
http://bugs.winehq.org/show_bug.cgi?id=14078
Dan Kegel dank@kegel.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |9916
--- Comment #3 from Austin English austinenglish@gmail.com 2009-10-26 12:56:17 --- The heap error was fixed by http://source.winehq.org/git/wine.git/?a=commitdiff;h=1e0b8367124c8a10648f66.... Jeremy doesn't think it will fix the valgrind error, but you might check..
http://bugs.winehq.org/show_bug.cgi?id=14078
--- Comment #4 from Dan Kegel dank@kegel.com 2009-10-26 23:17:57 --- Seems to be improved, see http://kegel.com/wine/valgrind/logs/2009-10-26-08.26/diff-oleaut32_tmarshal....
(Although there is now one more leak in that file?)
http://bugs.winehq.org/show_bug.cgi?id=14078
nathan.n saturn_systems@yahoo.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |saturn_systems@yahoo.com
http://bugs.winehq.org/show_bug.cgi?id=14078
Dan Kegel dank@kegel.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |austinenglish@gmail.com
--- Comment #5 from Dan Kegel dank@kegel.com 2011-10-15 15:35:05 CDT --- *** Bug 26105 has been marked as a duplicate of this bug. ***
http://bugs.winehq.org/show_bug.cgi?id=14078
--- Comment #6 from Dan Kegel dank@kegel.com 2011-10-15 15:37:55 CDT --- Still happening. Current valgrind warnings:
Conditional jump or move depends on uninitialised value(s) at serialize_param (tmarshal.c:789) by serialize_param (tmarshal.c:793) by xCall (tmarshal.c:1393) by ??? by func_tmarshal (tmarshal.c:1607) by run_test (test.h:556) by main (test.h:624) Uninitialised value was created by a stack allocation at test_typelibmarshal (tmarshal.c:997)
Conditional jump or move depends on uninitialised value(s) at deserialize_param (tmarshal.c:1066) by deserialize_param (tmarshal.c:1079) by TMStubImpl_Invoke (tmarshal.c:2066) by RPC_ExecuteCall (rpc.c:1417) by apartment_wndproc (compobj.c:1007) by ??? (in /oldhome/dank/wine-git/dlls/user32/user32.dll.so) by call_window_proc (winproc.c:242) by WINPROC_CallProcAtoW (winproc.c:601) by WINPROC_call_window (winproc.c:910) by DispatchMessageA (message.c:3738) by host_object_proc (tmarshal.c:93) by ??? (signal_i386.c:2473) by call_thread_entry_point (signal_i386.c:2499) by start_thread (thread.c:405) Uninitialised value was created by a stack allocation at test_typelibmarshal (tmarshal.c:997)
http://bugs.winehq.org/show_bug.cgi?id=14078
--- Comment #7 from Dan Kegel dank@kegel.com 2011-10-18 23:06:38 CDT --- Possibly related warning in ieframe/ie.ok:
Invalid read of size 4 at memcpy (mc_replace_strmem.c:635) by xbuf_add (tmarshal.c:107) by serialize_param (tmarshal.c:688) by serialize_param (tmarshal.c:793) by TMStubImpl_Invoke (tmarshal.c:2112) by RPC_ExecuteCall (rpc.c:1417) by apartment_wndproc (compobj.c:1007) by ??? (winproc.c:172) by call_window_proc (winproc.c:242) by WINPROC_call_window (winproc.c:899) by DispatchMessageW (message.c:3809) by IEWinMain (iexplore.c:1051) by WinMain (main.c:81) by main (exe_main.c:48)
https://bugs.winehq.org/show_bug.cgi?id=14078
--- Comment #8 from Austin English austinenglish@gmail.com --- (In reply to Dan Kegel from comment #7)
Possibly related warning in ieframe/ie.ok:
Invalid read of size 4 at memcpy (mc_replace_strmem.c:635) by xbuf_add (tmarshal.c:107) by serialize_param (tmarshal.c:688) by serialize_param (tmarshal.c:793) by TMStubImpl_Invoke (tmarshal.c:2112) by RPC_ExecuteCall (rpc.c:1417) by apartment_wndproc (compobj.c:1007) by ??? (winproc.c:172) by call_window_proc (winproc.c:242) by WINPROC_call_window (winproc.c:899) by DispatchMessageW (message.c:3809) by IEWinMain (iexplore.c:1051) by WinMain (main.c:81) by main (exe_main.c:48)
That's bug 36286
https://bugs.winehq.org/show_bug.cgi?id=14078
--- Comment #9 from Austin English austinenglish@gmail.com --- ==31591== 32 bytes in 1 blocks are definitely lost in loss record 230 of 638 ==31591== at 0x7BC4C735: notify_alloc (heap.c:255) ==31591== by 0x7BC50F79: RtlAllocateHeap (heap.c:1716) ==31591== by 0x4FF22D2: deserialize_param (tmarshal.c:1201) ==31591== by 0x4FF3941: xCall (tmarshal.c:1482) ==31591== by 0x6E701B2: ??? ==31591== by 0x4C741E3: func_tmarshal (tmarshal.c:1948) ==31591== by 0x4D75F00: run_test (test.h:584) ==31591== by 0x4D762EF: main (test.h:654)
still present.
https://bugs.winehq.org/show_bug.cgi?id=14078
Austin English austinenglish@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Keywords| |download, source, testcase, | |valgrind
https://bugs.winehq.org/show_bug.cgi?id=14078
Sebastian Lackner sebastian@fds-team.de changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |sebastian@fds-team.de
https://bugs.winehq.org/show_bug.cgi?id=14078
Zebediah Figura z.figura12@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |z.figura12@gmail.com Assignee|wine-bugs@winehq.org |z.figura12@gmail.com
--- Comment #10 from Zebediah Figura z.figura12@gmail.com --- Created attachment 62098 --> https://bugs.winehq.org/attachment.cgi?id=62098 eponymous rewrite
I've attached a set of patches, in a hopefully reviewable state, that perform this task.
These patches pass all of the currently existing tests in oleaut32. I haven't yet done exhaustive testing with real-world programs, partly because I don't know any real-world programs to test with. If anyone does, please let me know.
Currently there are two problems with these patches:
(1) Patch 0010 embeds an automatically generated type format string into the file itself. This is suboptimal: we'd like that string to be automatically generated, but it's not clear how to do that. Adding a dummy IDL and accessing its exported variables from ndr_typelib.c doesn't give us all that we need (in particular, we can get the address of the type format string itself, and the offsets of the Automation types, but we have no way of getting the string's length). Including the generated .c file inside of ndr_typelib.c won't work because it'll result in multiple definitions.
(2) shell32:shelldispatch fails. It fails because the implementation given here (in particular patch 0012) always delegates proxy methods to parents if they aren't IUnknown, and that runs into bug 45673 when we call IWebBrowser2_get_Application() [line 1128]. I don't know how to solve bug 45673, so I'd really appreciate advice there.
https://bugs.winehq.org/show_bug.cgi?id=14078
Alistair Leslie-Hughes leslie_alistair@hotmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |leslie_alistair@hotmail.com Attachment #62098|text/plain |application/x-compressed-ta mime type| |r Attachment #62098|1 |0 is patch| |
https://bugs.winehq.org/show_bug.cgi?id=14078
--- Comment #11 from Zebediah Figura z.figura12@gmail.com --- Bug 45673 has been resolved (904d1688a37197fe7f0c7c23f4491bc8320a79c7), so all that remains is issue #1 above (plus whatever other issues come up in reviewing).
https://bugs.winehq.org/show_bug.cgi?id=14078
Zebediah Figura z.figura12@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED Status|NEW |RESOLVED Fixed by SHA1| |077b4391d442e927a2a59b5afb2 | |44355b0634aaa
--- Comment #12 from Zebediah Figura z.figura12@gmail.com --- Resolving fixed; https://source.winehq.org/git/wine.git/commit/077b4391d442e927a2a59b5afb2443....
https://bugs.winehq.org/show_bug.cgi?id=14078
Alexandre Julliard julliard@winehq.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED
--- Comment #13 from Alexandre Julliard julliard@winehq.org --- Closing bugs fixed in 3.21.
-
wine-bugs＠winehq.org