|
From: <sv...@va...> - 2012-03-03 12:06:23
|
Author: philippe
Date: 2012-03-03 12:01:48 +0000 (Sat, 03 Mar 2012)
New Revision: 12414
Log:
* 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.
Added:
trunk/include/pub_tool_inner.h
Modified:
trunk/README_DEVELOPERS
trunk/coregrind/m_redir.c
trunk/coregrind/m_scheduler/sema.c
trunk/coregrind/m_scheduler/ticket-lock-linux.c
trunk/coregrind/m_syswrap/syswrap-linux.c
trunk/docs/xml/manual-core.xml
trunk/include/Makefile.am
Modified: trunk/README_DEVELOPERS
===================================================================
--- trunk/README_DEVELOPERS 2012-03-02 09:57:14 UTC (rev 12413)
+++ trunk/README_DEVELOPERS 2012-03-03 12:01:48 UTC (rev 12414)
@@ -149,11 +149,23 @@
(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
its output (ie. those lines beginning with "==<pid>==") prefixed with a '>',
Modified: trunk/coregrind/m_redir.c
===================================================================
--- trunk/coregrind/m_redir.c 2012-03-02 09:57:14 UTC (rev 12413)
+++ trunk/coregrind/m_redir.c 2012-03-03 12:01:48 UTC (rev 12414)
@@ -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);
Modified: trunk/coregrind/m_scheduler/sema.c
===================================================================
--- trunk/coregrind/m_scheduler/sema.c 2012-03-02 09:57:14 UTC (rev 12413)
+++ trunk/coregrind/m_scheduler/sema.c 2012-03-03 12:01:48 UTC (rev 12414)
@@ -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)
Modified: trunk/coregrind/m_scheduler/ticket-lock-linux.c
===================================================================
--- trunk/coregrind/m_scheduler/ticket-lock-linux.c 2012-03-02 09:57:14 UTC (rev 12413)
+++ trunk/coregrind/m_scheduler/ticket-lock-linux.c 2012-03-03 12:01:48 UTC (rev 12414)
@@ -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];
Modified: trunk/coregrind/m_syswrap/syswrap-linux.c
===================================================================
--- trunk/coregrind/m_syswrap/syswrap-linux.c 2012-03-02 09:57:14 UTC (rev 12413)
+++ trunk/coregrind/m_syswrap/syswrap-linux.c 2012-03-03 12:01:48 UTC (rev 12414)
@@ -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 */
Modified: trunk/docs/xml/manual-core.xml
===================================================================
--- trunk/docs/xml/manual-core.xml 2012-03-02 09:57:14 UTC (rev 12413)
+++ trunk/docs/xml/manual-core.xml 2012-03-03 12:01:48 UTC (rev 12414)
@@ -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>
Modified: trunk/include/Makefile.am
===================================================================
--- trunk/include/Makefile.am 2012-03-02 09:57:14 UTC (rev 12413)
+++ trunk/include/Makefile.am 2012-03-03 12:01:48 UTC (rev 12414)
@@ -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 \
Added: trunk/include/pub_tool_inner.h
===================================================================
--- trunk/include/pub_tool_inner.h (rev 0)
+++ trunk/include/pub_tool_inner.h 2012-03-03 12:01:48 UTC (rev 12414)
@@ -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
+ phi...@sk...
+
+ 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 ---*/
+/*--------------------------------------------------------------------*/
|
|
From: Florian K. <br...@ac...> - 2012-03-03 15:43:23
|
On 03/03/2012 07:01 AM, sv...@va... wrote: > Modified: trunk/coregrind/m_scheduler/sema.c > =================================================================== > --- trunk/coregrind/m_scheduler/sema.c 2012-03-02 09:57:14 UTC (rev 12413) > +++ trunk/coregrind/m_scheduler/sema.c 2012-03-03 12:01:48 UTC (rev 12414) > @@ -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" I thought including tool headers in coregrind was forbidden. No ? Florian |
|
From: Julian S. <js...@ac...> - 2012-03-03 15:58:25
|
On Saturday, March 03, 2012, Florian Krohm wrote: > On 03/03/2012 07:01 AM, sv...@va... wrote: > > Modified: trunk/coregrind/m_scheduler/sema.c > > =================================================================== > > --- trunk/coregrind/m_scheduler/sema.c 2012-03-02 09:57:14 UTC (rev > > 12413) +++ trunk/coregrind/m_scheduler/sema.c 2012-03-03 12:01:48 UTC > > (rev 12414) @@ -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" > > I thought including tool headers in coregrind was forbidden. No ? It is forbidden [well spotted; I didn't :-]. In this case though it only happens when we're building V as an "inner" to be run by an "outer" V; in which case the inner needs to pass various hints to the outer by using client requests in the usual way. J |