ntdll: exception.c: pretend signal handler parameters have no holes

John Reiser jreiser at BitWagon.com
Sun Jun 8 16:21:08 CDT 2008


Dan Kegel wrote:
> The operating system invokes signal handlers with
> pointers to two structs (siginfo_t and ucontext_t).
> For some reason (perhaps because these structs
> are very fluffy, and have large holes in them),
> valgrind objects when we send these structs to wineserver.
> So mark those structs fully defined to make Valgrind happy.
> (I should ask John Reiser to review this...)
> 
> Fixes warnings like
>  Syscall param writev(vector[...]) points to uninitialised byte(s)
>     at  (within /lib/ld-2.7.so)
>     by  send_request (server.c:246)
>     by  wine_server_call (server.c:327)
>     by  send_debug_event (exception.c:200)
>     by  raise_exception (exception.c:357)
>     by  __regs_RtlRaiseException (exception.c:388)
>     by  raise_segv_exception (signal_i386.c:1190)
>     by  ???
>     by  func_image (image.c:472)
>     by  run_test (test.h:421)
>     by  main (test.h:470)
>   Address 0x7f21fd34 is on thread 1's stack
>   Uninitialised value was created by a client request
>     at  setup_exception_record (signal_i386.c:1081)
>     by  segv_handler (signal_i386.c:1312)
>     by  (within /lib/tls/i686/cmov/libpthread-2.7.so)
> 
> 
> ------------------------------------------------------------------------
> 
> diff --git a/dlls/ntdll/exception.c b/dlls/ntdll/exception.c
> index 8aa906e..94b81ae 100644
> --- a/dlls/ntdll/exception.c
> +++ b/dlls/ntdll/exception.c
> @@ -38,6 +38,10 @@
>  #include "excpt.h"
>  #include "ntdll_misc.h"
>  
> +#ifdef HAVE_VALGRIND_MEMCHECK_H
> +#include <valgrind/memcheck.h>
> +#endif
> +
>  WINE_DEFAULT_DEBUG_CHANNEL(seh);
>  
>  /* Exception record for handling exceptions happening inside exception handlers */
> @@ -385,7 +389,23 @@ NTSTATUS WINAPI NtRaiseException( EXCEPTION_RECORD *rec, CONTEXT *context, BOOL
>   */
>  void WINAPI __regs_RtlRaiseException( EXCEPTION_RECORD *rec, CONTEXT *context )
>  {
> -    NTSTATUS status = raise_exception( rec, context, TRUE );
> +    NTSTATUS status;
> +
> +    /* The operating system passes us partially defined records,
> +     * and Valgrind gets annoyed when you send them to wineserver.
> +     * Mark these records fully defined to suppress Valgrind warnings.
> +     * We should do this immediately before sending the bytes
> +     * of the records to the wineserver, but for now do it here.
> +     */
> +#if defined(VALGRIND_MAKE_MEM_DEFINED)
> +    VALGRIND_DISCARD( VALGRIND_MAKE_MEM_DEFINED( rec, sizeof *rec ));
> +    VALGRIND_DISCARD( VALGRIND_MAKE_MEM_DEFINED( context, sizeof *context ));
> +#elif defined(VALGRIND_MAKE_READABLE)
> +    VALGRIND_DISCARD( VALGRIND_MAKE_READABLE( rec, sizeof *rec ));
> +    VALGRIND_DISCARD( VALGRIND_MAKE_READABLE( context, sizeof *context ));
> +#endif
> +
> +    status = raise_exception( rec, context, TRUE );
>      if (status != STATUS_SUCCESS)
>      {
>          EXCEPTION_RECORD newrec;

Yes, the patch will accomplish the goal of silencing a complaint
that the end user of memcheck+wine+program cannot do anything about.

But please put the implied task on a FIXME or list of things that
have been postponed:

   +     * We should do this immediately before sending the bytes
   +     * of the records to the wineserver, but for now do it here.

Otherwise the danger is that Wine itself will make an error
that will be undiagnosed by memcheck because of the cover-up.

-- 
John Reiser, jreiser at BitWagon.com



More information about the wine-patches mailing list