[Bug 14078] New: Rewrite typelib marshaller on top of NDR functions
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(a)winehq.org ReportedBy: dank(a)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 --- -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email 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.
http://bugs.winehq.org/show_bug.cgi?id=14078 Rob Shearman <robertshearman(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Component|ole |oleaut32 -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email 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.
http://bugs.winehq.org/show_bug.cgi?id=14078 Austin English <austinenglish(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version|CVS/GIT |unspecified --- Comment #1 from Austin English <austinenglish(a)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! -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email 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.
http://bugs.winehq.org/show_bug.cgi?id=14078 Dan Kegel <dank(a)kegel.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version|unspecified |0.9 --- Comment #2 from Dan Kegel <dank(a)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. -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email 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.
http://bugs.winehq.org/show_bug.cgi?id=14078 Dan Kegel <dank(a)kegel.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |9916 --- Comment #3 from Austin English <austinenglish(a)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.. -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email 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.
http://bugs.winehq.org/show_bug.cgi?id=14078 --- Comment #4 from Dan Kegel <dank(a)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?) -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email 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.
http://bugs.winehq.org/show_bug.cgi?id=14078 nathan.n <saturn_systems(a)yahoo.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |saturn_systems(a)yahoo.com -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email 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.
http://bugs.winehq.org/show_bug.cgi?id=14078 Dan Kegel <dank(a)kegel.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |austinenglish(a)gmail.com --- Comment #5 from Dan Kegel <dank(a)kegel.com> 2011-10-15 15:35:05 CDT --- *** Bug 26105 has been marked as a duplicate of this bug. *** -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email 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.
http://bugs.winehq.org/show_bug.cgi?id=14078 --- Comment #6 from Dan Kegel <dank(a)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) -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email 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.
http://bugs.winehq.org/show_bug.cgi?id=14078 --- Comment #7 from Dan Kegel <dank(a)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) -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email 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.
https://bugs.winehq.org/show_bug.cgi?id=14078 --- Comment #8 from Austin English <austinenglish(a)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 -- 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.
https://bugs.winehq.org/show_bug.cgi?id=14078 --- Comment #9 from Austin English <austinenglish(a)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. -- 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.
https://bugs.winehq.org/show_bug.cgi?id=14078 Austin English <austinenglish(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |download, source, testcase, | |valgrind -- 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.
https://bugs.winehq.org/show_bug.cgi?id=14078 Sebastian Lackner <sebastian(a)fds-team.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |sebastian(a)fds-team.de -- 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.
https://bugs.winehq.org/show_bug.cgi?id=14078 Zebediah Figura <z.figura12(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |z.figura12(a)gmail.com Assignee|wine-bugs(a)winehq.org |z.figura12(a)gmail.com --- Comment #10 from Zebediah Figura <z.figura12(a)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. -- 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.
https://bugs.winehq.org/show_bug.cgi?id=14078 Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |leslie_alistair(a)hotmail.com Attachment #62098|text/plain |application/x-compressed-ta mime type| |r Attachment #62098|1 |0 is 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.
https://bugs.winehq.org/show_bug.cgi?id=14078 --- Comment #11 from Zebediah Figura <z.figura12(a)gmail.com> --- Bug 45673 has been resolved (904d1688a37197fe7f0c7c23f4491bc8320a79c7), so all that remains is issue #1 above (plus whatever other issues come up in reviewing). -- 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.
https://bugs.winehq.org/show_bug.cgi?id=14078 Zebediah Figura <z.figura12(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED Status|NEW |RESOLVED Fixed by SHA1| |077b4391d442e927a2a59b5afb2 | |44355b0634aaa --- Comment #12 from Zebediah Figura <z.figura12(a)gmail.com> --- Resolving fixed; https://source.winehq.org/git/wine.git/commit/077b4391d442e927a2a59b5afb2443.... -- 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.
https://bugs.winehq.org/show_bug.cgi?id=14078 Alexandre Julliard <julliard(a)winehq.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED --- Comment #13 from Alexandre Julliard <julliard(a)winehq.org> --- Closing bugs fixed in 3.21. -- 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.
participants (1)
-
wine-bugs＠winehq.org