From: Philippe Waroquiers Date: Sat, 3 Mar 2012 12:01:48 +0000 (+0000) Subject: * README_DEVELOPERS : complete/enhance the section about outer/inner X-Git-Tag: svn/VALGRIND_3_8_0~439 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=74d44d1764da3dff758501aa686d3729c16e29be;p=thirdparty%2Fvalgrind.git * 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 --- diff --git a/README_DEVELOPERS b/README_DEVELOPERS index b4b7d4ffa2..a1a35032b2 100644 --- a/README_DEVELOPERS +++ b/README_DEVELOPERS @@ -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 diff --git a/coregrind/m_redir.c b/coregrind/m_redir.c index c996afb723..9c7dbd66a4 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 @@ 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)) { + /* /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 90097b70dd..7e4076c4ab 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 @@ 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) diff --git a/coregrind/m_scheduler/ticket-lock-linux.c b/coregrind/m_scheduler/ticket-lock-linux.c index 64b5d4d3f2..07a88a3889 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 @@ 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]; diff --git a/coregrind/m_syswrap/syswrap-linux.c b/coregrind/m_syswrap/syswrap-linux.c index cea1ffdb96..82bb8ab9f0 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 @@ 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 */ diff --git a/docs/xml/manual-core.xml b/docs/xml/manual-core.xml index 2d3d086afc..07488a47c3 100644 --- a/docs/xml/manual-core.xml +++ b/docs/xml/manual-core.xml @@ -1643,7 +1643,7 @@ need to use these. tiresome. - Enable some special + Enable some special magic needed when the program being run is itself Valgrind. diff --git a/include/Makefile.am b/include/Makefile.am index aaf64f0a04..851649251f 100644 --- a/include/Makefile.am +++ b/include/Makefile.am @@ -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 index 0000000000..909f637c22 --- /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 ---*/ +/*--------------------------------------------------------------------*/