]> git.ipfire.org Git - thirdparty/valgrind.git/commitdiff
* README_DEVELOPERS : complete/enhance the section about outer/inner
authorPhilippe Waroquiers <philippe.waroquiers@skynet.be>
Sat, 3 Mar 2012 12:01:48 +0000 (12:01 +0000)
committerPhilippe Waroquiers <philippe.waroquiers@skynet.be>
Sat, 3 Mar 2012 12:01:48 +0000 (12:01 +0000)
* manual-core.xml : fix a typo
* include/pub_tool_inner.h : new file, defining macros for inner annotation
  include/Makefile.am : reference this new file.
* syswrap-linux.c : when ENABLE_INNER, register the stacks for the outer.
   (otherwise, nothing works properly).
* m_redir.c : avoid inner interpreting the outer vgpreload instructions.
* sema.c : annotate the semaphore with RWLOCK annotations for helgrind
* ticket-lock-linux.c : similar.

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

README_DEVELOPERS
coregrind/m_redir.c
coregrind/m_scheduler/sema.c
coregrind/m_scheduler/ticket-lock-linux.c
coregrind/m_syswrap/syswrap-linux.c
docs/xml/manual-core.xml
include/Makefile.am
include/pub_tool_inner.h [new file with mode: 0644]

index b4b7d4ffa2d3df42cf13b97fc98f369e6fafe4b2..a1a35032b24802fbfc4fe47c9e4ffe5ddd774ed6 100644 (file)
@@ -149,10 +149,22 @@ To run Valgrind under Valgrind:
 (4) Choose a very simple program (date) and try
 
     outer/.../bin/valgrind --sim-hints=enable-outer --trace-children=yes  \
-       --tool=cachegrind -v inner/.../bin/valgrind --tool=none -v prog
+       --run-libc-freeres=no --tool=cachegrind -v \
+       inner/.../bin/valgrind --vgdb-prefix=./inner --tool=none -v prog
 
 If you omit the --trace-children=yes, you'll only monitor Inner's launcher
-program, not its stage2.
+program, not its stage2. Outer needs --run-libc-freeres=no, as otherwise
+it will try to find and run __libc_freeres in the inner, while libc is not
+used by the inner. Inner needs --vgdb-prefix=./inner to avoid inner
+gdbserver colliding with outer gdbserver.
+
+Debugging the whole thing might imply to use up to 3 GDB:
+  * a GDB attached to the Outer valgrind, allowing
+    to examine the state of Outer.
+  * a GDB using Outer gdbserver, allowing to
+    examine the state of Inner.
+  * a GDB using Inner gdbserver, allowing to
+    examine the state of prog.
 
 The whole thing is fragile, confusing and slow, but it does work well enough
 for you to get some useful performance data.  Inner has most of
index c996afb7235aaca4703e809e5dee5cfb7ebcce9a..9c7dbd66a4178e760af4f59bb9492e84226cc7ef 100644 (file)
@@ -36,6 +36,8 @@
 #include "pub_core_libcbase.h"
 #include "pub_core_libcassert.h"
 #include "pub_core_libcprint.h"
+#include "pub_core_vki.h"
+#include "pub_core_libcfile.h"
 #include "pub_core_seqmatch.h"
 #include "pub_core_mallocfree.h"
 #include "pub_core_options.h"
@@ -49,6 +51,7 @@
 #include "pub_core_xarray.h"
 #include "pub_core_clientstate.h"  // VG_(client___libc_freeres_wrapper)
 #include "pub_core_demangle.h"     // VG_(maybe_Z_demangle)
+#include "pub_core_libcproc.h"     // VG_(libdir)
 
 #include "config.h" /* GLIBC_2_* */
 
@@ -397,6 +400,82 @@ void VG_(redir_notify_new_DebugInfo)( DebugInfo* newdi )
    newdi_soname = VG_(DebugInfo_get_soname)(newdi);
    vg_assert(newdi_soname != NULL);
 
