From: Julian Seward Date: Tue, 7 Jun 2011 21:39:28 +0000 (+0000) Subject: Add a fourth --smc-check= variant, --smc-check=all-non-file. This X-Git-Tag: svn/VALGRIND_3_7_0~439 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=da49be7c1464d7396822bccbe2c9ce08cd44e9c5;p=thirdparty%2Fvalgrind.git Add a fourth --smc-check= variant, --smc-check=all-non-file. This adds self-modifying-code checks to all guest code taken from mappings which are not file backed, but omits checks in code from file backed mappings. This has the effect of giving complete smc-coverage of JIT generated code -- since that is invariably generated into anonymous mapped areas -- without burdening non-JIT generated code with such checks. Running Firefox 6, --smc-check=all-non-file reduces by a factor of between 3 and 10 the number of translations requiring a self check, compared to --smc-check=all. These changes depend on the vex interface changes in r2158. git-svn-id: svn://svn.valgrind.org/valgrind/trunk@11798 --- diff --git a/NEWS b/NEWS index 2388a8d7fe..1c883bf060 100644 --- a/NEWS +++ b/NEWS @@ -4,6 +4,7 @@ Release 3.7.0 (???) - Added the --mod-funcname option to cg_diff. - Further reduction in overheads caused by --smc-check=all, especially on 64-bit targets. +- new variant --smc-check=all-non-file * IBM z/Architecture (s390x) running Linux Valgrind can analyse 64-bit programs running on z/Architecture. diff --git a/coregrind/m_main.c b/coregrind/m_main.c index b27b5adb27..1bf66c6174 100644 --- a/coregrind/m_main.c +++ b/coregrind/m_main.c @@ -171,8 +171,10 @@ static void usage_NORETURN ( Bool debug_help ) " part of the path after 'string'. Allows removal\n" " of path prefixes. Use this flag multiple times\n" " to specify a set of prefixes to remove.\n" -" --smc-check=none|stack|all checks for self-modifying code: none,\n" -" only for code found in stacks, or all [stack]\n" +" --smc-check=none|stack|all|all-non-file [stack]\n" +" checks for self-modifying code: none, only for\n" +" code found in stacks, for all code, or for all\n" +" code except that from file-backed mappings\n" " --read-var-info=yes|no read debug info on stack and global variables\n" " and use it to print better error messages in\n" " tools that make use of it (Memcheck, Helgrind,\n" @@ -519,6 +521,9 @@ void main_process_cmd_line_options ( /*OUT*/Bool* logging_to_fd, Vg_SmcStack); else if VG_XACT_CLO(arg, "--smc-check=all", VG_(clo_smc_check), Vg_SmcAll); + else if VG_XACT_CLO(arg, "--smc-check=all-non-file", + VG_(clo_smc_check), + Vg_SmcAllNonFile); else if VG_STR_CLO (arg, "--kernel-variant", VG_(clo_kernel_variant)) {} diff --git a/coregrind/m_translate.c b/coregrind/m_translate.c index 36e5d8da78..473d3cf465 100644 --- a/coregrind/m_translate.c +++ b/coregrind/m_translate.c @@ -729,41 +729,87 @@ static Bool translations_allowable_from_seg ( NSegment const* seg ) } -/* Is a self-check required for a translation of a guest address - inside segment SEG when requested by thread TID ? */ +/* Produce a bitmask stating which of the supplied extents needs a + self-check. See documentation of + VexTranslateArgs::needs_self_check for more details about the + return convention. */ -static Bool self_check_required ( NSegment const* seg, ThreadId tid ) +static UInt needs_self_check ( void* closureV, + VexGuestExtents* vge ) { -#if defined(VGO_darwin) - // GrP fixme hack - dyld i386 IMPORT gets rewritten - // to really do this correctly, we'd need to flush the - // translation cache whenever a segment became +WX - if (seg->hasX && seg->hasW) { - return True; - } -#endif - switch (VG_(clo_smc_check)) { - case Vg_SmcNone: return False; - case Vg_SmcAll: return True; - case Vg_SmcStack: - return seg - ? (seg->start <= VG_(get_SP)(tid) - && VG_(get_SP)(tid)+sizeof(Word)-1 <= seg->end) - : False; - break; - default: - vg_assert2(0, "unknown VG_(clo_smc_check) value"); + VgCallbackClosure* closure = (VgCallbackClosure*)closureV; + UInt i, bitset; + + vg_assert(vge->n_used >= 1 && vge->n_used <= 3); + bitset = 0; + + for (i = 0; i < vge->n_used; i++) { + Bool check = False; + Addr addr = (Addr)vge->base[i]; + SizeT len = (SizeT)vge->len[i]; + NSegment const* segA = NULL; + +# if defined(VGO_darwin) + // GrP fixme hack - dyld i386 IMPORT gets rewritten. + // To really do this correctly, we'd need to flush the + // translation cache whenever a segment became +WX. + segA = VG_(am_find_nsegment)(addr); + if (segA && segA->hasX && segA->hasW) + check = True; +# endif + + if (!check) { + switch (VG_(clo_smc_check)) { + case Vg_SmcNone: + /* never check (except as per Darwin hack above) */ + break; + case Vg_SmcAll: + /* always check */ + check = True; + break; + case Vg_SmcStack: { + /* check if the address is in the same segment as this + thread's stack pointer */ + Addr sp = VG_(get_SP)(closure->tid); + if (!segA) { + segA = VG_(am_find_nsegment)(addr); + } + NSegment const* segSP = VG_(am_find_nsegment)(sp); + if (segA && segSP && segA == segSP) + check = True; + break; + } + case Vg_SmcAllNonFile: { + /* check if any part of the extent is not in a + file-mapped segment */ + if (!segA) { + segA = VG_(am_find_nsegment)(addr); + } + if (segA && segA->kind == SkFileC && segA->start <= addr + && (len == 0 || addr + len <= segA->end + 1)) { + /* in a file-mapped segment; skip the check */ + } else { + check = True; + } + break; + } + default: + vg_assert(0); + } + } + + if (check) + bitset |= (1 << i); } + + return bitset; } /* This is a callback passed to LibVEX_Translate. It stops Vex from chasing into function entry points that we wish to redirect. Chasing across them obviously defeats the redirect mechanism, with - bad effects for Memcheck, Addrcheck, and possibly others. - - Also, we must stop Vex chasing into blocks for which we might want - to self checking. + bad effects for Memcheck, Helgrind, DRD, Massif, and possibly others. */ static Bool chase_into_ok ( void* closureV, Addr64 addr64 ) { @@ -1284,7 +1330,6 @@ Bool VG_(translate) ( ThreadId tid, Addr64 addr; T_Kind kind; Int tmpbuf_used, verbosity, i; - Bool do_self_check; Bool (*preamble_fn)(void*,IRSB*); VexArch vex_arch; VexArchInfo vex_archinfo; @@ -1392,9 +1437,6 @@ Bool VG_(translate) ( ThreadId tid, return False; } - /* Do we want a self-checking translation? */ - do_self_check = self_check_required( seg, tid ); - /* True if a debug trans., or if bit N set in VG_(clo_trace_codegen). */ verbosity = 0; if (debugging_translation) { @@ -1509,7 +1551,7 @@ Bool VG_(translate) ( ThreadId tid, vta.finaltidy = VG_(needs).final_IR_tidy_pass ? VG_(tdict).tool_final_IR_tidy_pass : NULL; - vta.do_self_check = do_self_check; + vta.needs_self_check = needs_self_check; vta.traceflags = verbosity; /* Set up the dispatch-return info. For archs without a link @@ -1557,7 +1599,8 @@ Bool VG_(translate) ( ThreadId tid, /* Sheesh. Finally, actually _do_ the translation! */ tres = LibVEX_Translate ( &vta ); - vg_assert(tres == VexTransOK); + vg_assert(tres.status == VexTransOK); + vg_assert(tres.n_sc_extents >= 0 && tres.n_sc_extents <= 3); vg_assert(tmpbuf_used <= N_TMPBUF); vg_assert(tmpbuf_used > 0); @@ -1594,7 +1637,7 @@ Bool VG_(translate) ( ThreadId tid, nraddr, (Addr)(&tmpbuf[0]), tmpbuf_used, - do_self_check ); + tres.n_sc_extents > 0 ); } else { VG_(add_to_unredir_transtab)( &vge, nraddr, diff --git a/coregrind/pub_core_options.h b/coregrind/pub_core_options.h index bb7cb6c4d4..7706d8ee19 100644 --- a/coregrind/pub_core_options.h +++ b/coregrind/pub_core_options.h @@ -228,7 +228,9 @@ typedef Vg_SmcNone, // never generate self-checking translations Vg_SmcStack, // generate s-c-t's for code found in stacks // (this is the default) - Vg_SmcAll // make all translations self-checking. + Vg_SmcAll, // make all translations self-checking. + Vg_SmcAllNonFile // make all translations derived from + // non-file-backed memory self checking } VgSmc; diff --git a/docs/xml/manual-core.xml b/docs/xml/manual-core.xml index 3666aa51ba..7b57004497 100644 --- a/docs/xml/manual-core.xml +++ b/docs/xml/manual-core.xml @@ -1444,7 +1444,7 @@ need to use these. - + This option controls Valgrind's detection of self-modifying @@ -1453,33 +1453,50 @@ need to use these. continue to execute the translations it made for the old code. This will likely lead to incorrect behaviour and/or crashes. - Valgrind has three levels of self-modifying code detection: + Valgrind has four levels of self-modifying code detection: no detection, detect self-modifying code on the stack (which is used by - GCC to implement nested functions), or detect self-modifying code - everywhere. Note that the default option will catch the vast majority + GCC to implement nested functions), detect self-modifying code + everywhere, and detect self-modifying code everywhere except in + file-backed mappings. + + Note that the default option will catch the vast majority of cases. The main case it will not catch is programs such as JIT compilers that dynamically generate code and subsequently overwrite part or all of it. Running with - all will slow Valgrind down noticeably. Running with + all will slow Valgrind down noticeably. + Running with none will rarely speed things up, since very little code gets put on the stack for most programs. The - VALGRIND_DISCARD_TRANSLATIONS client request is - an alternative to that requires more - effort but is much faster. + VALGRIND_DISCARD_TRANSLATIONS client + request is an alternative to + that requires more programmer effort but allows Valgrind to run + your program faster, by telling it precisely when translations + need to be re-made. + provides a + cheaper but more limited version + of . It adds checks to any + translations that do not originate from file-backed memory + mappings. Typical applications that generate code, for example + JITs in web browsers, generate code into anonymous mmaped areas, + whereas the "fixed" code of the browser always lives in + file-backed mappings. + takes advantage of this observation, limiting the overhead of + checking to code which is likely to be JIT generated. + Some architectures (including ppc32, ppc64 and ARM) require programs which create code at runtime to flush the instruction cache in between code generation and first use. Valgrind observes and honours such instructions. Hence, on ppc32/Linux, ppc64/Linux and ARM/Linux, Valgrind always provides complete, transparent support for self-modifying code. It is only on platforms such as - x86/Linux, AMD64/Linux and x86/Darwin that you need to use this - option. + x86/Linux, AMD64/Linux, x86/Darwin and AMD64/Darwin + that you need to use this option. diff --git a/none/tests/cmdline1.stdout.exp b/none/tests/cmdline1.stdout.exp index 641304a894..6faa70f3b6 100644 --- a/none/tests/cmdline1.stdout.exp +++ b/none/tests/cmdline1.stdout.exp @@ -57,8 +57,10 @@ usage: valgrind [options] prog-and-args part of the path after 'string'. Allows removal of path prefixes. Use this flag multiple times to specify a set of prefixes to remove. - --smc-check=none|stack|all checks for self-modifying code: none, - only for code found in stacks, or all [stack] + --smc-check=none|stack|all|all-non-file [stack] + checks for self-modifying code: none, only for + code found in stacks, for all code, or for all + code except that from file-backed mappings --read-var-info=yes|no read debug info on stack and global variables and use it to print better error messages in tools that make use of it (Memcheck, Helgrind, diff --git a/none/tests/cmdline2.stdout.exp b/none/tests/cmdline2.stdout.exp index e66858d385..a65047e069 100644 --- a/none/tests/cmdline2.stdout.exp +++ b/none/tests/cmdline2.stdout.exp @@ -57,8 +57,10 @@ usage: valgrind [options] prog-and-args part of the path after 'string'. Allows removal of path prefixes. Use this flag multiple times to specify a set of prefixes to remove. - --smc-check=none|stack|all checks for self-modifying code: none, - only for code found in stacks, or all [stack] + --smc-check=none|stack|all|all-non-file [stack] + checks for self-modifying code: none, only for + code found in stacks, for all code, or for all + code except that from file-backed mappings --read-var-info=yes|no read debug info on stack and global variables and use it to print better error messages in tools that make use of it (Memcheck, Helgrind,