Signed-off-by: Zebediah Figura z.figura12@gmail.com --- dlls/rpcrt4/ndr_marshall.c | 5 ----- 1 file changed, 5 deletions(-)
diff --git a/dlls/rpcrt4/ndr_marshall.c b/dlls/rpcrt4/ndr_marshall.c index 52e2716..58f0108 100644 --- a/dlls/rpcrt4/ndr_marshall.c +++ b/dlls/rpcrt4/ndr_marshall.c @@ -2762,8 +2762,6 @@ static ULONG EmbeddedComplexSize(MIDL_STUB_MESSAGE *pStubMsg, case FC_LONG: case FC_ULONG: case FC_ENUM32: - case FC_INT3264: - case FC_UINT3264: return sizeof(ULONG); case FC_FLOAT: return sizeof(float); @@ -5642,9 +5640,6 @@ static ULONG get_discriminant(unsigned char fc, const unsigned char *pMemory) case FC_ULONG: case FC_ENUM32: return *(const ULONG *)pMemory; - case FC_INT3264: - case FC_UINT3264: - return *(const ULONG_PTR *)pMemory; default: FIXME("Unhandled base type: 0x%02x\n", fc); return 0;
Signed-off-by: Zebediah Figura z.figura12@gmail.com --- dlls/rpcrt4/ndr_stubless.c | 48 ++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 46 insertions(+), 2 deletions(-)
diff --git a/dlls/rpcrt4/ndr_stubless.c b/dlls/rpcrt4/ndr_stubless.c index 6a65a14..c2f260c 100644 --- a/dlls/rpcrt4/ndr_stubless.c +++ b/dlls/rpcrt4/ndr_stubless.c @@ -427,6 +427,45 @@ static inline BOOL param_needs_alloc( PARAM_ATTRIBUTES attr ) return attr.IsOut && !attr.IsIn && !attr.IsBasetype && !attr.IsByValue; }
+static inline BOOL param_is_out_basetype( PARAM_ATTRIBUTES attr ) +{ + return attr.IsOut && !attr.IsIn && attr.IsBasetype && attr.IsSimpleRef; +} + +static size_t basetype_arg_size( unsigned char fc ) +{ + switch (fc) + { + case FC_BYTE: + case FC_CHAR: + case FC_SMALL: + case FC_USMALL: + return sizeof(char); + case FC_WCHAR: + case FC_SHORT: + case FC_USHORT: + return sizeof(short); + case FC_LONG: + case FC_ULONG: + case FC_ENUM16: + case FC_ENUM32: + case FC_ERROR_STATUS_T: + return sizeof(int); + case FC_FLOAT: + return sizeof(float); + case FC_HYPER: + return sizeof(LONGLONG); + case FC_DOUBLE: + return sizeof(double); + case FC_INT3264: + case FC_UINT3264: + return sizeof(INT_PTR); + default: + FIXME("Unhandled basetype %#x.\n", fc); + return 0; + } +} + void client_do_args( PMIDL_STUB_MESSAGE pStubMsg, PFORMAT_STRING pFormat, enum stubless_phase phase, void **fpu_args, unsigned short number_of_params, unsigned char *pRetVal ) { @@ -458,8 +497,13 @@ void client_do_args( PMIDL_STUB_MESSAGE pStubMsg, PFORMAT_STRING pFormat, enum s switch (phase) { case STUBLESS_INITOUT: - if (param_needs_alloc(params[i].attr) && *(unsigned char **)pArg) - memset( *(unsigned char **)pArg, 0, calc_arg_size( pStubMsg, pTypeFormat )); + if (*(unsigned char **)pArg) + { + if (param_needs_alloc(params[i].attr)) + memset( *(unsigned char **)pArg, 0, calc_arg_size( pStubMsg, pTypeFormat )); + else if (param_is_out_basetype(params[i].attr)) + memset( *(unsigned char **)pArg, 0, basetype_arg_size( params[i].u.type_format_char )); + } break; case STUBLESS_CALCSIZE: if (params[i].attr.IsSimpleRef && !*(unsigned char **)pArg)
Signed-off-by: Huw Davies huw@codeweavers.com
Since it will have already been freed in the MUSTFREE pass.
Signed-off-by: Zebediah Figura z.figura12@gmail.com --- dlls/rpcrt4/ndr_stubless.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/dlls/rpcrt4/ndr_stubless.c b/dlls/rpcrt4/ndr_stubless.c index c2f260c..768bdc1 100644 --- a/dlls/rpcrt4/ndr_stubless.c +++ b/dlls/rpcrt4/ndr_stubless.c @@ -1215,7 +1215,7 @@ static LONG_PTR *stub_do_args(MIDL_STUB_MESSAGE *pStubMsg, } break; case STUBLESS_FREE: - if (params[i].attr.ServerAllocSize) + if (params[i].attr.ServerAllocSize && !params[i].attr.MustFree) { HeapFree(GetProcessHeap(), 0, *(void **)pArg); }
On Sat, Oct 13, 2018 at 05:46:44PM -0500, Zebediah Figura wrote:
Since it will have already been freed in the MUSTFREE pass.
Signed-off-by: Zebediah Figura z.figura12@gmail.com
dlls/rpcrt4/ndr_stubless.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/dlls/rpcrt4/ndr_stubless.c b/dlls/rpcrt4/ndr_stubless.c index c2f260c..768bdc1 100644 --- a/dlls/rpcrt4/ndr_stubless.c +++ b/dlls/rpcrt4/ndr_stubless.c @@ -1215,7 +1215,7 @@ static LONG_PTR *stub_do_args(MIDL_STUB_MESSAGE *pStubMsg, } break; case STUBLESS_FREE:
if (params[i].attr.ServerAllocSize)
if (params[i].attr.ServerAllocSize && !params[i].attr.MustFree) { HeapFree(GetProcessHeap(), 0, *(void **)pArg); }
Do we need to free in the IsSimpleRef case (like the else block below)? In this case the freer won't see the top-level ptr, though it may not be an issue with ServerAllocSize.
Huw.
On 15/10/18 04:18, Huw Davies wrote:
On Sat, Oct 13, 2018 at 05:46:44PM -0500, Zebediah Figura wrote:
Since it will have already been freed in the MUSTFREE pass.
Signed-off-by: Zebediah Figura z.figura12@gmail.com
dlls/rpcrt4/ndr_stubless.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/dlls/rpcrt4/ndr_stubless.c b/dlls/rpcrt4/ndr_stubless.c index c2f260c..768bdc1 100644 --- a/dlls/rpcrt4/ndr_stubless.c +++ b/dlls/rpcrt4/ndr_stubless.c @@ -1215,7 +1215,7 @@ static LONG_PTR *stub_do_args(MIDL_STUB_MESSAGE *pStubMsg, } break; case STUBLESS_FREE:
if (params[i].attr.ServerAllocSize)
if (params[i].attr.ServerAllocSize && !params[i].attr.MustFree) { HeapFree(GetProcessHeap(), 0, *(void **)pArg); }Do we need to free in the IsSimpleRef case (like the else block below)? In this case the freer won't see the top-level ptr, though it may not be an issue with ServerAllocSize.
Huw.
Hmm, no, you're right, we do need to free in that case. So I think the condition should be (ServerAllocSize && (!MustFree || IsSimpleRef)).
What confuses me about the whole ordeal is that in theory, ServerAllocSize represents storage space that should exist on the stub's stack. MSDN states this:
"The ServerAllocSize bits are nonzero if the parameter is [out], [in], or [in,out] pointer to pointer, or pointer to enum16, and will be initialized on the server interpreter's stack, rather than using a call to I_RpcAllocate."
and -Os mode stubs back this up — but only in some cases. Namely, a parameter like "[out] mystruct_t *x" will have a server size of 16 bits, and the inline stub will include a generated "MYSTRUCT _xM" which the server function is called with. However, in the case that's causing the crash for me, i.e. where IsSimpleRef is *not* set, e.g. any double pointer, the -Os mode stubs don't allocate stack space for the inner pointer, and just calling the type's freer — i.e. NdrPointerFree() — would incorrectly free that stack space if it were allocated. The effect, then, is that ServerAllocSize seems to have to be entirely ignored if IsSimpleRef isn't also set, which is weird. Am I missing something here?
On 15/10/18 12:39, Zebediah Figura wrote:
On 15/10/18 04:18, Huw Davies wrote:
On Sat, Oct 13, 2018 at 05:46:44PM -0500, Zebediah Figura wrote:
Since it will have already been freed in the MUSTFREE pass.
Signed-off-by: Zebediah Figura z.figura12@gmail.com
dlls/rpcrt4/ndr_stubless.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/dlls/rpcrt4/ndr_stubless.c b/dlls/rpcrt4/ndr_stubless.c index c2f260c..768bdc1 100644 --- a/dlls/rpcrt4/ndr_stubless.c +++ b/dlls/rpcrt4/ndr_stubless.c @@ -1215,7 +1215,7 @@ static LONG_PTR *stub_do_args(MIDL_STUB_MESSAGE *pStubMsg, } break; case STUBLESS_FREE:
if (params[i].attr.ServerAllocSize)
if (params[i].attr.ServerAllocSize && !params[i].attr.MustFree) { HeapFree(GetProcessHeap(), 0, *(void **)pArg); }Do we need to free in the IsSimpleRef case (like the else block below)? In this case the freer won't see the top-level ptr, though it may not be an issue with ServerAllocSize.
Huw.
Hmm, no, you're right, we do need to free in that case. So I think the condition should be (ServerAllocSize && (!MustFree || IsSimpleRef)).
What confuses me about the whole ordeal is that in theory, ServerAllocSize represents storage space that should exist on the stub's stack. MSDN states this:
"The ServerAllocSize bits are nonzero if the parameter is [out], [in], or [in,out] pointer to pointer, or pointer to enum16, and will be initialized on the server interpreter's stack, rather than using a call to I_RpcAllocate."
and -Os mode stubs back this up — but only in some cases. Namely, a parameter like "[out] mystruct_t *x" will have a server size of 16 bits, and the inline stub will include a generated "MYSTRUCT _xM" which the server function is called with. However, in the case that's causing the crash for me, i.e. where IsSimpleRef is *not* set, e.g. any double pointer, the -Os mode stubs don't allocate stack space for the inner pointer, and just calling the type's freer — i.e. NdrPointerFree() — would incorrectly free that stack space if it were allocated. The effect, then, is that ServerAllocSize seems to have to be entirely ignored if IsSimpleRef isn't also set, which is weird. Am I missing something here?
Actually, never mind, I just did a little more testing and found the problem: MIDL will mark these pointers as on-stack if and only if -Oicf mode is used, so they won't be freed by NdrPointerFree(). widl doesn't do that.
It's still kind of weird that MIDL doesn't allocate inner pointers on the stack in -Os mode. I guess it's just an optimization they never implemented because they want everyone to use -Oicf mode.
On Mon, Oct 15, 2018 at 01:13:51PM -0500, Zebediah Figura wrote:
On 15/10/18 12:39, Zebediah Figura wrote:
Hmm, no, you're right, we do need to free in that case. So I think the condition should be (ServerAllocSize && (!MustFree || IsSimpleRef)).
What confuses me about the whole ordeal is that in theory, ServerAllocSize represents storage space that should exist on the stub's stack. MSDN states this:
"The ServerAllocSize bits are nonzero if the parameter is [out], [in], or [in,out] pointer to pointer, or pointer to enum16, and will be initialized on the server interpreter's stack, rather than using a call to I_RpcAllocate."
and -Os mode stubs back this up — but only in some cases. Namely, a parameter like "[out] mystruct_t *x" will have a server size of 16 bits, and the inline stub will include a generated "MYSTRUCT _xM" which the server function is called with. However, in the case that's causing the crash for me, i.e. where IsSimpleRef is *not* set, e.g. any double pointer, the -Os mode stubs don't allocate stack space for the inner pointer, and just calling the type's freer — i.e. NdrPointerFree() — would incorrectly free that stack space if it were allocated. The effect, then, is that ServerAllocSize seems to have to be entirely ignored if IsSimpleRef isn't also set, which is weird. Am I missing something here?
Actually, never mind, I just did a little more testing and found the problem: MIDL will mark these pointers as on-stack if and only if -Oicf mode is used, so they won't be freed by NdrPointerFree(). widl doesn't do that.
It's still kind of weird that MIDL doesn't allocate inner pointers on the stack in -Os mode. I guess it's just an optimization they never implemented because they want everyone to use -Oicf mode.
Yes, it's annoying that the format string changes across the different modes---I've been bitten by that in the past.
Huw.
-
Huw Davies -
Zebediah Figura