From: Julian Seward Date: Tue, 18 Oct 2016 17:16:11 +0000 (+0000) Subject: Add to Memcheck a flag --ignore-range-below-sp=-, for X-Git-Tag: svn/VALGRIND_3_13_0~332 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=766292973dfb9741d9a7fd4fccd47c07b60987ce;p=thirdparty%2Fvalgrind.git Add to Memcheck a flag --ignore-range-below-sp=-, for ignoring accesses on the stack below SP. Serves as a more modern replacement for --workaround-gcc296-bugs, which is now deprecated. Fixes #360571. git-svn-id: svn://svn.valgrind.org/valgrind/trunk@16073 --- diff --git a/coregrind/m_libcbase.c b/coregrind/m_libcbase.c index 29ede3b48e..220247f327 100644 --- a/coregrind/m_libcbase.c +++ b/coregrind/m_libcbase.c @@ -511,6 +511,25 @@ Bool VG_(parse_Addr) ( const HChar** ppc, Addr* result ) return True; } +Bool VG_(parse_UInt) ( const HChar** ppc, UInt* result ) +{ + ULong res64 = 0; + Int used, limit = 10; + used = 0; + while (VG_(isdigit)(**ppc)) { + res64 = res64 * 10 + ((ULong)(**ppc)) - (ULong)'0'; + (*ppc)++; + used++; + if (used > limit) return False; + } + if (used == 0) + return False; + if ((res64 >> 32) != 0) + return False; + *result = (UInt)res64; + return True; +} + Bool VG_(parse_enum_set) ( const HChar *tokens, Bool allow_all, const HChar *input, diff --git a/include/pub_tool_libcbase.h b/include/pub_tool_libcbase.h index c22bd77e63..34331730ee 100644 --- a/include/pub_tool_libcbase.h +++ b/include/pub_tool_libcbase.h @@ -101,11 +101,15 @@ extern HChar* VG_(strtok_r) (HChar* s, const HChar* delim, HChar** saveptr extern HChar* VG_(strtok) (HChar* s, const HChar* delim); /* Parse a 32- or 64-bit hex number, including leading 0x, from string - starting at *ppc, putting result in *result, and return True. Or - fail, in which case *ppc and *result are undefined, and return - False. */ + starting at *ppc, putting result in *result, advance *ppc past the + characters used, and return True. Or fail, in which case *ppc and + *result are undefined, and return False. */ extern Bool VG_(parse_Addr) ( const HChar** ppc, Addr* result ); +/* Parse an unsigned 32 bit number, written using decimals only. + Calling conventions are the same as for VG_(parse_Addr). */ +extern Bool VG_(parse_UInt) ( const HChar** ppc, UInt* result ); + /* Parse an "enum set" made of one or more words comma separated. The allowed word values are given in 'tokens', separated by comma. If a word in 'tokens' is found in 'input', the corresponding bit diff --git a/memcheck/docs/mc-manual.xml b/memcheck/docs/mc-manual.xml index e8e08e2586..0fdd266d6d 100644 --- a/memcheck/docs/mc-manual.xml +++ b/memcheck/docs/mc-manual.xml @@ -1107,9 +1107,38 @@ is conversions. This is in violation of the 32-bit PowerPC ELF specification, which makes no provision for locations below the stack pointer to be accessible. + + This option is deprecated as of version 3.12 and may be + removed from future versions. You should instead use + to specify the exact + range of offsets below the stack pointer that should be ignored. + A suitable equivalent + is . + + + + + + + This is a more general replacement for the deprecated + option. When + specified, it causes Memcheck not to report errors for accesses + at the specified offsets below the stack pointer. The two + offsets must be positive decimal numbers and -- somewhat + counterintuitively -- the first one must be larger, in order to + imply a non-wraparound address range to ignore. For example, + to ignore 4 byte accesses at 8192 bytes below the stack + pointer, + use . Only + one range may be specified. + + + + diff --git a/memcheck/mc_errors.c b/memcheck/mc_errors.c index 4c13e2d438..07d4c7bd89 100644 --- a/memcheck/mc_errors.c +++ b/memcheck/mc_errors.c @@ -746,13 +746,20 @@ void MC_(record_address_error) ( ThreadId tid, Addr a, Int szB, if (VG_(is_watched)( (isWrite ? write_watchpoint : read_watchpoint), a, szB)) return; - just_below_esp = is_just_below_ESP( VG_(get_SP)(tid), a ); + Addr current_sp = VG_(get_SP)(tid); + just_below_esp = is_just_below_ESP( current_sp, a ); /* If this is caused by an access immediately below %ESP, and the user asks nicely, we just ignore it. */ if (MC_(clo_workaround_gcc296_bugs) && just_below_esp) return; + /* Also, if this is caused by an access in the range of offsets + below the stack pointer as described by + --ignore-range-below-sp, ignore it. */ + if (MC_(in_ignored_range_below_sp)( current_sp, a, szB )) + return; + extra.Err.Addr.isWrite = isWrite; extra.Err.Addr.szB = szB; extra.Err.Addr.maybe_gcc = just_below_esp; diff --git a/memcheck/mc_include.h b/memcheck/mc_include.h index 6a81ad4021..d36ea0ba1e 100644 --- a/memcheck/mc_include.h +++ b/memcheck/mc_include.h @@ -572,6 +572,10 @@ void MC_(pp_describe_addr) (Addr a); /* Is this address in a user-specified "ignored range" ? */ Bool MC_(in_ignored_range) ( Addr a ); +/* Is this address in a user-specified "ignored range of offsets below + the current thread's stack pointer?" */ +Bool MC_(in_ignored_range_below_sp) ( Addr sp, Addr a, UInt szB ); + /*------------------------------------------------------------*/ /*--- Client blocks ---*/ @@ -715,6 +719,12 @@ extern Bool MC_(clo_show_mismatched_frees); operations? Default: NO */ extern Bool MC_(clo_expensive_definedness_checks); +/* Do we have a range of stack offsets to ignore? Default: NO */ +extern Bool MC_(clo_ignore_range_below_sp); +extern UInt MC_(clo_ignore_range_below_sp__first_offset); +extern UInt MC_(clo_ignore_range_below_sp__last_offset); + + /*------------------------------------------------------------*/ /*--- Instrumentation ---*/ /*------------------------------------------------------------*/ diff --git a/memcheck/mc_main.c b/memcheck/mc_main.c index 7e6b087dcd..30fa0084e9 100644 --- a/memcheck/mc_main.c +++ b/memcheck/mc_main.c @@ -1121,9 +1121,32 @@ Bool MC_(in_ignored_range) ( Addr a ) /*NOTREACHED*/ } -/* Parse two Addr separated by a dash, or fail. */ +Bool MC_(in_ignored_range_below_sp) ( Addr sp, Addr a, UInt szB ) +{ + if (LIKELY(!MC_(clo_ignore_range_below_sp))) + return False; + tl_assert(szB >= 1 && szB <= 32); + tl_assert(MC_(clo_ignore_range_below_sp__first_offset) + > MC_(clo_ignore_range_below_sp__last_offset)); + Addr range_lo = sp - MC_(clo_ignore_range_below_sp__first_offset); + Addr range_hi = sp - MC_(clo_ignore_range_below_sp__last_offset); + if (range_lo >= range_hi) { + /* Bizarre. We have a wraparound situation. What should we do? */ + return False; // Play safe + } else { + /* This is the expected case. */ + if (range_lo <= a && a + szB - 1 <= range_hi) + return True; + else + return False; + } + /*NOTREACHED*/ + tl_assert(0); +} + +/* Parse two Addrs (in hex) separated by a dash, or fail. */ -static Bool parse_range ( const HChar** ppc, Addr* result1, Addr* result2 ) +static Bool parse_Addr_pair ( const HChar** ppc, Addr* result1, Addr* result2 ) { Bool ok = VG_(parse_Addr) (ppc, result1); if (!ok) @@ -1137,6 +1160,23 @@ static Bool parse_range ( const HChar** ppc, Addr* result1, Addr* result2 ) return True; } +/* Parse two UInts (32 bit unsigned, in decimal) separated by a dash, + or fail. */ + +static Bool parse_UInt_pair ( const HChar** ppc, UInt* result1, UInt* result2 ) +{ + Bool ok = VG_(parse_UInt) (ppc, result1); + if (!ok) + return False; + if (**ppc != '-') + return False; + (*ppc)++; + ok = VG_(parse_UInt) (ppc, result2); + if (!ok) + return False; + return True; +} + /* Parse a set of ranges separated by commas into 'ignoreRanges', or fail. If they are valid, add them to the global set of ignored ranges. */ @@ -1148,7 +1188,7 @@ static Bool parse_ignore_ranges ( const HChar* str0 ) while (1) { Addr start = ~(Addr)0; Addr end = (Addr)0; - Bool ok = parse_range(ppc, &start, &end); + Bool ok = parse_Addr_pair(ppc, &start, &end); if (!ok) return False; if (start > end) @@ -5976,6 +6016,9 @@ KeepStacktraces MC_(clo_keep_stacktraces) = KS_alloc_and_free; Int MC_(clo_mc_level) = 2; Bool MC_(clo_show_mismatched_frees) = True; Bool MC_(clo_expensive_definedness_checks) = False; +Bool MC_(clo_ignore_range_below_sp) = False; +UInt MC_(clo_ignore_range_below_sp__first_offset) = 0; +UInt MC_(clo_ignore_range_below_sp__last_offset) = 0; static const HChar * MC_(parse_leak_heuristics_tokens) = "-,stdstring,length64,newarray,multipleinheritance"; @@ -6106,6 +6149,48 @@ static Bool mc_process_cmd_line_options(const HChar* arg) } } + else if VG_STR_CLO(arg, "--ignore-range-below-sp", tmp_str) { + /* This seems at first a bit weird, but: in order to imply + a non-wrapped-around address range, the first offset needs to be + larger than the second one. For example + --ignore-range-below-sp=8192,8189 + would cause accesses to in the range [SP-8192, SP-8189] to be + ignored. */ + UInt offs1 = 0, offs2 = 0; + Bool ok = parse_UInt_pair(&tmp_str, &offs1, &offs2); + // Ensure we used all the text after the '=' sign. + if (ok && *tmp_str != 0) ok = False; + if (!ok) { + VG_(message)(Vg_DebugMsg, + "ERROR: --ignore-range-below-sp: invalid syntax. " + " Expected \"...=decimalnumber-decimalnumber\".\n"); + return False; + } + if (offs1 > 1000*1000 /*arbitrary*/ || offs2 > 1000*1000 /*ditto*/) { + VG_(message)(Vg_DebugMsg, + "ERROR: --ignore-range-below-sp: suspiciously large " + "offset(s): %u and %u\n", offs1, offs2); + return False; + } + if (offs1 <= offs2) { + VG_(message)(Vg_DebugMsg, + "ERROR: --ignore-range-below-sp: invalid offsets " + "(the first must be larger): %u and %u\n", offs1, offs2); + return False; + } + tl_assert(offs1 > offs2); + if (offs1 - offs2 > 4096 /*arbitrary*/) { + VG_(message)(Vg_DebugMsg, + "ERROR: --ignore-range-below-sp: suspiciously large " + "range: %u-%u (size %u)\n", offs1, offs2, offs1 - offs2); + return False; + } + MC_(clo_ignore_range_below_sp) = True; + MC_(clo_ignore_range_below_sp__first_offset) = offs1; + MC_(clo_ignore_range_below_sp__last_offset) = offs2; + return True; + } + else if VG_BHEX_CLO(arg, "--malloc-fill", MC_(clo_malloc_fill), 0x00,0xFF) {} else if VG_BHEX_CLO(arg, "--free-fill", MC_(clo_free_fill), 0x00,0xFF) {} @@ -6163,8 +6248,11 @@ static void mc_print_usage(void) " Use extra-precise definedness tracking [no]\n" " --freelist-vol= volume of freed blocks queue [20000000]\n" " --freelist-big-blocks= releases first blocks with size>= [1000000]\n" -" --workaround-gcc296-bugs=no|yes self explanatory [no]\n" +" --workaround-gcc296-bugs=no|yes self explanatory [no]. Deprecated.\n" +" Use --ignore-range-below-sp instead.\n" " --ignore-ranges=0xPP-0xQQ[,0xRR-0xSS] assume given addresses are OK\n" +" --ignore-range-below-sp=- do not report errors for\n" +" accesses at the given offsets below SP\n" " --malloc-fill= fill malloc'd areas with given value\n" " --free-fill= fill free'd areas with given value\n" " --keep-stacktraces=alloc|free|alloc-and-free|alloc-then-free|none\n" @@ -7667,12 +7755,23 @@ static void mc_post_clo_init ( void ) MC_(clo_leak_check) = LC_Full; } - if (MC_(clo_freelist_big_blocks) >= MC_(clo_freelist_vol)) + if (MC_(clo_freelist_big_blocks) >= MC_(clo_freelist_vol) + && VG_(clo_verbosity) == 1 && !VG_(clo_xml)) { VG_(message)(Vg_UserMsg, "Warning: --freelist-big-blocks value %lld has no effect\n" "as it is >= to --freelist-vol value %lld\n", MC_(clo_freelist_big_blocks), MC_(clo_freelist_vol)); + } + + if (MC_(clo_workaround_gcc296_bugs) + && VG_(clo_verbosity) == 1 && !VG_(clo_xml)) { + VG_(umsg)( + "Warning: --workaround-gcc296-bugs=yes is deprecated.\n" + "Warning: Instead use: --ignore-range-below-sp=1024-1\n" + "\n" + ); + } tl_assert( MC_(clo_mc_level) >= 1 && MC_(clo_mc_level) <= 3 ); diff --git a/memcheck/tests/amd64-linux/Makefile.am b/memcheck/tests/amd64-linux/Makefile.am index 9b31b8001c..56a390be78 100644 --- a/memcheck/tests/amd64-linux/Makefile.am +++ b/memcheck/tests/amd64-linux/Makefile.am @@ -5,10 +5,15 @@ dist_noinst_SCRIPTS = \ filter_stderr filter_defcfaexpr EXTRA_DIST = \ + access_below_sp_1.vgtest \ + access_below_sp_1.stderr.exp access_below_sp_1.stdout.exp \ + access_below_sp_2.vgtest \ + access_below_sp_2.stderr.exp access_below_sp_2.stdout.exp \ defcfaexpr.vgtest defcfaexpr.stderr.exp \ int3-amd64.vgtest int3-amd64.stderr.exp int3-amd64.stdout.exp check_PROGRAMS = \ + access_below_sp \ defcfaexpr \ int3-amd64 diff --git a/memcheck/tests/amd64-linux/access_below_sp.c b/memcheck/tests/amd64-linux/access_below_sp.c new file mode 100644 index 0000000000..b6d0d29732 --- /dev/null +++ b/memcheck/tests/amd64-linux/access_below_sp.c @@ -0,0 +1,39 @@ + +#include + + +#define COMPILER_BARRIER __asm__ __volatile("":::"cc","memory") + +/* force the kernel to map in 15k below SP, so we can safely poke + around there. */ +__attribute__((noinline)) void make_below_sp_safe ( void ) +{ + const int N = 15000; + unsigned char a[N]; + int i; + + for (i = 0; i < N; i++) { + a[i] = i & 0xFF; + } + + COMPILER_BARRIER; + + unsigned int r = 0; + for (i = 0; i < N; i++) { + r = (r << 1) | (r >> 31); + r ^= (unsigned int)a[i]; + } + fprintf(stderr, "Checksum: %08x\n", r); +} + + +int main ( void ) +{ + make_below_sp_safe(); + + unsigned int res; + __asm__ __volatile__("movl -8192(%%rsp), %0" + : "=r"(res) : : "memory","cc"); + fprintf(stderr, "Got %08x\n", res); + return 0; +} diff --git a/memcheck/tests/amd64-linux/access_below_sp_1.stderr.exp b/memcheck/tests/amd64-linux/access_below_sp_1.stderr.exp new file mode 100644 index 0000000000..58d1fde2fa --- /dev/null +++ b/memcheck/tests/amd64-linux/access_below_sp_1.stderr.exp @@ -0,0 +1,2 @@ +Checksum: 7ff7077f +Got e3e2e1e0 diff --git a/memcheck/tests/amd64-linux/access_below_sp_1.stdout.exp b/memcheck/tests/amd64-linux/access_below_sp_1.stdout.exp new file mode 100644 index 0000000000..e69de29bb2 diff --git a/memcheck/tests/amd64-linux/access_below_sp_1.vgtest b/memcheck/tests/amd64-linux/access_below_sp_1.vgtest new file mode 100644 index 0000000000..3d5b37e427 --- /dev/null +++ b/memcheck/tests/amd64-linux/access_below_sp_1.vgtest @@ -0,0 +1,2 @@ +prog: access_below_sp +vgopts: -q --ignore-range-below-sp=8192-8189 diff --git a/memcheck/tests/amd64-linux/access_below_sp_2.stderr.exp b/memcheck/tests/amd64-linux/access_below_sp_2.stderr.exp new file mode 100644 index 0000000000..3ddfc3ecd0 --- /dev/null +++ b/memcheck/tests/amd64-linux/access_below_sp_2.stderr.exp @@ -0,0 +1,7 @@ +Checksum: 7ff7077f +Invalid read of size 4 + ... + Address 0x........ is on thread 1's stack + .... bytes below stack pointer + +Got e3e2e1e0 diff --git a/memcheck/tests/amd64-linux/access_below_sp_2.stdout.exp b/memcheck/tests/amd64-linux/access_below_sp_2.stdout.exp new file mode 100644 index 0000000000..e69de29bb2 diff --git a/memcheck/tests/amd64-linux/access_below_sp_2.vgtest b/memcheck/tests/amd64-linux/access_below_sp_2.vgtest new file mode 100644 index 0000000000..35ecd70dea --- /dev/null +++ b/memcheck/tests/amd64-linux/access_below_sp_2.vgtest @@ -0,0 +1,2 @@ +prog: access_below_sp +vgopts: -q