+#ifdef ENABLE_INNER
+   {
+      /* When an outer Valgrind is executing an inner Valgrind, the
+         inner "sees" in its address space the mmap-ed vgpreload files
+         of the outer.  The inner must avoid interpreting the
+         redirections given in the outer vgpreload mmap-ed files.
+         Otherwise, some tool combinations badly fail.
+
+         Example: outer memcheck tool executing an inner none tool.
+
+         If inner none interprets the outer malloc redirection, the
+         inner will redirect malloc to a memcheck function it does not
+         have (as the redirection target is from the outer).  With
+         such a failed redirection, a call to malloc inside the inner
+         will then result in a "no-operation" (and so no memory will
+         be allocated).
+
+         When running as an inner, no redirection will be done
+         for a vgpreload file if this file is not located in the
+         inner VALGRIND_LIB directory.
+
+         Recognising a vgpreload file based on a filename pattern
+         is a kludge. An alternate solution would be to change
+         the _vgr prefix according to outer/inner/client.
+      */
+      const UChar* newdi_filename = VG_(DebugInfo_get_filename)(newdi);
+      const UChar* newdi_basename = VG_(basename) (newdi_filename);
+      if (VG_(strncmp) (newdi_basename, "vgpreload_", 10) == 0) {
+         /* This looks like a vgpreload file => check if this file
+            is from the inner VALGRIND_LIB.
+            We do this check using VG_(stat) + dev/inode comparison
+            as vg-in-place defines a VALGRIND_LIB with symlinks
+            pointing to files inside the valgrind build directories. */
+         struct vg_stat newdi_stat;
+         SysRes newdi_res;
+         Char in_vglib_filename[VKI_PATH_MAX];
+         struct vg_stat in_vglib_stat;
+         SysRes in_vglib_res;
+
+         newdi_res = VG_(stat)(newdi_filename, &newdi_stat);
+         
+         VG_(strncpy) (in_vglib_filename, VG_(libdir), VKI_PATH_MAX);
+         VG_(strncat) (in_vglib_filename, "/", VKI_PATH_MAX);
+         VG_(strncat) (in_vglib_filename, newdi_basename, VKI_PATH_MAX);
+         in_vglib_res = VG_(stat)(in_vglib_filename, &in_vglib_stat);
+
+         /* If we find newdi_basename in inner VALGRIND_LIB
+            but newdi_filename is not the same file, then we do
+            not execute the redirection. */
+         if (!sr_isError(in_vglib_res)
+             && !sr_isError(newdi_res)
+             && (newdi_stat.dev != in_vglib_stat.dev 
+                 || newdi_stat.ino != in_vglib_stat.ino)) {
+            /* <inner VALGRIND_LIB>/newdi_basename is an existing file
+               and is different of newdi_filename.
+               So, we do not execute newdi_filename redirection. */
+            if ( VG_(clo_verbosity) > 1 ) {
+               VG_(message)( Vg_DebugMsg,
+                             "Skipping vgpreload redir in %s"
+                             " (not from VALGRIND_LIB_INNER)\n",
+                             newdi_filename);
+            }
+            return;
+         } else {
+            if ( VG_(clo_verbosity) > 1 ) {
+               VG_(message)( Vg_DebugMsg,
+                             "Executing vgpreload redir in %s"
+                             " (from VALGRIND_LIB_INNER)\n",
+                             newdi_filename);
+            }
+         }
+      }
+   }
+#endif
+
+
    /* stay sane: we don't already have this. */
    for (ts = topSpecs; ts; ts = ts->next)
       vg_assert(ts->seginfo != newdi);
index 90097b70dd2e292f354ebedeb6262941508b2db0..7e4076c4ab6caed368db0c8a4b12bc85c4aafeac 100644 (file)
 #include "pub_core_libcassert.h"
 #include "pub_core_libcfile.h"
 #include "pub_core_libcproc.h"      // For VG_(gettid)()
+#include "pub_tool_inner.h"
+#if defined(ENABLE_INNER_CLIENT_REQUEST)
+#include "helgrind/helgrind.h"
+#endif
 #include "priv_sema.h"
 
 /* 
@@ -72,6 +76,7 @@ void ML_(sema_init)(vg_sema_t *sema)
    buf[0] = sema_char; 
    buf[1] = 0;
    sema_char++;
+   INNER_REQUEST(ANNOTATE_RWLOCK_CREATE(sema));
    res = VG_(write)(sema->pipe[1], buf, 1);
    vg_assert(res == 1);
 }
@@ -80,6 +85,7 @@ void ML_(sema_deinit)(vg_sema_t *sema)
 {
    vg_assert(sema->owner_lwpid != -1); /* must be initialised */
    vg_assert(sema->pipe[0] != sema->pipe[1]);
+   INNER_REQUEST(ANNOTATE_RWLOCK_DESTROY(sema));
    VG_(close)(sema->pipe[0]);
    VG_(close)(sema->pipe[1]);
    sema->pipe[0] = sema->pipe[1] = -1;
@@ -99,6 +105,7 @@ void ML_(sema_down)( vg_sema_t *sema, Bool as_LL )
   again:
    buf[0] = buf[1] = 0;
    ret = VG_(read)(sema->pipe[0], buf, 1);
