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?
On Fri, Jun 20, 2008 at 09:56:01PM -0700, Dan Kegel wrote:
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?
diff --git a/dlls/oleaut32/tests/tmarshal.c b/dlls/oleaut32/tests/tmarshal.c index 3812b72..b1ade9b 100644 --- a/dlls/oleaut32/tests/tmarshal.c +++ b/dlls/oleaut32/tests/tmarshal.c @@ -792,6 +792,7 @@ static void test_typelibmarshal(void) ok_ole_success(hr, CoUnmarshalInterface); IStream_Release(pStream);
- pWidget = NULL; hr = IKindaEnumWidget_Next(pKEW, &pWidget); ok_ole_success(hr, IKindaEnumWidget_Next);
Not sure why, but I think it is unclear from the [out] tag where the code starts allocating.
Perhaps in *widget, but perhaps it might just fill out **widget.
Not sure if IDL has a clear definition on when the [out] tag starts to apply.
ciao, Marcus
2008/6/21 Dan Kegel dank@kegel.com:
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?
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.
On Mon, Jun 23, 2008 at 12:12 AM, Rob Shearman robertshearman@gmail.com wrote:
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.
Thanks! Bug filed, http://bugs.winehq.org/show_bug.cgi?id=14078