]> git.ipfire.org Git - thirdparty/valgrind.git/commitdiff
In an inner valgrind, register the interim stack earlier
authorPhilippe Waroquiers <philippe.waroquiers@skynet.be>
Tue, 22 Oct 2013 21:20:14 +0000 (21:20 +0000)
committerPhilippe Waroquiers <philippe.waroquiers@skynet.be>
Tue, 22 Oct 2013 21:20:14 +0000 (21:20 +0000)
The "late" registration of the interim stack is causing false
positive non addressable memcheck errors in x86.

Registering the interim stack earlier avoids these false positive.

Note however that this is just a bypass for the problem.

I believe there is a more fundamental problem in m_stacks.c stack handling:
In case a thread is switching of stack while the new stack is not yet
registered, the stack switching code will keep the old stack as current stack,
as the stack corresponding to the new sp cannot be found.
In such a case, the zone between the old and new SP in this unknown stack
can be marked either as addressable (if unknown stack is growing)
or unaddressable (if unknown stack is shrinking).
Then at some point in time, the new stack is registered.
If just after that, the sp is changed so as to grow the stack
by nr of bytes not determinable at translation time, VG_(unknown_SP_update)
will be called, will detect the stack switch and will do nothing.
This leaves a certain zone of the stack (the grown zone) in a not
addressable state, as the stack switch code has in fact wrongly
guessed a stack switch, while in fact what should have been detected
is just a sp change in a stack previously unknown.

Proper fixes might be:
 1. in "IF_STACK_SWITCH_SET_current_stack_AND_RETURN", do not return
    if old and new sp are in the stack stack.
    rather continue so as to execute correctly the sp change in
    the newly discovered stack.
and/or
 2. in the stack registration code (client request), if the current SP
    is inside the stack being registered, also set the current stack
    to the just registered stack pointer

None of these fixes are being looked at currently, as such changes looks
too adventurous close to 3.9.0

git-svn-id: svn://svn.valgrind.org/valgrind/trunk@13674

coregrind/m_main.c

index ee87d5218271995b4c3288dca1b1c739854e6494..35c11e1fb304cbf08733dbdb00d70dce10c9d6ea 100644 (file)
@@ -1530,19 +1530,6 @@ Int valgrind_main ( Int argc, HChar **argv, HChar **envp )
    struct vki_rlimit zero = { 0, 0 };
    XArray* addr2dihandle = NULL;
 
-   // For an inner Valgrind, register the interim stack asap.
-   // This is needed to allow the outer valgrind to do stacktraces during init.
-   // Note that this stack is not unregistered when the main thread
-   // is switching to the (real) stack. Unregistering this would imply
-   // to save the stack id in a global variable, and have a "if"
-   // in run_a_thread_NORETURN to do the unregistration only for the
-   // main thread. This unregistration is not worth this complexity.
-   INNER_REQUEST
-      ((void) VALGRIND_STACK_REGISTER
-       (&VG_(interim_stack).bytes[0],
-        &VG_(interim_stack).bytes[0] + sizeof(VG_(interim_stack))));
-
-
    //============================================================
    //
    // Nb: startup is complex.  Prerequisites are shown at every step.
@@ -3029,6 +3016,18 @@ void _start_in_C_linux ( UWord* pArgc )
    HChar** argv = (HChar**)&pArgc[1];
    HChar** envp = (HChar**)&pArgc[1+argc+1];
 
+   // For an inner Valgrind, register the interim stack asap.
+   // This is needed to allow the outer valgrind to do stacktraces during init.
+   // Note that this stack is not unregistered when the main thread
+   // is switching to the (real) stack. Unregistering this would imply
+   // to save the stack id in a global variable, and have a "if"
+   // in run_a_thread_NORETURN to do the unregistration only for the
+   // main thread. This unregistration is not worth this complexity.
+   INNER_REQUEST
+      ((void) VALGRIND_STACK_REGISTER
+       (&VG_(interim_stack).bytes[0],
+        &VG_(interim_stack).bytes[0] + sizeof(VG_(interim_stack))));
+
    VG_(memset)( &the_iicii, 0, sizeof(the_iicii) );
    VG_(memset)( &the_iifii, 0, sizeof(the_iifii) );
 
@@ -3162,6 +3161,12 @@ void _start_in_C_darwin ( UWord* pArgc )
    HChar** argv = (HChar**)&pArgc[1];
    HChar** envp = (HChar**)&pArgc[1+argc+1];
 
+   // See _start_in_C_linux
+   INNER_REQUEST
+      ((void) VALGRIND_STACK_REGISTER
+       (&VG_(interim_stack).bytes[0],
+        &VG_(interim_stack).bytes[0] + sizeof(VG_(interim_stack))));
+
    VG_(memset)( &the_iicii, 0, sizeof(the_iicii) );
    VG_(memset)( &the_iifii, 0, sizeof(the_iifii) );