+   INNER_REQUEST(ANNOTATE_RWLOCK_ACQUIRED(sema, /*is_w*/1));
 
    if (ret != 1) 
       VG_(debugLog)(0, "scheduler", 
@@ -131,6 +138,7 @@ void ML_(sema_up)( vg_sema_t *sema, Bool as_LL )
 
    sema->owner_lwpid = 0;
 
+   INNER_REQUEST(ANNOTATE_RWLOCK_RELEASED(sema, /*is_w*/1));
    ret = VG_(write)(sema->pipe[1], buf, 1);
 
    if (ret != 1) 
index 64b5d4d3f21912c33720af964697680e62ef39c0..07a88a388986d283f27de4b2b2be7c8d1317b63d 100644 (file)
 #include "pub_tool_libcproc.h"
 #include "pub_tool_mallocfree.h"
 #include "pub_tool_threadstate.h"
+#include "pub_tool_inner.h"
+#if defined(ENABLE_INNER_CLIENT_REQUEST)
+#include "helgrind/helgrind.h"
+#endif
 #include "priv_sched-lock.h"
 #include "priv_sched-lock-impl.h"
 
@@ -83,11 +87,13 @@ static struct sched_lock *create_sched_lock(void)
       VG_(memset)((void*)p->futex, 0, sizeof(p->futex));
       p->owner = 0;
    }
+   INNER_REQUEST(ANNOTATE_RWLOCK_CREATE(p));
    return p;
 }
 
 static void destroy_sched_lock(struct sched_lock *p)
 {
+   INNER_REQUEST(ANNOTATE_RWLOCK_DESTROY(p));
    VG_(free)(p);
 }
 
@@ -135,6 +141,7 @@ static void acquire_sched_lock(struct sched_lock *p)
          vg_assert(False);
       }
    }
+   INNER_REQUEST(ANNOTATE_RWLOCK_ACQUIRED(p, /*is_w*/1));
    vg_assert(p->owner == 0);
    p->owner = VG_(gettid)();
 }
@@ -156,6 +163,7 @@ static void release_sched_lock(struct sched_lock *p)
 
    vg_assert(p->owner != 0);
    p->owner = 0;
