From: Philippe Waroquiers Date: Thu, 17 Oct 2013 22:10:41 +0000 (+0000) Subject: Allow tools to provide some statistics in suppression list produced at the end X-Git-Tag: svn/VALGRIND_3_9_0~40 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=619be966dbf6596a4ab283bcb8f55e5ada451ff2;p=thirdparty%2Fvalgrind.git Allow tools to provide some statistics in suppression list produced at the end Option -v outputs a list of used suppressions. This only gives the nr of times a suppression was used. For a leak search, this only gives the nr of loss records that have been suppressed, but it does not give additional needed details to understand more precisely what has been suppressed (i.e. nr of blocks and nr of bytes). => Add in the tool interface update_extra_suppression_use and print_extra_suppression_info functions to allow the tool to record additioonal use statistics for a suppression. These statistics can be done depending on the error (and its data) which is suppressed. Use this in memcheck for the leak suppressions, to maintain and output the nr of blocks and bytes suppressed by a suppression during the last leak search. git-svn-id: svn://svn.valgrind.org/valgrind/trunk@13651 --- diff --git a/NEWS b/NEWS index 1e435c1c55..dccbefe32d 100644 --- a/NEWS +++ b/NEWS @@ -32,6 +32,12 @@ Release 3.9.0 (?? ?????? 201?) for 'use after free' errors or to decrease Valgrind memory and/or cpu usage by recording less information for heap blocks. + - The list of used suppressions (shown when giving the -v option) + now shows for the leak suppressions how many blocks and bytes were + suppressed during the last leak search for each suppression. + The suppression count for a leak suppression shows the total nr + of loss records which were suppressed by this suppression. + * ==================== OTHER CHANGES ==================== - Option --merge-recursive-frames= tells Valgrind to @@ -63,6 +69,9 @@ Release 3.9.0 (?? ?????? 201?) 'v.do expensive_sanity_check_general' that checks the sanity of various Valgrind aspects, including the Valgrind heap. + - The list of used suppressions (shown when giving the -v option) + now gives the filename and linenr where the suppression is defined. + - remote debuginfo server + overhaul of debuginfo reading - some fixes for OSX 10.8 diff --git a/coregrind/m_errormgr.c b/coregrind/m_errormgr.c index 106bb8cd63..091fce5d6e 100644 --- a/coregrind/m_errormgr.c +++ b/coregrind/m_errormgr.c @@ -913,12 +913,19 @@ static Bool show_used_suppressions ( void ) " \n", su->count, su->sname ); } else { + HChar xtra[256]; /* assumed big enough (is overrun-safe) */ + Bool anyXtra; // blank line before the first shown suppression, if any if (!any_supp) VG_(dmsg)("\n"); - VG_(dmsg)("used_suppression: %6d %s %s:%d\n", su->count, su->sname, + VG_(memset)(xtra, 0, sizeof(xtra)); + anyXtra = VG_TDICT_CALL(tool_print_extra_suppression_use, + su, xtra, sizeof(xtra)); + vg_assert(xtra[sizeof(xtra)-1] == 0); + VG_(dmsg)("used_suppression: %6d %s %s:%d%s%s\n", su->count, su->sname, VG_(clo_suppressions)[su->clo_suppressions_i], - su->sname_lineno); + su->sname_lineno, + anyXtra ? " " : "", xtra); } any_supp = True; } @@ -1659,8 +1666,9 @@ static Supp* is_suppressible_error ( Error* err ) /* Conceptually, ip2fo contains an array of function names and an array of object names, corresponding to the array of IP of err->where. These names are just computed 'on demand' (so once maximum), - then stored (efficiently, avoiding too many allocs) in ip2fo to be re-usable - for the matching of the same IP with the next suppression pattern. + then stored (efficiently, avoiding too many allocs) in ip2fo to be + re-usable for the matching of the same IP with the next suppression + pattern. VG_(generic_match) gets this 'IP to Fun or Obj name completer' as one of its arguments. It will then pass it to the function @@ -1687,7 +1695,10 @@ static Supp* is_suppressible_error ( Error* err ) em_supplist_cmps++; if (supp_matches_error(su, err) && supp_matches_callers(&ip2fo, su)) { - /* got a match. Move this entry to the head of the list + /* got a match. */ + /* Inform the tool that err is suppressed by su. */ + (void)VG_TDICT_CALL(tool_update_extra_suppression_use, err, su); + /* Move this entry to the head of the list in the hope of making future searches cheaper. */ if (su_prev) { vg_assert(su_prev->next == su); diff --git a/coregrind/m_tooliface.c b/coregrind/m_tooliface.c index bcf825530d..24f0b3abe0 100644 --- a/coregrind/m_tooliface.c +++ b/coregrind/m_tooliface.c @@ -234,7 +234,9 @@ void VG_(needs_tool_errors)( Bool (*read_extra) (Int, HChar**, SizeT*, Int*, Supp*), Bool (*matches) (Error*, Supp*), const HChar* (*name) (Error*), - Bool (*get_xtra_si)(Error*,/*OUT*/HChar*,Int) + Bool (*get_xtra_si)(Error*,/*OUT*/HChar*,Int), + Bool (*print_xtra_su)(Supp*,/*OUT*/HChar*,Int), + void (*update_xtra_su)(Error*, Supp*) ) { VG_(needs).tool_errors = True; @@ -248,6 +250,8 @@ void VG_(needs_tool_errors)( VG_(tdict).tool_error_matches_suppression = matches; VG_(tdict).tool_get_error_name = name; VG_(tdict).tool_get_extra_suppression_info = get_xtra_si; + VG_(tdict).tool_print_extra_suppression_use = print_xtra_su; + VG_(tdict).tool_update_extra_suppression_use = update_xtra_su; } void VG_(needs_command_line_options)( diff --git a/coregrind/pub_core_tooliface.h b/coregrind/pub_core_tooliface.h index 0946bff3de..bc11ffd002 100644 --- a/coregrind/pub_core_tooliface.h +++ b/coregrind/pub_core_tooliface.h @@ -127,6 +127,8 @@ typedef struct { Bool (*tool_error_matches_suppression) (Error*, Supp*); const HChar* (*tool_get_error_name) (Error*); Bool (*tool_get_extra_suppression_info) (Error*,/*OUT*/HChar*,Int); + Bool (*tool_print_extra_suppression_use) (Supp*,/*OUT*/HChar*,Int); + void (*tool_update_extra_suppression_use) (Error*, Supp*); // VG_(needs).superblock_discards void (*tool_discard_superblock_info)(Addr64, VexGuestExtents); diff --git a/docs/xml/manual-core.xml b/docs/xml/manual-core.xml index 9e2708aeed..4de9a04fa0 100644 --- a/docs/xml/manual-core.xml +++ b/docs/xml/manual-core.xml @@ -384,14 +384,17 @@ suppression mechanism is designed to allow precise yet flexible specification of errors to suppress. If you use the option, at the end of execution, -Valgrind prints out one line for each used suppression, giving its name -and the number of times it got used. Here's the suppressions used by a -run of valgrind --tool=memcheck ls -l: +Valgrind prints out one line for each used suppression, giving the number of times +it got used, its name and the filename and line number where the suppression is +defined. Depending on the suppression kind, the filename and line number are optionally +followed by additional information (such as the number of blocks and bytes suppressed +by a memcheck leak suppression). Here's the suppressions used by a +run of valgrind -v --tool=memcheck ls -l: +--1610-- used_suppression: 2 dl-hack3-cond-1 /usr/lib/valgrind/default.supp:1234 +--1610-- used_suppression: 2 glibc-2.5.x-on-SUSE-10.2-(PPC)-2a /usr/lib/valgrind/default.supp:1234 +]]> Multiple suppressions files are allowed. By default, Valgrind uses $PREFIX/lib/valgrind/default.supp. You can diff --git a/drd/drd_error.c b/drd/drd_error.c index 8e81174122..12bfeff491 100644 --- a/drd/drd_error.c +++ b/drd/drd_error.c @@ -608,6 +608,19 @@ Bool drd_get_extra_suppression_info(Error* e, return False; } +static +Bool drd_print_extra_suppression_use(Supp* su, + /*OUT*/HChar* buf, Int nBuf) +{ + return False; +} + +static +void drd_update_extra_suppresion_use(Error* e, Supp* supp) +{ + return; +} + /** Tell the Valgrind core about DRD's error handlers. */ void DRD_(register_error_handlers)(void) { @@ -620,5 +633,7 @@ void DRD_(register_error_handlers)(void) drd_read_extra_suppression_info, drd_error_matches_suppression, drd_get_error_name, - drd_get_extra_suppression_info); + drd_get_extra_suppression_info, + drd_print_extra_suppression_use, + drd_update_extra_suppresion_use); } diff --git a/exp-sgcheck/pc_common.c b/exp-sgcheck/pc_common.c index f7b8006e1b..1ea8784152 100644 --- a/exp-sgcheck/pc_common.c +++ b/exp-sgcheck/pc_common.c @@ -793,6 +793,16 @@ Bool pc_get_extra_suppression_info ( Error* err, } } +Bool pc_print_extra_suppression_use ( Supp* su, + /*OUT*/HChar* buf, Int nBuf ) +{ + return False; +} + +void pc_update_extra_suppression_use (Error* err, Supp* su) +{ + return; +} /*--------------------------------------------------------------------*/ /*--- end pc_common.c ---*/ diff --git a/exp-sgcheck/pc_common.h b/exp-sgcheck/pc_common.h index 418f8b48a6..3a0fb5ac17 100644 --- a/exp-sgcheck/pc_common.h +++ b/exp-sgcheck/pc_common.h @@ -58,6 +58,9 @@ Bool pc_error_matches_suppression (Error* err, Supp* su); const HChar* pc_get_error_name ( Error* err ); Bool pc_get_extra_suppression_info ( Error* err, /*OUT*/HChar* buf, Int nBuf ); +Bool pc_print_extra_suppression_use ( Supp* su, + /*OUT*/HChar* buf, Int nBuf ); +void pc_update_extra_suppression_use (Error* err, Supp* su); extern Bool h_clo_partial_loads_ok; /* extern Bool h_clo_lossage_check; */ diff --git a/exp-sgcheck/pc_main.c b/exp-sgcheck/pc_main.c index 5cfd94cf80..84c1fce78a 100644 --- a/exp-sgcheck/pc_main.c +++ b/exp-sgcheck/pc_main.c @@ -105,7 +105,9 @@ static void pc_pre_clo_init(void) pc_read_extra_suppression_info, pc_error_matches_suppression, pc_get_error_name, - pc_get_extra_suppression_info); + pc_get_extra_suppression_info, + pc_print_extra_suppression_use, + pc_update_extra_suppression_use); VG_(needs_xml_output) (); diff --git a/helgrind/hg_errors.c b/helgrind/hg_errors.c index ffa31025cb..45ba4c81ed 100644 --- a/helgrind/hg_errors.c +++ b/helgrind/hg_errors.c @@ -1396,6 +1396,19 @@ Bool HG_(get_extra_suppression_info) ( Error* err, return False; } +Bool HG_(print_extra_suppression_use) ( Supp* su, + /*OUT*/HChar* buf, Int nBuf ) +{ + /* Do nothing */ + return False; +} + +void HG_(update_extra_suppression_use) ( Error* err, Supp* su ) +{ + /* Do nothing */ + return; +} + /*--------------------------------------------------------------------*/ /*--- end hg_errors.c ---*/ diff --git a/helgrind/hg_errors.h b/helgrind/hg_errors.h index 03d88c19fc..a73fa151aa 100644 --- a/helgrind/hg_errors.h +++ b/helgrind/hg_errors.h @@ -46,6 +46,9 @@ Bool HG_(error_matches_suppression) ( Error* err, Supp* su ); const HChar* HG_(get_error_name) ( Error* err ); Bool HG_(get_extra_suppression_info) ( Error* err, /*OUT*/HChar* buf, Int nBuf ); +Bool HG_(print_extra_suppression_use) ( Supp* su, + /*OUT*/HChar* buf, Int nBuf ); +void HG_(update_extra_suppression_use) ( Error* err, Supp* su ); /* Functions for recording various kinds of errors. */ void HG_(record_error_Race) ( Thread* thr, diff --git a/helgrind/hg_main.c b/helgrind/hg_main.c index 3d241bf9fa..40fbbe3fd8 100644 --- a/helgrind/hg_main.c +++ b/helgrind/hg_main.c @@ -5286,7 +5286,9 @@ static void hg_pre_clo_init ( void ) HG_(read_extra_suppression_info), HG_(error_matches_suppression), HG_(get_error_name), - HG_(get_extra_suppression_info)); + HG_(get_extra_suppression_info), + HG_(print_extra_suppression_use), + HG_(update_extra_suppression_use)); VG_(needs_xml_output) (); diff --git a/include/pub_tool_tooliface.h b/include/pub_tool_tooliface.h index 3d43be8a54..3d6fc9880e 100644 --- a/include/pub_tool_tooliface.h +++ b/include/pub_tool_tooliface.h @@ -343,7 +343,20 @@ extern void VG_(needs_tool_errors) ( // do nothing, and return False. This function is the inverse of // VG_(tdict).tool_read_extra_suppression_info(). Bool (*print_extra_suppression_info)(Error* err, - /*OUT*/HChar* buf, Int nBuf) + /*OUT*/HChar* buf, Int nBuf), + + // This is similar to print_extra_suppression_info, but is used + // to print information such as additional statistical counters + // as part of the used suppression list produced by -v. + Bool (*print_extra_suppression_use)(Supp* su, + /*OUT*/HChar* buf, Int nBuf), + + // Called by error mgr once it has been established that err + // is suppressed by su. update_extra_suppression_use typically + // can be used to update suppression extra information such as + // some statistical counters that will be printed by + // print_extra_suppression_use. + void (*update_extra_suppression_use)(Error* err, Supp* su) ); /* Is information kept by the tool about specific instructions or diff --git a/memcheck/docs/mc-manual.xml b/memcheck/docs/mc-manual.xml index 4c6f862e8b..304407e4f2 100644 --- a/memcheck/docs/mc-manual.xml +++ b/memcheck/docs/mc-manual.xml @@ -1166,11 +1166,28 @@ leak kinds are matched by this suppression entry. <set> is specified similarly to the option . If this optional extra line is not present, the suppression entry will match -all leak kinds. - +all leak kinds. The other memcheck error kinds do not have extra lines. + +If you give the option, Valgrind will print +the list of used suppressions at the end of the execution. +For a leak suppression, this output gives the number of different +loss records that matches the suppression, the number of bytes +and blocks suppressed by the suppressions. +In case several leak searches are done, the number of bytes and blocks +are reset to 0 before a new leak search. Note that the number of different +loss records is not reset to 0. +In the example below, in the last leak search, 7 blocks and 96 bytes have +been suppressed by the +suppression. + + + The first line of the calling context: for ValueN and AddrN errors, it is either the name of the function in which the error occurred, or, failing that, the full path of the diff --git a/memcheck/mc_errors.c b/memcheck/mc_errors.c index b9aa00a39e..b4b82a83f4 100644 --- a/memcheck/mc_errors.c +++ b/memcheck/mc_errors.c @@ -1532,6 +1532,14 @@ typedef struct _MC_LeakSuppExtra MC_LeakSuppExtra; struct _MC_LeakSuppExtra { UInt match_leak_kinds; + + /* Maintains nr of blocks and bytes suppressed with this suppression + during the leak search identified by leak_search_gen. + blocks_suppressed and bytes_suppressed are reset to 0 when + used the first time during a leak search. */ + SizeT blocks_suppressed; + SizeT bytes_suppressed; + UInt leak_search_gen; }; Bool MC_(read_extra_suppression_info) ( Int fd, HChar** bufpp, @@ -1549,6 +1557,9 @@ Bool MC_(read_extra_suppression_info) ( Int fd, HChar** bufpp, MC_LeakSuppExtra* lse; lse = VG_(malloc)("mc.resi.2", sizeof(MC_LeakSuppExtra)); lse->match_leak_kinds = RallS; + lse->blocks_suppressed = 0; + lse->bytes_suppressed = 0; + lse->leak_search_gen = 0; VG_(set_supp_extra)(su, lse); // By default, all kinds will match. eof = VG_(get_line) ( fd, bufpp, nBufp, lineno ); if (eof) return True; // old LeakSupp style, no match-leak-kinds line. @@ -1617,6 +1628,13 @@ Bool MC_(error_matches_suppression) ( Error* err, Supp* su ) case LeakSupp: if (ekind == Err_Leak) { MC_LeakSuppExtra* lse = (MC_LeakSuppExtra*) VG_(get_supp_extra)(su); + if (lse->leak_search_gen != MC_(leak_search_gen)) { + // First time we see this suppression during this leak search. + // => reset the counters to 0. + lse->blocks_suppressed = 0; + lse->bytes_suppressed = 0; + lse->leak_search_gen = MC_(leak_search_gen); + } return RiS(extra->Err.Leak.lr->key.state, lse->match_leak_kinds); } else return False; @@ -1695,6 +1713,37 @@ Bool MC_(get_extra_suppression_info) ( Error* err, } } +Bool MC_(print_extra_suppression_use) ( Supp *su, + /*OUT*/HChar *buf, Int nBuf ) +{ + if (VG_(get_supp_kind)(su) == LeakSupp) { + MC_LeakSuppExtra *lse = (MC_LeakSuppExtra*) VG_(get_supp_extra) (su); + + if (lse->leak_search_gen == MC_(leak_search_gen) + && lse->blocks_suppressed > 0) { + VG_(snprintf) (buf, nBuf-1, + "suppressed: %'lu bytes in %'lu blocks", + lse->bytes_suppressed, + lse->blocks_suppressed); + return True; + } else + return False; + } else + return False; +} + +void MC_(update_extra_suppression_use) ( Error* err, Supp* su) +{ + if (VG_(get_supp_kind)(su) == LeakSupp) { + MC_LeakSuppExtra *lse = (MC_LeakSuppExtra*) VG_(get_supp_extra) (su); + MC_Error* extra = VG_(get_error_extra)(err); + + tl_assert (lse->leak_search_gen = MC_(leak_search_gen)); + lse->blocks_suppressed += extra->Err.Leak.lr->num_blocks; + lse->bytes_suppressed + += extra->Err.Leak.lr->szB + extra->Err.Leak.lr->indirect_szB; + } +} /*--------------------------------------------------------------------*/ /*--- end mc_errors.c ---*/ diff --git a/memcheck/mc_include.h b/memcheck/mc_include.h index 46e824d148..a1c9a3d54b 100644 --- a/memcheck/mc_include.h +++ b/memcheck/mc_include.h @@ -351,6 +351,10 @@ typedef void MC_(detect_memory_leaks) ( ThreadId tid, LeakCheckParams * lcp); +// Each time a leak search is done, the leak search generation +// MC_(leak_search_gen) is incremented. +extern UInt MC_(leak_search_gen); + // maintains the lcp.deltamode given in the last call to detect_memory_leaks extern LeakCheckDeltaMode MC_(detect_memory_leaks_last_delta_mode); @@ -404,6 +408,9 @@ Bool MC_(error_matches_suppression) ( Error* err, Supp* su ); Bool MC_(get_extra_suppression_info) ( Error* err, /*OUT*/HChar* buf, Int nBuf ); +Bool MC_(print_extra_suppression_use) ( Supp* su, + /*OUT*/HChar* buf, Int nBuf ); +void MC_(update_extra_suppression_use) ( Error* err, Supp* su ); const HChar* MC_(get_error_name) ( Error* err ); diff --git a/memcheck/mc_leakcheck.c b/memcheck/mc_leakcheck.c index af0bfcf169..8bfd02ae9d 100644 --- a/memcheck/mc_leakcheck.c +++ b/memcheck/mc_leakcheck.c @@ -474,6 +474,10 @@ static UInt detect_memory_leaks_last_heuristics; // The below avoids replicating the delta_mode in each LossRecord. LeakCheckDeltaMode MC_(detect_memory_leaks_last_delta_mode); +// Each leak search run increments the below generation counter. +// A used suppression during a leak search will contain this +// generation number. +UInt MC_(leak_search_gen); // Records chunks that are currently being processed. Each element in the // stack is an index into lc_chunks and lc_extras. Its size is @@ -1646,7 +1650,7 @@ void MC_(detect_memory_leaks) ( ThreadId tid, LeakCheckParams* lcp) // before checking for (smaller) page skipping. tl_assert((SM_SIZE % VKI_PAGE_SIZE) == 0); - + MC_(leak_search_gen)++; MC_(detect_memory_leaks_last_delta_mode) = lcp->deltamode; detect_memory_leaks_last_heuristics = lcp->heuristics; diff --git a/memcheck/mc_main.c b/memcheck/mc_main.c index c3891b8270..f6e1e01020 100644 --- a/memcheck/mc_main.c +++ b/memcheck/mc_main.c @@ -6699,7 +6699,9 @@ static void mc_pre_clo_init(void) MC_(read_extra_suppression_info), MC_(error_matches_suppression), MC_(get_error_name), - MC_(get_extra_suppression_info)); + MC_(get_extra_suppression_info), + MC_(print_extra_suppression_use), + MC_(update_extra_suppression_use)); VG_(needs_libc_freeres) (); VG_(needs_command_line_options)(mc_process_cmd_line_options, mc_print_usage,