* README_DEVELOPERS : complete/enhance the section about outer/inner
* 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 a5019735-40e9-0310-863c-91ae7b9d1cf9
diff --git a/README_DEVELOPERS b/README_DEVELOPERS
index b4b7d4f..a1a3503 100644
--- a/README_DEVELOPERS
+++ b/README_DEVELOPERS
@@ -149,10 +149,22 @@
(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
diff --git a/coregrind/m_redir.c b/coregrind/m_redir.c
index c996afb..9c7dbd6 100644
--- a/coregrind/m_redir.c
+++ b/coregrind/m_redir.c
@@ -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 @@
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);
diff --git a/coregrind/m_scheduler/sema.c b/coregrind/m_scheduler/sema.c
index 90097b7..7e4076c 100644
--- a/coregrind/m_scheduler/sema.c
+++ b/coregrind/m_scheduler/sema.c
@@ -34,6 +34,10 @@
#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 @@
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 @@
{
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 @@
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 @@
sema->owner_lwpid = 0;
+ INNER_REQUEST(ANNOTATE_RWLOCK_RELEASED(sema, /*is_w*/1));
ret = VG_(write)(sema->pipe[1], buf, 1);
if (ret != 1)
diff --git a/coregrind/m_scheduler/ticket-lock-linux.c b/coregrind/m_scheduler/ticket-lock-linux.c
index 64b5d4d..07a88a3 100644
--- a/coregrind/m_scheduler/ticket-lock-linux.c
+++ b/coregrind/m_scheduler/ticket-lock-linux.c
@@ -44,6 +44,10 @@
#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 @@
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 @@
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 @@
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];
diff --git a/coregrind/m_syswrap/syswrap-linux.c b/coregrind/m_syswrap/syswrap-linux.c
index cea1ffd..82bb8ab 100644
--- a/coregrind/m_syswrap/syswrap-linux.c
+++ b/coregrind/m_syswrap/syswrap-linux.c
@@ -54,6 +54,10 @@
#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 @@
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 @@
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 @@
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 @@
"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 */
diff --git a/docs/xml/manual-core.xml b/docs/xml/manual-core.xml
index 2d3d086..07488a4 100644
--- a/docs/xml/manual-core.xml
+++ b/docs/xml/manual-core.xml
@@ -1643,7 +1643,7 @@
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>
diff --git a/include/Makefile.am b/include/Makefile.am
index aaf64f0..8516492 100644
--- a/include/Makefile.am
+++ b/include/Makefile.am
@@ -15,6 +15,7 @@
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
index 0000000..909f637
--- /dev/null
+++ b/include/pub_tool_inner.h
@@ -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 ---*/
+/*--------------------------------------------------------------------*/