+   INNER_REQUEST(ANNOTATE_RWLOCK_RELEASED(p, /*is_w*/1));
    wakeup_ticket = __sync_fetch_and_add(&p->head, 1) + 1;
    if (p->tail != wakeup_ticket) {
       futex = &p->futex[wakeup_ticket & TL_FUTEX_MASK];
index cea1ffdb96615b942c4b7a617b3763e6d7615b5d..82bb8ab9f0679d854d9926e951b923eb610e6b38 100644 (file)
 #include "pub_core_signals.h"
 #include "pub_core_syscall.h"
 #include "pub_core_syswrap.h"
+#include "pub_tool_inner.h"
+#if defined(ENABLE_INNER_CLIENT_REQUEST)
+#include "valgrind.h"
+#endif
 
 #include "priv_types_n_macros.h"
 #include "priv_syswrap-generic.h"
@@ -123,6 +127,9 @@ static void run_a_thread_NORETURN ( Word tidW )
    VgSchedReturnCode src;
    Int               c;
    ThreadState*      tst;
+#ifdef ENABLE_INNER_CLIENT_REQUEST
+   Int               registered_vgstack_id;
+#endif
 
    VG_(debugLog)(1, "syswrap-linux", 
                     "run_a_thread_NORETURN(tid=%lld): pre-thread_wrapper\n",
@@ -131,6 +138,19 @@ static void run_a_thread_NORETURN ( Word tidW )
    tst = VG_(get_ThreadState)(tid);
    vg_assert(tst);
 
+   /* An thread has two stacks:
+      * the simulated stack (used by the synthetic cpu. Guest process
+        is using this stack).
+      * the valgrind stack (used by the real cpu. Valgrind code is running
+        on this stack).
+      When Valgrind runs as an inner, it must signals that its (real) stack
+      is the stack to use by the outer to e.g. do stacktraces.
+   */
+   INNER_REQUEST
+      (registered_vgstack_id 
+       = VALGRIND_STACK_REGISTER (tst->os_state.valgrind_stack_base,
+                                  tst->os_state.valgrind_stack_init_SP));
+   
    /* Run the thread all the way through. */
    src = thread_wrapper(tid);  
 
@@ -190,6 +210,8 @@ static void run_a_thread_NORETURN ( Word tidW )
       VG_(exit_thread)(tid);
       vg_assert(tst->status == VgTs_Zombie);
 
+      INNER_REQUEST (VALGRIND_STACK_DEREGISTER (registered_vgstack_id));
+
       /* We have to use this sequence to terminate the thread to
          prevent a subtle race.  If VG_(exit_thread)() had left the
          ThreadState as Empty, then it could have been reallocated,
@@ -320,6 +342,18 @@ void VG_(main_thread_wrapper_NORETURN)(ThreadId tid)
                     "entering VG_(main_thread_wrapper_NORETURN)\n");
 
    sp = ML_(allocstack)(tid);
+#if defined(ENABLE_INNER_CLIENT_REQUEST)
+   {
+      // we must register the main thread stack before the call
+      // to ML_(call_on_new_stack_0_1), otherwise the outer valgrind
+      // reports 'write error' on the non registered stack.
+      ThreadState* tst = VG_(get_ThreadState)(tid);
+      INNER_REQUEST
+         ((void) 
+          VALGRIND_STACK_REGISTER (tst->os_state.valgrind_stack_base,
+                                   tst->os_state.valgrind_stack_init_SP));
+   }
+#endif
 
 #if defined(VGP_ppc32_linux)
    /* make a stack frame */
index 2d3d086afccbe6b731265b5fcc1773d55ef8e787..07488a47c32b8695437b31c126d1e32836895b78 100644 (file)
@@ -1643,7 +1643,7 @@ need to use these.</para>
           tiresome.</para>
         </listitem>
         <listitem>
-          <para><option>enable-inner: </option> Enable some special
+          <para><option>enable-outer: </option> Enable some special
           magic needed when the program being run is itself
           Valgrind.</para>
         </listitem>
index aaf64f0a04585e4dc36422037db69677b22c5110..851649251fbed119502af8b236439dee97150561 100644 (file)
@@ -15,6 +15,7 @@ nobase_pkginclude_HEADERS = \
        pub_tool_gdbserver.h            \
        pub_tool_poolalloc.h            \
        pub_tool_hashtable.h            \
+       pub_tool_inner.h                \
        pub_tool_libcbase.h             \
        pub_tool_libcassert.h           \
        pub_tool_libcfile.h             \
diff --git a/include/pub_tool_inner.h b/include/pub_tool_inner.h
new file mode 100644 (file)
index 0000000..909f637
--- /dev/null
@@ -0,0 +1,72 @@
+
+/*--------------------------------------------------------------------*/
+/*--- Utilities for inner Valgrind                pub_tool_inner.h ---*/
+/*--------------------------------------------------------------------*/
+
+/*
+   This file is part of Valgrind, a dynamic binary instrumentation
+   framework.
+
+   Copyright (C) 2012-2012 Philippe Waroquiers
+      philippe.waroquiers@skynet.be
+
+   This program is free software; you can redistribute it and/or
+   modify it under the terms of the GNU General Public License as
+   published by the Free Software Foundation; either version 2 of the
+   License, or (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful, but
+   WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software
+   Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
+   02111-1307, USA.
+
+   The GNU General Public License is contained in the file COPYING.
+*/
+
+#ifndef __PUB_TOOL_INNER_H
+#define __PUB_TOOL_INNER_H
+
+//--------------------------------------------------------------------
+// PURPOSE: This header should be imported by every  file in Valgrind
+// which needs specific behaviour when running as an "inner" Valgrind.
+// Valgrind can self-host itself (i.e. Valgrind can run Valgrind) :
+// The outer Valgrind executes the inner Valgrind.
+// For more details, see README_DEVELOPPERS.
+//--------------------------------------------------------------------
+
+#include "config.h" 
+
+// The code of the inner Valgrind (core or tool code) contains client
+// requests (e.g. from helgrind.h, memcheck.h, ...) to help the
+// outer Valgrind finding (relevant) errors in the inner Valgrind.
+// Such client requests should only be compiled in for an inner Valgrind.
+// Use the macro INNER_REQUEST to allow a central enabling/disabling
+// of these client requests.
+#if defined(ENABLE_INNER)
+
+// By default, the client requests 
+// undefine the below to have an inner Valgrind without any annotation.
+#define ENABLE_INNER_CLIENT_REQUEST 1
+
+#if defined(ENABLE_INNER_CLIENT_REQUEST)
+#define INNER_REQUEST(a) a
+#else
+#define INNER_REQUEST(a) 0
+#endif
+
+#else
+
+#define INNER_REQUEST(a) 0
+
+#endif
+
+#endif   // __PUB_TOOL_INNER_H
+
+/*--------------------------------------------------------------------*/
+/*--- end                                                          ---*/
+/*--------------------------------------------------------------------*/