|
From: <sv...@va...> - 2013-09-29 13:47:45
|
Author: philippe
Date: Sun Sep 29 13:47:32 2013
New Revision: 13582
Log:
add heuristics decreasing false possible "possible leaks" in c++ code.
The option --leak-check-heuristics=heur1,heur2,... can activate
various heuristics to decrease the number of false positive
"possible leaks" for C++ code. The available heuristics are
detecting valid interior pointers to std::stdstring, to new[] allocated
arrays with elements having destructors and to interior pointers pointing
to an inner part of a C++ object using multiple inheritance.
This fixes 280271 Valgrind reports possible memory leaks on still-reachable
std::string
This has been tested on x86/amd64/ppc32/ppc64.
First performance measurements seems to show a neglectible impact on
the leak search.
More feedback welcome both on performance and functional aspects
(false positive 'possibly leaked' rate decrease and/or
false negative 'possibly leaked' rate increase).
Note that the heuristic is not checking that the memory has been
allocated with "new" or "new[]", as it is expected that in some cases,
specific alloc fn are used for c++ objects instead of the standard new/new[].
If needed, we might add an option to check the alloc functions
to be new/new[].
Added:
trunk/memcheck/tests/leak_cpp_interior.cpp
trunk/memcheck/tests/leak_cpp_interior.stderr.exp
trunk/memcheck/tests/leak_cpp_interior.stderr.exp-64bit
trunk/memcheck/tests/leak_cpp_interior.vgtest
Modified:
trunk/NEWS
trunk/gdbserver_tests/mchelp.stdoutB.exp
trunk/memcheck/docs/mc-manual.xml
trunk/memcheck/mc_include.h
trunk/memcheck/mc_leakcheck.c
trunk/memcheck/mc_main.c
trunk/memcheck/tests/Makefile.am
Modified: trunk/NEWS
==============================================================================
--- trunk/NEWS (original)
+++ trunk/NEWS Sun Sep 29 13:47:32 2013
@@ -20,6 +20,13 @@
This is a.o. useful to avoid definite leaks being "catched"
by a suppression entry aimed at suppressing possibly lost blocks.
+ - The option --leak-check-heuristics=heur1,heur2,... can activate
+ various heuristics to decrease the number of false positive
+ "possible leaks" for C++ code. The available heuristics are
+ detecting valid interior pointers to std::stdstring, to new[] allocated
+ arrays with elements having destructors and to interior pointers pointing
+ to an inner part of a C++ object using multiple inheritance.
+
- The option --keep-stacktraces controls which stack trace(s) to keep for
malloc'd and/or free'd blocks. This can be used to obtain more information
for 'use after free' errors or to decrease Valgrind memory and/or cpu usage
@@ -93,6 +100,7 @@
274695 [390] s390x: Support "compare to/from logical" instructions (z196)
275800 [390] s390x: Add support for the ecag instruction (part 1)
275800 [390] s390x: Autodetect cache info (part 2)
+280271 Valgrind reports possible memory leaks on still-reachable std::string
284540 [390] Memcheck shouldn't count suppressions matching still-reachable allocations
296311 [390] Wrong stack traces due to -fomit-frame-pointer (x86)
Modified: trunk/gdbserver_tests/mchelp.stdoutB.exp
==============================================================================
--- trunk/gdbserver_tests/mchelp.stdoutB.exp (original)
+++ trunk/gdbserver_tests/mchelp.stdoutB.exp Sun Sep 29 13:47:32 2013
@@ -25,10 +25,12 @@
and outputs a description of <addr>
leak_check [full*|summary]
[kinds kind1,kind2,...|reachable|possibleleak*|definiteleak]
+ [heuristics heur1,heur2,...]
[increased*|changed|any]
[unlimited*|limited <max_loss_records_output>]
* = defaults
where kind is one of definite indirect possible reachable all none
+ where heur is one of stdstring newarray multipleinheritance all none
Examples: leak_check
leak_check summary any
leak_check full kinds indirect,possible
@@ -78,10 +80,12 @@
and outputs a description of <addr>
leak_check [full*|summary]
[kinds kind1,kind2,...|reachable|possibleleak*|definiteleak]
+ [heuristics heur1,heur2,...]
[increased*|changed|any]
[unlimited*|limited <max_loss_records_output>]
* = defaults
where kind is one of definite indirect possible reachable all none
+ where heur is one of stdstring newarray multipleinheritance all none
Examples: leak_check
leak_check summary any
leak_check full kinds indirect,possible
Modified: trunk/memcheck/docs/mc-manual.xml
==============================================================================
--- trunk/memcheck/docs/mc-manual.xml (original)
+++ trunk/memcheck/docs/mc-manual.xml Sun Sep 29 13:47:32 2013
@@ -396,7 +396,7 @@
<para>There are two ways a block can be reached. The first is with a
"start-pointer", i.e. a pointer to the start of the block. The second is with
an "interior-pointer", i.e. a pointer to the middle of the block. There are
-three ways we know of that an interior-pointer can occur:</para>
+several ways we know of that an interior-pointer can occur:</para>
<itemizedlist>
<listitem>
@@ -422,8 +422,33 @@
See <ulink url="http://theory.uwinnipeg.ca/gnu/gcc/gxxint_14.html">this
page</ulink> for more information.</para>
</listitem>
+
+ <listitem>
+ <para>It might be a pointer to the inner char array of a C++
+ <computeroutput>std::string</computeroutput>. For example, some
+ compilers add 3 words at the beginning of the std::string to
+ store the length, the capacity and a reference count before the
+ memory containing the array of characters. They return a pointer
+ just after these 3 words, pointing at the char array.</para>
+ </listitem>
+
+ <listitem>
+ <para>It might be a pointer to an inner part of a C++ object using
+ multiple inheritance. </para>
+ </listitem>
</itemizedlist>
+<para>You can optionally activate heuristics to use during the leak
+search to detect the interior pointers corresponding to
+the <computeroutput>newarray</computeroutput>,
+<computeroutput>stdstring</computeroutput> and
+<computeroutput>multipleinheritance</computeroutput> cases. If the
+heuristic detects that an interior pointer corresponds to such a case,
+the block will be considered as reachable by the interior
+pointer. In other words, the interior pointer will be treated
+as if it were a start pointer.</para>
+
+
<para>With that in mind, consider the nine possible cases described by the
following figure.</para>
@@ -524,6 +549,26 @@
suppressed: 0 bytes in 0 blocks.
]]></programlisting>
+<para>If heuristics have been used to consider some blocks as
+reachable, the leak summary details the heuristically reachable subset
+of 'still reachable:' per heuristic. In the below example, of the 79
+bytes still reachable, 71 bytes (56+7+8) have been considered
+heuristically reachable.
+</para>
+
+<programlisting><![CDATA[
+LEAK SUMMARY:
+ definitely lost: 4 bytes in 1 blocks
+ indirectly lost: 0 bytes in 0 blocks
+ possibly lost: 0 bytes in 0 blocks
+ still reachable: 79 bytes in 5 blocks
+ of which reachable via heuristic:
+ stdstring : 56 bytes in 2 blocks
+ newarray : 7 bytes in 1 blocks
+ multipleinheritance: 8 bytes in 1 blocks
+ suppressed: 0 bytes in 0 blocks
+]]></programlisting>
+
<para>If <option>--leak-check=full</option> is specified,
Memcheck will give details for each definitely lost or possibly lost block,
including where it was allocated. (Actually, it merges results for all
@@ -705,6 +750,40 @@
</varlistentry>
+ <varlistentry id="opt.leak-check-heuristics" xreflabel="--leak-check-heuristics">
+ <term>
+ <option><![CDATA[--leak-check-heuristics=<set> [default: none] ]]></option>
+ </term>
+ <listitem>
+ <para>Specifies the leak check heuristics to use during leak search
+ to discover interior pointers with which a block should be considered
+ as reachable. The heuristic set is specified by using one of the
+ following forms:
+
+ <itemizedlist>
+ <listitem>a comma separated list of one or more of
+ <option>stdstring newarray multipleinheritance</option>.
+ </listitem>
+
+ <listitem><option>all</option> to activate the complete set of
+ heuristics.
+ It is equivalent to
+ <option>--leak-check-heuristics=stdstring,newarray,multipleinheritance</option>.
+ </listitem>
+
+ <listitem><option>none</option> is the empty set.
+ </listitem>
+ </itemizedlist>
+ </para>
+
+ <para>Note that these heuristics are dependent on the layout of the objects
+ produced by the C++ compiler. They have been tested with some gcc versions
+ (e.g. 4.4 and 4.7). They might not work properly with other C++ compilers.
+ </para>
+ </listitem>
+ </varlistentry>
+
+
<varlistentry id="opt.show-reachable" xreflabel="--show-reachable">
<term>
<option><![CDATA[--show-reachable=<yes|no> ]]></option>
@@ -1755,8 +1834,33 @@
==20852== tid 1 register EBX interior points at 2 bytes inside 0x40281a8
(gdb)
]]></programlisting>
- </listitem>
+ <para> When <varname>who_points_at</varname> finds an interior pointer,
+ it will report the heuristic(s) with which this interior pointer
+ will be considered as reachable. Note that this is done independently
+ of the value of the option <option>--leak-check-heuristics</option>.
+ In the below example, the loss record 6 indicates a possibly lost
+ block. <varname>who_points_at</varname> reports that there is an interior
+ pointer pointing in this block, and that the block can be considered
+ reachable using the heuristic
+ <computeroutput>multipleinheritance</computeroutput>.
+ </para>
+
+<programlisting><![CDATA[
+(gdb) monitor block_list 6
+==3748== 8 bytes in 1 blocks are possibly lost in loss record 6 of 7
+==3748== at 0x4007D77: operator new(unsigned int) (vg_replace_malloc.c:313)
+==3748== by 0x8048954: main (leak_cpp_interior.cpp:43)
+==3748== 0x402A0E0[8]
+(gdb) monitor who_points_at 0x402A0E0 8
+==3748== Searching for pointers pointing in 8 bytes from 0x402a0e0
+==3748== *0xbe8ee078 interior points at 4 bytes inside 0x402a0e0
+==3748== Address 0xbe8ee078 is on thread 1's stack
+==3748== block at 0x402a0e0 considered reachable by ptr 0x402a0e4 using multipleinheritance heuristic
+(gdb)
+]]></programlisting>
+
+ </listitem>
</itemizedlist>
Modified: trunk/memcheck/mc_include.h
==============================================================================
--- trunk/memcheck/mc_include.h (original)
+++ trunk/memcheck/mc_include.h Sun Sep 29 13:47:32 2013
@@ -342,8 +342,9 @@
LeakCheckMode mode;
UInt show_leak_kinds;
UInt errors_for_leak_kinds;
+ UInt heuristics;
LeakCheckDeltaMode deltamode;
- UInt max_loss_records_output; // limit on the nr of loss records output.
+ UInt max_loss_records_output; // limit on the nr of loss records output.
Bool requested_by_monitor_command; // True when requested by gdb/vgdb.
}
LeakCheckParams;
@@ -354,8 +355,8 @@
extern LeakCheckDeltaMode MC_(detect_memory_leaks_last_delta_mode);
// prints the list of blocks corresponding to the given loss_record_nr.
-// Returns True if loss_record_nr identifies a correct loss record from last leak search.
-// Returns False otherwise.
+// Returns True if loss_record_nr identifies a correct loss record from last
+// leak search, returns False otherwise.
Bool MC_(print_block_list) ( UInt loss_record_nr);
// Prints the addresses/registers/... at which a pointer to
@@ -494,6 +495,39 @@
Default : R2S(Possible) | R2S(Unreached). */
extern UInt MC_(clo_errors_for_leak_kinds);
+/* Various leak check heuristics which can be activated/deactivated. */
+typedef
+ enum {
+ LchNone =0,
+ // no heuristic.
+ LchStdString =1,
+ // Consider interior pointer pointing at the array of char in a
+ // std::string as reachable.
+ LchNewArray =2,
+ // Consider interior pointer pointing at second word of a new[] array as
+ // reachable. Such interior pointers are used for arrays whose elements
+ // have a destructor.
+ LchMultipleInheritance =3,
+ // Conside interior pointer pointing just after what looks a vtable
+ // as reachable.
+ }
+ LeakCheckHeuristic;
+
+// Nr of heuristics.
+#define N_LEAK_CHECK_HEURISTICS 4
+
+// Build mask to check or set Heuristic h membership
+#define H2S(h) (1 << (h))
+// CppHeuristic h is member of the Set s ?
+#define HiS(h,s) ((s) & R2S(h))
+// A set with all Heuristics:
+#define HallS \
+ (H2S(LchStdString) | H2S(LchNewArray) | H2S(LchMultipleInheritance))
+
+/* Heuristics set to use for the leak search.
+ Default : no heuristic. */
+extern UInt MC_(clo_leak_check_heuristics);
+
/* Assume accesses immediately below %esp are due to gcc-2.96 bugs.
* default: NO */
extern Bool MC_(clo_workaround_gcc296_bugs);
Modified: trunk/memcheck/mc_leakcheck.c
==============================================================================
--- trunk/memcheck/mc_leakcheck.c (original)
+++ trunk/memcheck/mc_leakcheck.c Sun Sep 29 13:47:32 2013
@@ -427,12 +427,17 @@
typedef
struct {
UInt state:2; // Reachedness.
- UInt pending:1; // Scan pending.
+ UInt pending:1; // Scan pending.
+ UInt heuristic: (sizeof(UInt)*8)-3;
+ // Heuristic with which this block was considered reachable.
+ // LchNone if state != Reachable or no heuristic needed to
+ // consider it reachable.
+
union {
- SizeT indirect_szB : (sizeof(SizeT)*8)-3; // If Unreached, how many bytes
- // are unreachable from here.
- SizeT clique : (sizeof(SizeT)*8)-3; // if IndirectLeak, clique leader
- // to which it belongs.
+ SizeT indirect_szB;
+ // If Unreached, how many bytes are unreachable from here.
+ SizeT clique;
+ // if IndirectLeak, clique leader to which it belongs.
} IorC;
}
LC_Extra;
@@ -461,9 +466,11 @@
// Array of sorted loss record (produced during last leak search).
static LossRecord** lr_array;
+// Value of the heuristics parameter used in the current (or last) leak check.
+static UInt detect_memory_leaks_last_heuristics;
// DeltaMode used the last time we called detect_memory_leaks.
-// The recorded leak errors must be output using a logic based on this delta_mode.
+// The recorded leak errors are output using a logic based on this delta_mode.
// The below avoids replicating the delta_mode in each LossRecord.
LeakCheckDeltaMode MC_(detect_memory_leaks_last_delta_mode);
@@ -495,6 +502,13 @@
SizeT MC_(blocks_reachable) = 0;
SizeT MC_(blocks_suppressed) = 0;
+// Subset of MC_(bytes_reachable) and MC_(blocks_reachable) which
+// are considered reachable due to the corresponding heuristic.
+static SizeT MC_(bytes_heuristically_reachable)[N_LEAK_CHECK_HEURISTICS]
+ = {0,0,0,0};
+static SizeT MC_(blocks_heuristically_reachable)[N_LEAK_CHECK_HEURISTICS]
+ = {0,0,0,0};
+
// Determines if a pointer is to a chunk. Returns the chunk number et al
// via call-by-reference.
static Bool
@@ -568,6 +582,181 @@
}
}
+static const HChar* pp_heuristic(LeakCheckHeuristic h)
+{
+ switch(h) {
+ case LchNone: return "none";
+ case LchStdString: return "stdstring";
+ case LchNewArray: return "newarray";
+ case LchMultipleInheritance: return "multipleinheritance";
+ default: return "???invalid heuristic???";
+ }
+}
+
+// True if ptr looks like the address of a vtable, i.e. if ptr
+// points to an array of pointers to functions.
+// It is assumed the only caller of this function is heuristic_reachedness
+// which must check that ptr is aligned and above page 0.
+// Checking that ptr is above page 0 is an optimisation : it is assumed
+// that no vtable is located in the page 0. So, all small integer values
+// encountered during the scan will not incur the cost of calling this
+// function.
+static Bool aligned_ptr_above_page0_is_vtable_addr(Addr ptr)
+{
+ // ??? If performance problem:
+ // ??? maybe implement a cache (array indexed by ptr % primenr)
+ // ??? of "I am a vtable ptr" ???
+
+ // ??? Maybe the debug info could (efficiently?) be used to detect vtables ?
+
+ // We consider ptr as a vtable ptr if it points to a table
+ // where we find only NULL pointers or pointers pointing at an
+ // executable region. We must find at least 2 non NULL pointers
+ // before considering ptr as a vtable pointer.
+ // We scan a maximum of VTABLE_MAX_CHECK words for these 2 non NULL
+ // pointers.
+#define VTABLE_MAX_CHECK 20
+
+ NSegment const *seg;
+ UInt nr_fn_ptrs = 0;
+ Addr scan;
+ Addr scan_max;
+
+ // First verify ptr points inside a client mapped file section.
+ // ??? is a vtable always in a file mapped readable section ?
+ seg = VG_(am_find_nsegment) (ptr);
+ if (seg == NULL
+ || seg->kind != SkFileC
+ || !seg->hasR)
+ return False;
+
+ // Check potential function pointers, up to a maximum of VTABLE_MAX_CHECK.
+ scan_max = ptr + VTABLE_MAX_CHECK*sizeof(Addr);
+ // If ptr is near the end of seg, avoid scan_max exceeding the end of seg:
+ if (scan_max > seg->end - sizeof(Addr))
+ scan_max = seg->end - sizeof(Addr);
+ for (scan = ptr; scan <= scan_max; scan+=sizeof(Addr)) {
+ Addr pot_fn = *((Addr *)scan);
+ if (pot_fn == 0)
+ continue; // NULL fn pointer. Seems it can happen in vtable.
+ seg = VG_(am_find_nsegment) (pot_fn);
+#if defined(VGA_ppc64)
+ // ppc64 use a thunk table. So, we have one more level of indirection
+ // to follow.
+ if (seg == NULL
+ || seg->kind != SkFileC
+ || !seg->hasR
+ || !seg->hasW)
+ return False; // ptr to nowhere, or not a ptr to thunks.
+ pot_fn = *((Addr *)pot_fn);
+ if (pot_fn == 0)
+ continue; // NULL fn pointer. Seems it can happen in vtable.
+ seg = VG_(am_find_nsegment) (pot_fn);
+#endif
+ if (seg == NULL
+ || seg->kind != SkFileC
+ || !seg->hasT)
+ return False; // ptr to nowhere, or not a fn ptr.
+ nr_fn_ptrs++;
+ if (nr_fn_ptrs == 2)
+ return True;
+ }
+
+ return False;
+}
+
+// If ch is heuristically reachable via an heuristic member of heur_set,
+// returns this heuristic.
+// If ch cannot be considered reachable using one of these heuristics,
+// return LchNone.
+// This should only be called when ptr is an interior ptr to ch.
+// The StdString/NewArray/MultipleInheritance heuristics are directly
+// inspired from DrMemory:
+// see http://www.burningcutlery.com/derek/docs/drmem-CGO11.pdf [section VI,C]
+// and bug 280271.
+static LeakCheckHeuristic heuristic_reachedness (Addr ptr,
+ MC_Chunk *ch, LC_Extra *ex,
+ UInt heur_set)
+{
+ if (HiS(LchStdString, heur_set)) {
+ // Detects inner pointers to Std::String for layout being
+ // length capacity refcount char_array[] \0
+ // where ptr points to the beginning of the char_array.
+ if ( ptr == ch->data + 3 * sizeof(SizeT)) {
+ const SizeT length = *((SizeT*)ch->data);
+ const SizeT capacity = *((SizeT*)ch->data+1);
+ if (length <= capacity
+ && (3 * sizeof(SizeT) + capacity + 1 == ch->szB)) {
+ // ??? could check there is no null byte from ptr to ptr+length-1
+ // ??? and that there is a null byte at ptr+length.
+ // ???
+ // ??? could check that ch->allockind is MC_AllocNew ???
+ // ??? probably not a good idea, as I guess stdstring
+ // ??? allocator can be done via custom allocator
+ // ??? or even a call to malloc ????
+ return LchStdString;
+ }
+ }
+ }
+
+ if (HiS(LchNewArray, heur_set)) {
+ // Detects inner pointers at second word of new[] array, following
+ // a plausible nr of elements.
+ // Such inner pointers are used for arrays of elements
+ // having a destructor, as the delete[] of the array must know
+ // how many elements to destroy.
+ //
+ // We have a strange/wrong case for 'ptr = new MyClass[0];' :
+ // For such a case, the returned ptr points just outside the
+ // allocated chunk. This chunk is then seen as a definite
+ // leak by Valgrind, as it is not considered an interior pointer.
+ // It is the c++ equivalent of bug 99923 (malloc(0) wrongly considered
+ // as definitely leaked). See the trick in find_chunk_for handling
+ // 0-sized block. This trick does not work for 'new MyClass[0]'
+ // because a chunk "word-sized" is allocated to store the (0) nr
+ // of elements.
+ if ( ptr == ch->data + sizeof(SizeT)) {
+ const SizeT nr_elts = *((SizeT*)ch->data);
+ if (nr_elts > 0 && (ch->szB - sizeof(SizeT)) % nr_elts == 0) {
+ // ??? could check that ch->allockind is MC_AllocNewVec ???
+ return LchNewArray;
+ }
+ }
+ }
+
+ if (HiS(LchMultipleInheritance, heur_set)) {
+ // Detect inner pointer used for multiple inheritance.
+ // Assumption is that the vtable pointers are before the object.
+ if (VG_IS_WORD_ALIGNED(ptr)) {
+ Addr first_addr;
+ Addr inner_addr;
+
+ // Avoid the call to is_vtable_addr when the addr is not
+ // aligned or points in the page0, as it is unlikely
+ // a vtable is located in this page. This last optimisation
+ // avoids to call aligned_ptr_above_page0_is_vtable_addr
+ // for all small integers.
+ // Note: we could possibly also avoid calling this function
+ // for small negative integers, as no vtable should be located
+ // in the last page.
+ inner_addr = *((Addr*)ptr);
+ if (VG_IS_WORD_ALIGNED(inner_addr)
+ && inner_addr >= (Addr)VKI_PAGE_SIZE) {
+ first_addr = *((Addr*)ch->data);
+ if (VG_IS_WORD_ALIGNED(first_addr)
+ && first_addr >= (Addr)VKI_PAGE_SIZE
+ && aligned_ptr_above_page0_is_vtable_addr(inner_addr)
+ && aligned_ptr_above_page0_is_vtable_addr(first_addr)) {
+ // ??? could check that ch->allockind is MC_AllocNew ???
+ return LchMultipleInheritance;
+ }
+ }
+ }
+ }
+
+ return LchNone;
+}
+
// If 'ptr' is pointing to a heap-allocated block which hasn't been seen
// before, push it onto the mark stack.
@@ -577,16 +766,45 @@
Int ch_no;
MC_Chunk* ch;
LC_Extra* ex;
+ Reachedness ch_via_ptr; // Is ch reachable via ptr, and how ?
if ( ! lc_is_a_chunk_ptr(ptr, &ch_no, &ch, &ex) )
return;
+
+ if (ex->state == Reachable) {
+ // If block was considered reachable via an heuristic,
+ // and it is now directly reachable via ptr, clear the
+ // heuristic.
+ if (ex->heuristic && ptr == ch->data) {
+ // ch was up to now considered as reachable dur to
+ // ex->heuristic. We have a direct ptr now => clear
+ // the heuristic field.
+ ex->heuristic = LchNone;
+ }
+ return;
+ }
// Possibly upgrade the state, ie. one of:
// - Unreached --> Possible
// - Unreached --> Reachable
// - Possible --> Reachable
- if (ptr == ch->data && is_prior_definite && ex->state != Reachable) {
- // 'ptr' points to the start of the block, and the prior node is
+
+ if (ptr == ch->data)
+ ch_via_ptr = Reachable;
+ else if (detect_memory_leaks_last_heuristics) {
+ ex->heuristic
+ = heuristic_reachedness (ptr, ch, ex,
+ detect_memory_leaks_last_heuristics);
+ if (ex->heuristic)
+ ch_via_ptr = Reachable;
+ else
+ ch_via_ptr = Possible;
+ } else
+ ch_via_ptr = Possible;
+
+ if (ch_via_ptr == Reachable && is_prior_definite) {
+ // 'ptr' points to the start of the block or is to be considered as
+ // pointing to the start of the block, and the prior node is
// definite, which means that this block is definitely reachable.
ex->state = Reachable;
@@ -657,7 +875,8 @@
}
static void
-lc_push_if_a_chunk_ptr(Addr ptr, Int clique, Int cur_clique, Bool is_prior_definite)
+lc_push_if_a_chunk_ptr(Addr ptr,
+ Int clique, Int cur_clique, Bool is_prior_definite)
{
if (-1 == clique)
lc_push_without_clique_if_a_chunk_ptr(ptr, is_prior_definite);
@@ -703,11 +922,12 @@
// 2. Search ptr mode (searched != 0).
// -----------------------------------
// In this mode, searches for pointers to a specific address range
-// In such a case, lc_scan_memory just scans [start..start+len[ for pointers to searched
-// and outputs the places where searched is found. It does not recursively scans the
-// found memory.
+// In such a case, lc_scan_memory just scans [start..start+len[ for pointers
+// to searched and outputs the places where searched is found.
+// It does not recursively scans the found memory.
static void
-lc_scan_memory(Addr start, SizeT len, Bool is_prior_definite, Int clique, Int cur_clique,
+lc_scan_memory(Addr start, SizeT len, Bool is_prior_definite,
+ Int clique, Int cur_clique,
Addr searched, SizeT szB)
{
/* memory scan is based on the assumption that valid pointers are aligned
@@ -795,12 +1015,28 @@
// let's see if its contents point to a chunk.
if (UNLIKELY(searched)) {
if (addr >= searched && addr < searched + szB) {
- if (addr == searched)
+ if (addr == searched) {
VG_(umsg)("*%#lx points at %#lx\n", ptr, searched);
- else
+ MC_(pp_describe_addr) (ptr);
+ } else {
+ Int ch_no;
+ MC_Chunk *ch;
+ LC_Extra *ex;
VG_(umsg)("*%#lx interior points at %lu bytes inside %#lx\n",
ptr, (long unsigned) addr - searched, searched);
- MC_(pp_describe_addr) (ptr);
+ MC_(pp_describe_addr) (ptr);
+ if (lc_is_a_chunk_ptr(addr, &ch_no, &ch, &ex) ) {
+ Int h;
+ for (h = LchStdString; h <= LchMultipleInheritance; h++) {
+ if (heuristic_reachedness(addr, ch, ex, H2S(h)) == h) {
+ VG_(umsg)("block at %#lx considered reachable "
+ "by ptr %#lx using %s heuristic\n",
+ ch->data, addr, pp_heuristic(h));
+ }
+ }
+ tl_assert (h == N_LEAK_CHECK_HEURISTICS - 1);
+ }
+ }
}
} else {
lc_push_if_a_chunk_ptr(addr, clique, cur_clique, is_prior_definite);
@@ -947,7 +1183,8 @@
Int i, n_lossrecords, start_lr_output_scan;
LossRecord* lr;
Bool is_suppressed;
- SizeT old_bytes_leaked = MC_(bytes_leaked); /* to report delta in summary */
+ /* old_* variables are used to report delta in summary. */
+ SizeT old_bytes_leaked = MC_(bytes_leaked);
SizeT old_bytes_indirect = MC_(bytes_indirect);
SizeT old_bytes_dubious = MC_(bytes_dubious);
SizeT old_bytes_reachable = MC_(bytes_reachable);
@@ -958,6 +1195,18 @@
SizeT old_blocks_reachable = MC_(blocks_reachable);
SizeT old_blocks_suppressed = MC_(blocks_suppressed);
+ SizeT old_bytes_heuristically_reachable[N_LEAK_CHECK_HEURISTICS];
+ SizeT old_blocks_heuristically_reachable[N_LEAK_CHECK_HEURISTICS];
+
+ for (i = 0; i < N_LEAK_CHECK_HEURISTICS; i++) {
+ old_bytes_heuristically_reachable[i]
+ = MC_(bytes_heuristically_reachable)[i];
+ MC_(bytes_heuristically_reachable)[i] = 0;
+ old_blocks_heuristically_reachable[i]
+ = MC_(blocks_heuristically_reachable)[i];
+ MC_(blocks_heuristically_reachable)[i] = 0;
+ }
+
if (lr_table == NULL)
// Create the lr_table, which holds the loss records.
// If the lr_table already exists, it means it contains
@@ -1003,6 +1252,15 @@
lrkey.state = ex->state;
lrkey.allocated_at = MC_(allocated_at)(ch);
+ if (ex->heuristic) {
+ MC_(bytes_heuristically_reachable)[ex->heuristic] += ch->szB;
+ MC_(blocks_heuristically_reachable)[ex->heuristic]++;
+ if (VG_DEBUG_LEAKCHECK)
+ VG_(printf)("heuristic %s %#lx len %lu\n",
+ pp_heuristic(ex->heuristic),
+ ch->data, (unsigned long)ch->szB);
+ }
+
old_lr = VG_(OSetGen_Lookup)(lr_table, &lrkey);
if (old_lr) {
// We found an existing loss record matching this chunk. Update the
@@ -1108,45 +1366,68 @@
if (VG_(clo_verbosity) > 0 && !VG_(clo_xml)) {
HChar d_bytes[20];
HChar d_blocks[20];
+# define DBY(new,old) \
+ MC_(snprintf_delta) (d_bytes, 20, (new), (old), lcp->deltamode)
+# define DBL(new,old) \
+ MC_(snprintf_delta) (d_blocks, 20, (new), (old), lcp->deltamode)
VG_(umsg)("LEAK SUMMARY:\n");
VG_(umsg)(" definitely lost: %'lu%s bytes in %'lu%s blocks\n",
MC_(bytes_leaked),
- MC_(snprintf_delta) (d_bytes, 20, MC_(bytes_leaked), old_bytes_leaked, lcp->deltamode),
+ DBY (MC_(bytes_leaked), old_bytes_leaked),
MC_(blocks_leaked),
- MC_(snprintf_delta) (d_blocks, 20, MC_(blocks_leaked), old_blocks_leaked, lcp->deltamode));
+ DBL (MC_(blocks_leaked), old_blocks_leaked));
VG_(umsg)(" indirectly lost: %'lu%s bytes in %'lu%s blocks\n",
MC_(bytes_indirect),
- MC_(snprintf_delta) (d_bytes, 20, MC_(bytes_indirect), old_bytes_indirect, lcp->deltamode),
+ DBY (MC_(bytes_indirect), old_bytes_indirect),
MC_(blocks_indirect),
- MC_(snprintf_delta) (d_blocks, 20, MC_(blocks_indirect), old_blocks_indirect, lcp->deltamode) );
+ DBL (MC_(blocks_indirect), old_blocks_indirect));
VG_(umsg)(" possibly lost: %'lu%s bytes in %'lu%s blocks\n",
MC_(bytes_dubious),
- MC_(snprintf_delta) (d_bytes, 20, MC_(bytes_dubious), old_bytes_dubious, lcp->deltamode),
+ DBY (MC_(bytes_dubious), old_bytes_dubious),
MC_(blocks_dubious),
- MC_(snprintf_delta) (d_blocks, 20, MC_(blocks_dubious), old_blocks_dubious, lcp->deltamode) );
+ DBL (MC_(blocks_dubious), old_blocks_dubious));
VG_(umsg)(" still reachable: %'lu%s bytes in %'lu%s blocks\n",
MC_(bytes_reachable),
- MC_(snprintf_delta) (d_bytes, 20, MC_(bytes_reachable), old_bytes_reachable, lcp->deltamode),
+ DBY (MC_(bytes_reachable), old_bytes_reachable),
MC_(blocks_reachable),
- MC_(snprintf_delta) (d_blocks, 20, MC_(blocks_reachable), old_blocks_reachable, lcp->deltamode) );
+ DBL (MC_(blocks_reachable), old_blocks_reachable));
+ for (i = 0; i < N_LEAK_CHECK_HEURISTICS; i++)
+ if (old_blocks_heuristically_reachable[i] > 0
+ || MC_(blocks_heuristically_reachable)[i] > 0) {
+ VG_(umsg)(" of which "
+ "reachable via heuristic:\n");
+ break;
+ }
+ for (i = 0; i < N_LEAK_CHECK_HEURISTICS; i++)
+ if (old_blocks_heuristically_reachable[i] > 0
+ || MC_(blocks_heuristically_reachable)[i] > 0)
+ VG_(umsg)(" %19s: "
+ "%'lu%s bytes in %'lu%s blocks\n",
+ pp_heuristic(i),
+ MC_(bytes_heuristically_reachable)[i],
+ DBY (MC_(bytes_heuristically_reachable)[i],
+ old_bytes_heuristically_reachable[i]),
+ MC_(blocks_heuristically_reachable)[i],
+ DBL (MC_(blocks_heuristically_reachable)[i],
+ old_blocks_heuristically_reachable[i]));
VG_(umsg)(" suppressed: %'lu%s bytes in %'lu%s blocks\n",
MC_(bytes_suppressed),
- MC_(snprintf_delta) (d_bytes, 20, MC_(bytes_suppressed), old_bytes_suppressed, lcp->deltamode),
+ DBY (MC_(bytes_suppressed), old_bytes_suppressed),
MC_(blocks_suppressed),
- MC_(snprintf_delta) (d_blocks, 20, MC_(blocks_suppressed), old_blocks_suppressed, lcp->deltamode) );
+ DBL (MC_(blocks_suppressed), old_blocks_suppressed));
if (lcp->mode != LC_Full &&
(MC_(blocks_leaked) + MC_(blocks_indirect) +
MC_(blocks_dubious) + MC_(blocks_reachable)) > 0) {
if (lcp->requested_by_monitor_command)
- VG_(umsg)("To see details of leaked memory, give 'full' arg to leak_check\n");
+ VG_(umsg)("To see details of leaked memory, "
+ "give 'full' arg to leak_check\n");
else
VG_(umsg)("Rerun with --leak-check=full to see details "
"of leaked memory\n");
}
if (lcp->mode == LC_Full &&
- MC_(blocks_reachable) > 0 && !RiS(Reachable,lcp->show_leak_kinds))
- {
+ MC_(blocks_reachable) > 0 && !RiS(Reachable,lcp->show_leak_kinds)) {
VG_(umsg)("Reachable blocks (those to which a pointer "
"was found) are not shown.\n");
if (lcp->requested_by_monitor_command)
@@ -1156,6 +1437,8 @@
"--show-leak-kinds=all\n");
}
VG_(umsg)("\n");
+ #undef DBL
+ #undef DBY
}
}
@@ -1169,7 +1452,8 @@
for (ind = 0; ind < lc_n_chunks; ind++) {
LC_Extra* ind_ex = &(lc_extras)[ind];
- if (ind_ex->state == IndirectLeak && ind_ex->IorC.clique == (SizeT) clique) {
+ if (ind_ex->state == IndirectLeak
+ && ind_ex->IorC.clique == (SizeT) clique) {
MC_Chunk* ind_ch = lc_chunks[ind];
LossRecord* ind_lr;
LossRecordKey ind_lrkey;
@@ -1232,7 +1516,7 @@
old_lr = VG_(OSetGen_Lookup)(lr_table, &lrkey);
if (old_lr) {
// We found an existing loss record matching this chunk.
- // If this is the loss record we are looking for, then output the pointer.
+ // If this is the loss record we are looking for, output the pointer.
if (old_lr == lr_array[loss_record_nr]) {
VG_(umsg)("%p[%lu]\n",
(void *)ch->data, (unsigned long) ch->szB);
@@ -1257,8 +1541,8 @@
// If searched = 0, scan memory root set, pushing onto the mark stack the blocks
// encountered.
-// Otherwise (searched != 0), scan the memory root set searching for ptr pointing
-// inside [searched, searched+szB[.
+// Otherwise (searched != 0), scan the memory root set searching for ptr
+// pointing inside [searched, searched+szB[.
static void scan_memory_root_set(Addr searched, SizeT szB)
{
Int i;
@@ -1332,6 +1616,7 @@
MC_(detect_memory_leaks_last_delta_mode) = lcp->deltamode;
+ detect_memory_leaks_last_heuristics = lcp->heuristics;
// Get the chunks, stop if there were none.
if (lc_chunks) {
@@ -1438,6 +1723,7 @@
for (i = 0; i < lc_n_chunks; i++) {
lc_extras[i].state = Unreached;
lc_extras[i].pending = False;
+ lc_extras[i].heuristic = LchNone;
lc_extras[i].IorC.indirect_szB = 0;
}
@@ -1540,11 +1826,12 @@
VG_(umsg) ("Searching for pointers pointing in %lu bytes from %#lx\n",
szB, address);
+ chunks = find_active_chunks(&n_chunks);
+
// Scan memory root-set, searching for ptr pointing in address[szB]
scan_memory_root_set(address, szB);
// Scan active malloc-ed chunks
- chunks = find_active_chunks(&n_chunks);
for (i = 0; i < n_chunks; i++) {
lc_scan_memory(chunks[i]->data, chunks[i]->szB,
/*is_prior_definite*/True,
Modified: trunk/memcheck/mc_main.c
==============================================================================
--- trunk/memcheck/mc_main.c (original)
+++ trunk/memcheck/mc_main.c Sun Sep 29 13:47:32 2013
@@ -5021,12 +5021,62 @@
VgRes MC_(clo_leak_resolution) = Vg_HighRes;
UInt MC_(clo_show_leak_kinds) = R2S(Possible) | R2S(Unreached);
UInt MC_(clo_error_for_leak_kinds) = R2S(Possible) | R2S(Unreached);
+UInt MC_(clo_leak_check_heuristics) = 0;
Bool MC_(clo_workaround_gcc296_bugs) = False;
Int MC_(clo_malloc_fill) = -1;
Int MC_(clo_free_fill) = -1;
KeepStacktraces MC_(clo_keep_stacktraces) = KS_alloc_then_free;
Int MC_(clo_mc_level) = 2;
+static Bool MC_(parse_leak_heuristics) ( const HChar *str0, UInt *lhs )
+{
+ SizeT str0len = VG_(strlen)(str0);
+ if (str0len > 1000) return False; /* "obviously invalid" */
+ HChar tok_str0[str0len+1];
+ HChar *saveptr;
+ HChar *token;
+
+ Bool seen_all_kw = False;
+ Bool seen_none_kw = False;
+
+ VG_(strcpy) (tok_str0, str0);
+ *lhs = 0;
+
+ for (token = VG_(strtok_r)(tok_str0, ",", &saveptr);
+ token;
+ token = VG_(strtok_r)(NULL, ",", &saveptr)) {
+ if (0 == VG_(strcmp)(token, "stdstring"))
+ *lhs |= H2S(LchStdString);
+ else if (0 == VG_(strcmp)(token, "newarray"))
+ *lhs |= H2S(LchNewArray);
+ else if (0 == VG_(strcmp)(token, "multipleinheritance"))
+ *lhs |= H2S(LchMultipleInheritance);
+ else if (0 == VG_(strcmp)(token, "all"))
+ seen_all_kw = True;
+ else if (0 == VG_(strcmp)(token, "none"))
+ seen_none_kw = True;
+ else
+ return False;
+ }
+
+ if (seen_all_kw) {
+ if (seen_none_kw || *lhs)
+ return False; // mixing all with either none or a specific value.
+ *lhs = HallS;
+ } else if (seen_none_kw) {
+ if (seen_all_kw || *lhs)
+ return False; // mixing none with either all or a specific value.
+ *lhs = 0;
+ } else {
+ // seen neither all or none, we must see at least one value
+ if (*lhs == 0)
+ return False;
+ }
+
+ return True;
+}
+
+
static Bool mc_process_cmd_line_options(const HChar* arg)
{
const HChar* tmp_str;
@@ -5079,6 +5129,10 @@
if (!MC_(parse_leak_kinds)(tmp_str, &MC_(clo_show_leak_kinds)))
return False;
}
+ else if VG_STR_CLO(arg, "--leak-check-heuristics", tmp_str) {
+ if (!MC_(parse_leak_heuristics)(tmp_str, &MC_(clo_leak_check_heuristics)))
+ return False;
+ }
else if (VG_BOOL_CLO(arg, "--show-reachable", tmp_show)) {
if (tmp_show) {
MC_(clo_show_leak_kinds) = RallS;
@@ -5182,6 +5236,9 @@
" --errors-for-leak-kinds=kind1,kind2,.. which leak kinds are errors?\n"
" [definite,possible]\n"
" where kind is one of definite indirect possible reachable all none\n"
+" --leak-check-heuristics=heur1,heur2,... which heuristics to use for\n"
+" improving leak search false positive [none]\n"
+" where heur is one of stdstring newarray multipleinheritance all none\n"
" --show-reachable=yes same as --show-leak-kinds=all\n"
" --show-reachable=no --show-possibly-lost=yes\n"
" same as --show-leak-kinds=definite,possible\n"
@@ -5313,10 +5370,12 @@
" and outputs a description of <addr>\n"
" leak_check [full*|summary]\n"
" [kinds kind1,kind2,...|reachable|possibleleak*|definiteleak]\n"
+" [heuristics heur1,heur2,...]\n"
" [increased*|changed|any]\n"
" [unlimited*|limited <max_loss_records_output>]\n"
" * = defaults\n"
" where kind is one of definite indirect possible reachable all none\n"
+" where heur is one of stdstring newarray multipleinheritance all none\n"
" Examples: leak_check\n"
" leak_check summary any\n"
" leak_check full kinds indirect,possible\n"
@@ -5408,6 +5467,7 @@
switch (VG_(keyword_id)
("full summary "
"kinds reachable possibleleak definiteleak "
+ "heuristics "
"increased changed any "
"unlimited limited ",
kw, kwd_report_all)) {
@@ -5436,15 +5496,24 @@
case 5: /* definiteleak */
lcp.show_leak_kinds = R2S(Unreached);
break;
- case 6: /* increased */
+ case 6: { /* heuristics */
+ wcmd = VG_(strtok_r) (NULL, " ", &ssaveptr);
+ if (wcmd == NULL || !MC_(parse_leak_heuristics)(wcmd,
+ &lcp.heuristics)) {
+ VG_(gdb_printf) ("missing or malformed heuristics set\n");
+ err++;
+ }
+ break;
+ }
+ case 7: /* increased */
lcp.deltamode = LCD_Increased; break;
- case 7: /* changed */
+ case 8: /* changed */
lcp.deltamode = LCD_Changed; break;
- case 8: /* any */
+ case 9: /* any */
lcp.deltamode = LCD_Any; break;
- case 9: /* unlimited */
+ case 10: /* unlimited */
lcp.max_loss_records_output = 999999999; break;
- case 10: { /* limited */
+ case 11: { /* limited */
Int int_value;
const HChar* endptr;
@@ -5515,7 +5584,7 @@
switch (kwdid) {
case -2: break;
case -1: break;
- case 0:
+ case 0: /* addressable */
if (is_mem_addressable ( address, szB, &bad_addr ))
VG_(printf) ("Address %p len %ld addressable\n",
(void *)address, szB);
@@ -5525,7 +5594,8 @@
(void *)address, szB, (void *) bad_addr);
MC_(pp_describe_addr) (address);
break;
- case 1: res = is_mem_defined ( address, szB, &bad_addr, &otag );
+ case 1: /* defined */
+ res = is_mem_defined ( address, szB, &bad_addr, &otag );
if (MC_AddrErr == res)
VG_(printf)
("Address %p len %ld not addressable:\nbad address %p\n",
@@ -5679,6 +5749,7 @@
lcp.show_leak_kinds = MC_(clo_show_leak_kinds);
lcp.errors_for_leak_kinds = MC_(clo_error_for_leak_kinds);
+ lcp.heuristics = MC_(clo_leak_check_heuristics);
if (arg[2] == 0)
lcp.deltamode = LCD_Any;
@@ -6458,6 +6529,7 @@
LeakCheckParams lcp;
lcp.mode = MC_(clo_leak_check);
lcp.show_leak_kinds = MC_(clo_show_leak_kinds);
+ lcp.heuristics = MC_(clo_leak_check_heuristics);
lcp.errors_for_leak_kinds = MC_(clo_error_for_leak_kinds);
lcp.deltamode = LCD_Any;
lcp.max_loss_records_output = 999999999;
Modified: trunk/memcheck/tests/Makefile.am
==============================================================================
--- trunk/memcheck/tests/Makefile.am (original)
+++ trunk/memcheck/tests/Makefile.am Sun Sep 29 13:47:32 2013
@@ -88,6 +88,7 @@
cond_st.vgtest cond_st.stdout.exp cond_st.stderr.exp-arm \
cond_st.stderr.exp-64bit-non-arm \
cond_st.stderr.exp-32bit-non-arm \
+ leak_cpp_interior.stderr.exp leak_cpp_interior.stderr.exp-64bit leak_cpp_interior.vgtest \
custom_alloc.stderr.exp custom_alloc.vgtest \
custom_alloc.stderr.exp-s390x-mvc \
custom-overlap.stderr.exp custom-overlap.vgtest \
@@ -282,6 +283,7 @@
clireq_nofill \
clo_redzone \
cond_ld_st \
+ leak_cpp_interior \
custom_alloc \
custom-overlap \
deep-backtrace \
@@ -365,6 +367,8 @@
atomic_incs_CFLAGS = $(AM_CFLAGS)
endif
+leak_cpp_interior_SOURCES = leak_cpp_interior.cpp
+
deep_templates_SOURCES = deep_templates.cpp
deep_templates_CXXFLAGS = $(AM_CFLAGS) -O -gstabs
Added: trunk/memcheck/tests/leak_cpp_interior.cpp
==============================================================================
--- trunk/memcheck/tests/leak_cpp_interior.cpp (added)
+++ trunk/memcheck/tests/leak_cpp_interior.cpp Sun Sep 29 13:47:32 2013
@@ -0,0 +1,105 @@
+#include <stdio.h>
+#include <unistd.h>
+#include <string>
+#include "../memcheck.h"
+// Derived from test provided by Timur Iskhodzhanov (bug 280271)
+
+
+class MyClass
+{
+public:
+ ~MyClass()
+ { fprintf(stderr, "destruct MyClass\n");
+ }
+};
+
+// Two hierarchies using MI, one with no fields,
+// the other one with some data.
+struct Ae
+{
+ virtual ~Ae()
+ { fprintf(stderr, "destruct Ae\n");
+ }
+};
+struct Be
+{
+ virtual ~Be()
+ { fprintf(stderr, "destruct Be\n");
+ }
+};
+struct Ce : public Ae, public Be
+{
+ virtual ~Ce()
+ { fprintf(stderr, "destruct Ce\n");
+ }
+};
+
+struct A
+{
+ char a;
+ A()
+ { a = 'a';
+ }
+ virtual ~A()
+ { fprintf(stderr, "destruct A\n");
+ }
+};
+struct B
+{
+ char b;
+ B()
+ { b = 'b';
+ }
+ virtual ~B()
+ { fprintf(stderr, "destruct B\n");
+ }
+};
+struct C : public A, public B
+{
+ char c;
+ C()
+ { c = 'c';
+ }
+ virtual ~C()
+ { fprintf(stderr, "destruct C\n");
+ }
+};
+
+int main() {
+ std::string str = "Valgrind"; // interior ptr.
+ std::string str2 = str;
+ MyClass *ptr = new MyClass[3]; // interior ptr.
+ MyClass *ptr2 = new MyClass[0]; // "interior but exterior ptr".
+ // ptr2 points after the chunk, is wrongly considered by memcheck as definitely leaked.
+
+ Be *ptrBCe = new Ce; // interior ptr.
+ Ae *ptrACe = new Ce; // not an interior pointer.
+ B *ptrBC = new C; // interior ptr.
+ A *ptrAC = new C; // not an interior pointer.
+
+
+ str2 += " rocks (str2)\n"; // interior ptr.
+
+ VALGRIND_MONITOR_COMMAND("v.set log_output");
+
+ fprintf(stderr, "VALGRIND_DO_LEAK_CHECK\n");
+ VALGRIND_DO_LEAK_CHECK; // All possible leaks should be detected, giving only reachable data.
+
+ // Check individually each heuristic
+ fprintf(stderr, "leak_check summary heuristics multipleinheritance\n");
+ VALGRIND_MONITOR_COMMAND("leak_check summary heuristics multipleinheritance");
+ fprintf(stderr, "leak_check summary any heuristics newarray\n");
+ VALGRIND_MONITOR_COMMAND("leak_check summary heuristics newarray");
+ fprintf(stderr, "leak_check summary heuristics stdstring\n");
+ VALGRIND_MONITOR_COMMAND("leak_check summary heuristics stdstring");
+
+ delete [] ptr;
+ delete [] ptr2;
+ delete ptrBCe;
+ delete ptrACe;
+ delete ptrBC;
+ delete ptrAC;
+ fprintf(stderr, "Finished!\n");
+ return 0;
+}
+
Added: trunk/memcheck/tests/leak_cpp_interior.stderr.exp
==============================================================================
--- trunk/memcheck/tests/leak_cpp_interior.stderr.exp (added)
+++ trunk/memcheck/tests/leak_cpp_interior.stderr.exp Sun Sep 29 13:47:32 2013
@@ -0,0 +1,81 @@
+
+valgrind output will go to log
+VALGRIND_DO_LEAK_CHECK
+4 bytes in 1 blocks are definitely lost in loss record ... of ...
+ by 0x........: main (leak_cpp_interior.cpp:72)
+
+LEAK SUMMARY:
+ definitely lost: 4 bytes in 1 blocks
+ indirectly lost: 0 bytes in 0 blocks
+ possibly lost: 0 bytes in 0 blocks
+ still reachable: 111 bytes in 7 blocks
+ of which reachable via heuristic:
+ stdstring : 56 bytes in 2 blocks
+ newarray : 7 bytes in 1 blocks
+ multipleinheritance: 24 bytes in 2 blocks
+ suppressed: 0 bytes in 0 blocks
+Reachable blocks (those to which a pointer was found) are not shown.
+To see them, rerun with: --leak-check=full --show-leak-kinds=all
+
+leak_check summary heuristics multipleinheritance
+LEAK SUMMARY:
+ definitely lost: 4 (+0) bytes in 1 (+0) blocks
+ indirectly lost: 0 (+0) bytes in 0 (+0) blocks
+ possibly lost: 63 (+63) bytes in 3 (+3) blocks
+ still reachable: 48 (-63) bytes in 4 (-3) blocks
+ of which reachable via heuristic:
+ stdstring : 0 (-56) bytes in 0 (-2) blocks
+ newarray : 0 (-7) bytes in 0 (-1) blocks
+ multipleinheritance: 24 (+0) bytes in 2 (+0) blocks
+ suppressed: 0 (+0) bytes in 0 (+0) blocks
+To see details of leaked memory, give 'full' arg to leak_check
+
+leak_check summary any heuristics newarray
+LEAK SUMMARY:
+ definitely lost: 4 (+0) bytes in 1 (+0) blocks
+ indirectly lost: 0 (+0) bytes in 0 (+0) blocks
+ possibly lost: 80 (+17) bytes in 4 (+1) blocks
+ still reachable: 31 (-17) bytes in 3 (-1) blocks
+ of which reachable via heuristic:
+ newarray : 7 (+7) bytes in 1 (+1) blocks
+ multipleinheritance: 0 (-24) bytes in 0 (-2) blocks
+ suppressed: 0 (+0) bytes in 0 (+0) blocks
+To see details of leaked memory, give 'full' arg to leak_check
+
+leak_check summary heuristics stdstring
+LEAK SUMMARY:
+ definitely lost: 4 (+0) bytes in 1 (+0) blocks
+ indirectly lost: 0 (+0) bytes in 0 (+0) blocks
+ possibly lost: 31 (-49) bytes in 3 (-1) blocks
+ still reachable: 80 (+49) bytes in 4 (+1) blocks
+ of which reachable via heuristic:
+ stdstring : 56 (+56) bytes in 2 (+2) blocks
+ newarray : 0 (-7) bytes in 0 (-1) blocks
+ suppressed: 0 (+0) bytes in 0 (+0) blocks
+To see details of leaked memory, give 'full' arg to leak_check
+
+destruct MyClass
+destruct MyClass
+destruct MyClass
+destruct Ce
+destruct Be
+destruct Ae
+destruct Ce
+destruct Be
+destruct Ae
+destruct C
+destruct B
+destruct A
+destruct C
+destruct B
+destruct A
+Finished!
+
+HEAP SUMMARY:
+ in use at exit: 0 bytes in 0 blocks
+ total heap usage: 8 allocs, 8 frees, 115 bytes allocated
+
+All heap blocks were freed -- no leaks are possible
+
+For counts of detected and suppressed errors, rerun with: -v
+ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)
Added: trunk/memcheck/tests/leak_cpp_interior.stderr.exp-64bit
==============================================================================
--- trunk/memcheck/tests/leak_cpp_interior.stderr.exp-64bit (added)
+++ trunk/memcheck/tests/leak_cpp_interior.stderr.exp-64bit Sun Sep 29 13:47:32 2013
@@ -0,0 +1,81 @@
+
+valgrind output will go to log
+VALGRIND_DO_LEAK_CHECK
+8 bytes in 1 blocks are definitely lost in loss record ... of ...
+ by 0x........: main (leak_cpp_interior.cpp:72)
+
+LEAK SUMMARY:
+ definitely lost: 8 bytes in 1 blocks
+ indirectly lost: 0 bytes in 0 blocks
+ possibly lost: 0 bytes in 0 blocks
+ still reachable: 187 bytes in 7 blocks
+ of which reachable via heuristic:
+ stdstring : 80 bytes in 2 blocks
+ newarray : 11 bytes in 1 blocks
+ multipleinheritance: 48 bytes in 2 blocks
+ suppressed: 0 bytes in 0 blocks
+Reachable blocks (those to which a pointer was found) are not shown.
+To see them, rerun with: --leak-check=full --show-leak-kinds=all
+
+leak_check summary heuristics multipleinheritance
+LEAK SUMMARY:
+ definitely lost: 8 (+0) bytes in 1 (+0) blocks
+ indirectly lost: 0 (+0) bytes in 0 (+0) blocks
+ possibly lost: 91 (+91) bytes in 3 (+3) blocks
+ still reachable: 96 (-91) bytes in 4 (-3) blocks
+ of which reachable via heuristic:
+ stdstring : 0 (-80) bytes in 0 (-2) blocks
+ newarray : 0 (-11) bytes in 0 (-1) blocks
+ multipleinheritance: 48 (+0) bytes in 2 (+0) blocks
+ suppressed: 0 (+0) bytes in 0 (+0) blocks
+To see details of leaked memory, give 'full' arg to leak_check
+
+leak_check summary any heuristics newarray
+LEAK SUMMARY:
+ definitely lost: 8 (+0) bytes in 1 (+0) blocks
+ indirectly lost: 0 (+0) bytes in 0 (+0) blocks
+ possibly lost: 128 (+37) bytes in 4 (+1) blocks
+ still reachable: 59 (-37) bytes in 3 (-1) blocks
+ of which reachable via heuristic:
+ newarray : 11 (+11) bytes in 1 (+1) blocks
+ multipleinheritance: 0 (-48) bytes in 0 (-2) blocks
+ suppressed: 0 (+0) bytes in 0 (+0) blocks
+To see details of leaked memory, give 'full' arg to leak_check
+
+leak_check summary heuristics stdstring
+LEAK SUMMARY:
+ definitely lost: 8 (+0) bytes in 1 (+0) blocks
+ indirectly lost: 0 (+0) bytes in 0 (+0) blocks
+ possibly lost: 59 (-69) bytes in 3 (-1) blocks
+ still reachable: 128 (+69) bytes in 4 (+1) blocks
+ of which reachable via heuristic:
+ stdstring : 80 (+80) bytes in 2 (+2) blocks
+ newarray : 0 (-11) bytes in 0 (-1) blocks
+ suppressed: 0 (+0) bytes in 0 (+0) blocks
+To see details of leaked memory, give 'full' arg to leak_check
+
+destruct MyClass
+destruct MyClass
+destruct MyClass
+destruct Ce
+destruct Be
+destruct Ae
+destruct Ce
+destruct Be
+destruct Ae
+destruct C
+destruct B
+destruct A
+destruct C
+destruct B
+destruct A
+Finished!
+
+HEAP SUMMARY:
+ in use at exit: 0 bytes in 0 blocks
+ total heap usage: 8 allocs, 8 frees, 195 bytes allocated
+
+All heap blocks were freed -- no leaks are possible
+
+For counts of detected and suppressed errors, rerun with: -v
+ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)
Added: trunk/memcheck/tests/leak_cpp_interior.vgtest
==============================================================================
--- trunk/memcheck/tests/leak_cpp_interior.vgtest (added)
+++ trunk/memcheck/tests/leak_cpp_interior.vgtest Sun Sep 29 13:47:32 2013
@@ -0,0 +1,2 @@
+prog: leak_cpp_interior
+vgopts: --leak-check=summary --leak-check-heuristics=multipleinheritance,stdstring,newarray
|
|
From: Matthias S. <zz...@ge...> - 2013-10-02 05:01:55
|
On 29.09.2013 15:47, sv...@va... wrote:
> Author: philippe
> Date: Sun Sep 29 13:47:32 2013
> New Revision: 13582
>
> Log:
> add heuristics decreasing false possible "possible leaks" in c++ code.
>
> The option --leak-check-heuristics=heur1,heur2,... can activate
> various heuristics to decrease the number of false positive
> "possible leaks" for C++ code. The available heuristics are
> detecting valid interior pointers to std::stdstring, to new[] allocated
> arrays with elements having destructors and to interior pointers pointing
> to an inner part of a C++ object using multiple inheritance.
>
> This fixes 280271 Valgrind reports possible memory leaks on still-reachable
> std::string
>
> This has been tested on x86/amd64/ppc32/ppc64.
>
> First performance measurements seems to show a neglectible impact on
> the leak search.
>
> More feedback welcome both on performance and functional aspects
> (false positive 'possibly leaked' rate decrease and/or
> false negative 'possibly leaked' rate increase).
>
> Note that the heuristic is not checking that the memory has been
> allocated with "new" or "new[]", as it is expected that in some cases,
> specific alloc fn are used for c++ objects instead of the standard new/new[].
> If needed, we might add an option to check the alloc functions
> to be new/new[].
>
[...]
> static void
> -lc_scan_memory(Addr start, SizeT len, Bool is_prior_definite, Int clique, Int cur_clique,
> +lc_scan_memory(Addr start, SizeT len, Bool is_prior_definite,
> + Int clique, Int cur_clique,
> Addr searched, SizeT szB)
> {
> /* memory scan is based on the assumption that valid pointers are aligned
> @@ -795,12 +1015,28 @@
> // let's see if its contents point to a chunk.
> if (UNLIKELY(searched)) {
> if (addr >= searched && addr < searched + szB) {
> - if (addr == searched)
> + if (addr == searched) {
> VG_(umsg)("*%#lx points at %#lx\n", ptr, searched);
> - else
> + MC_(pp_describe_addr) (ptr);
> + } else {
> + Int ch_no;
> + MC_Chunk *ch;
> + LC_Extra *ex;
> VG_(umsg)("*%#lx interior points at %lu bytes inside %#lx\n",
> ptr, (long unsigned) addr - searched, searched);
> - MC_(pp_describe_addr) (ptr);
> + MC_(pp_describe_addr) (ptr);
> + if (lc_is_a_chunk_ptr(addr, &ch_no, &ch, &ex) ) {
> + Int h;
> + for (h = LchStdString; h <= LchMultipleInheritance; h++) {
> + if (heuristic_reachedness(addr, ch, ex, H2S(h)) == h) {
> + VG_(umsg)("block at %#lx considered reachable "
> + "by ptr %#lx using %s heuristic\n",
> + ch->data, addr, pp_heuristic(h));
> + }
> + }
> + tl_assert (h == N_LEAK_CHECK_HEURISTICS - 1);
Hi!
This assert in line mc_leakcheck.c, line 1037 fails for me, when doing
gdb monitor command who_points_at on a possibly leaked block.
I wonder if the existing gdb test does not run into this.
The assert must be:
tl_assert (h == N_LEAK_CHECK_HEURISTICS);
I wonder why the loop before also does not use the check "h <
N_LEAK_CHECK_HEURISTICS" if there already is a constant
Regards
Matthias
|
|
From: Philippe W. <phi...@sk...> - 2013-10-02 21:03:25
|
On Wed, 2013-10-02 at 07:01 +0200, Matthias Schwarzott wrote:
> > + for (h = LchStdString; h <= LchMultipleInheritance; h++) {
> > + if (heuristic_reachedness(addr, ch, ex, H2S(h)) == h) {
> > + VG_(umsg)("block at %#lx considered reachable "
> > + "by ptr %#lx using %s heuristic\n",
> > + ch->data, addr, pp_heuristic(h));
> > + }
> > + }
> > + tl_assert (h == N_LEAK_CHECK_HEURISTICS - 1);
>
> Hi!
>
> This assert in line mc_leakcheck.c, line 1037 fails for me, when doing
> gdb monitor command who_points_at on a possibly leaked block.
Thanks, fixed the problem in rev 13609.
> I wonder if the existing gdb test does not run into this.
No, because there was no test testing who_points_at with an
"interior-ly" pointed block.
This is tested now :).
>
> The assert must be:
>
> tl_assert (h == N_LEAK_CHECK_HEURISTICS);
>
> I wonder why the loop before also does not use the check "h <
> N_LEAK_CHECK_HEURISTICS" if there already is a constant
The idea of the assert is to double check the consistency between
the enum and the N_LEAK_CHECK_HEURISTICS: if the enum is changed
without changing the N_LEAK_CHECK_HEURISTICS, this assert should
detect it.
This detection half-worked in this case, except there was no test :(.
Have you tested these heuristics with a (big) c++ application ?
Philippe
|
|
From: Matthias S. <zz...@ge...> - 2013-10-07 19:22:25
|
On 02.10.2013 23:03, Philippe Waroquiers wrote:
> On Wed, 2013-10-02 at 07:01 +0200, Matthias Schwarzott wrote:
> No, because there was no test testing who_points_at with an
> "interior-ly" pointed block.
> This is tested now :).
Good, works!
>
> This detection half-worked in this case, except there was no test :(.
>
>
> Have you tested these heuristics with a (big) c++ application ?
Yes, I tested it with a big c++ application.
And all three kinds of heuristics apply to some blocks.
But I need to do some real comparison of the results of a short run,
with and without the heuristics, but as the application also has real
leaks it is not only checking if the leaks-list is empty.
Will it work to add some code after the shutdown of our application to
call into the leaks checker like this to show full leak list and the see
which blocks disappear by adding one heuristic at a time (copied from
leak_cpp_interior test):
VALGRIND_DO_LEAK_CHECK; // full list of leaked blocks
(void) VALGRIND_MONITOR_COMMAND("leak_check changed heuristics
multipleinheritance");
(void) VALGRIND_MONITOR_COMMAND("leak_check changed heuristics
multipleinheritance,newarray");
(void) VALGRIND_MONITOR_COMMAND("leak_check changed heuristics
multipleinheritance,newarray,stdstring");
Could the leaks list be changed to show all possible leaks and add the
information if an heuristic matched it?
If the performance is too bad, this could be optional.
The application also has other kinds of possible leaks that need to be
supressed.
One of these is the allocation by sqlite3malloc on 32bit platform.
sqlite3 always allocates 64bits before and stores there the length of
the remaining block in bytes.
On a 64bit platform this should be catched by the newarray heuristic I
think.
Second known possible leak that is no leak is the tls code of pthreads
(glibc/nptl).
It calls calloc and later stores a pointer to the second element of the
array.
This can be easily done by a supression entry.
Maybe valgrind needs some mechanism to specify the allocation function
and the allowed offset for a pointer.
Regards
Matthias
|
|
From: Philippe W. <phi...@sk...> - 2013-10-09 22:32:25
|
On Mon, 2013-10-07 at 21:22 +0200, Matthias Schwarzott wrote:
> Will it work to add some code after the shutdown of our application to
> call into the leaks checker like this to show full leak list and the see
> which blocks disappear by adding one heuristic at a time (copied from
> leak_cpp_interior test):
>
> VALGRIND_DO_LEAK_CHECK; // full list of leaked blocks
> (void) VALGRIND_MONITOR_COMMAND("leak_check changed heuristics
> multipleinheritance");
> (void) VALGRIND_MONITOR_COMMAND("leak_check changed heuristics
> multipleinheritance,newarray");
> (void) VALGRIND_MONITOR_COMMAND("leak_check changed heuristics
> multipleinheritance,newarray,stdstring");
>
> Could the leaks list be changed to show all possible leaks and add the
> information if an heuristic matched it?
> If the performance is too bad, this could be optional.
During the heuristic development, I thought to implement a specific
leak search mode to only report blocks that are reachable via
a selected set of heuristics (and ignore all the other blocks).
Not too difficult, but at the end I found the above commands good enough
to see what changes in a reachable block list when adding/removing
an heuristic.
>
>
> The application also has other kinds of possible leaks that need to be
> supressed.
>
> One of these is the allocation by sqlite3malloc on 32bit platform.
> sqlite3 always allocates 64bits before and stores there the length of
> the remaining block in bytes.
> On a 64bit platform this should be catched by the newarray heuristic I
> think.
>
> Second known possible leak that is no leak is the tls code of pthreads
> (glibc/nptl).
> It calls calloc and later stores a pointer to the second element of the
> array.
> This can be easily done by a supression entry.
Suppressions are for sure the easiest at least for a "single level"
of possibly lost. The advantage with heuristics is that they transform
a possible lost into a reachable, which means that it avoids to have
suppressions for all the indirectly reachable blocks (and related stack
traces).
>
> Maybe valgrind needs some mechanism to specify the allocation function
> and the allowed offset for a pointer.
What we should maybe have is a kind of "leak suppression" which
"transforms" a lost or possibly lost block into a reachable block
(see my other mail, where I suggested even an expression evaluator to
allow the user to define heuristics).
Philippe
|
|
From: Matthias S. <zz...@ge...> - 2013-10-09 04:53:36
|
On 02.10.2013 23:03, Philippe Waroquiers wrote: > Have you tested these heuristics with a (big) c++ application ? Philippe Hi Philippe, I did some real tests with our application. This is from one "smaller" test run: ==3917== LEAK SUMMARY: ==3917== definitely lost: 7,145,200 (+0) bytes in 2,117 (+0) blocks ==3917== indirectly lost: 39,872 (+0) bytes in 1,246 (+0) blocks ==3917== possibly lost: 1,140,720 (-558,683) bytes in 201 (-3,297) blocks ==3917== still reachable: 6,742,624 (+558,683) bytes in 44,916 (+3,297) blocks ==3917== of which reachable via heuristic: ==3917== stdstring : 5,527 (+5,527) bytes in 157 (+157) blocks ==3917== newarray : 308,552 (+308,552) bytes in 2,525 (+2,525) blocks ==3917== multipleinheritance: 7,928 (+7,928) bytes in 4 (+4) blocks ==3917== Array64Length : 244,584 (+244,584) bytes in 614 (+614) blocks ==3917== suppressed: 0 (+0) bytes in 0 (+0) blocks ==3917== To see details of leaked memory, give 'full' arg to leak_check ==3917== You see, we have possible leaks for all of your heuristics: multipleinheritance: 1 matching block, this is correct stdstring: lots of std::string, everything correct here newarray: No data allocated by new[] here, only memory from icu_50::UnicodeString::cloneArrayIfNeeded The problem with icu_50::UnicodeStringis, that is has the reference count in the word before the location pointed at. And that is 1 at the allocation time and later in most cases only a small integer. As long as it is 1, 2 or 4 it should always match the allocated size. In other cases it might match. Then I have the heuristic "array64length" I have written. This one checks if the pointer has offset 8 and the 64bits before match the remaining length (a subset of newarray on a 64bit platform). This one only matches memory allocated by sqlite3 (e.g. sqlite3MemMalloc). The remaining possible leaks can be grouped like this: - icu_50::UnicodeString (when reference count does not match length) - Some internal Pool implementations that maybe should be instrumented - sqlite3MemMalloc (why is this not matching Array64length ??)- pthreads TLS: 38x (one per thread) ==2503== 144 bytes in 1 blocks are possibly lost in loss record 18,895 of 20,196 ==2503== at 0x4006256: calloc (vg_replace_malloc.c:618) ==2503== by 0x4D8477B8: allocate_dtv (dl-tls.c:297) ==2503== by 0x4D847F5E: _dl_allocate_tls (dl-tls.c:461) ==2503== by 0x4DA1A6A0: pthread_create@@GLIBC_2.1 (allocatestack.c:572) […] So multipleinheritance and std::string have no false positives for me. Newarray is not as reliable, as each memory block that starts with a word of 1,2 or 4 will match (if the pointer has offset 4). Maybe some kind of matching is needed for combinations of pointer offset and allocating callstack. Regards Matthias |
|
From: Philippe W. <phi...@sk...> - 2013-10-09 22:26:51
|
On Wed, 2013-10-09 at 06:53 +0200, Matthias Schwarzott wrote: > On 02.10.2013 23:03, Philippe Waroquiers wrote: > > Have you tested these heuristics with a (big) c++ application ? Philippe > > Hi Philippe, > > I did some real tests with our application. Thanks for the below feedback, very interesting. > multipleinheritance: 1 matching block, this is correct > stdstring: lots of std::string, everything correct here > newarray: No data allocated by new[] here, only memory from > icu_50::UnicodeString::cloneArrayIfNeeded > > The problem with icu_50::UnicodeStringis, that is has the reference > count in the word before the location pointed at. > And that is 1 at the allocation time and later in most cases only a > small integer. As long as it is 1, 2 or 4 it should always match the > allocated size. In other cases it might match. Is the pointer to a unicodestring pointing one word after the ref count ? Then the heuristic is maybe not designed for that, but it properly detects that the unicode string is effectively reachable (and not possibly lost). I quickly looked at the unicode library. It looks like the allocation function can be redefined to be directly on top of malloc. So, it is not possible to add the condition of seeing "new[]" in the stack trace, as this "new[]" is not necessarily the function called to allocate the memory of a c++ array (IIUC : my c++ knowledge is very close to 0). > > Then I have the heuristic "array64length" I have written. This one > checks if the pointer has offset 8 and the 64bits before match the > remaining length (a subset of newarray on a 64bit platform). > This one only matches memory allocated by sqlite3 (e.g. sqlite3MemMalloc). > > The remaining possible leaks can be grouped like this: > - icu_50::UnicodeString (when reference count does not match length) > - Some internal Pool implementations that maybe should be instrumented > - sqlite3MemMalloc (why is this not matching Array64length ??)- pthreads > TLS: 38x (one per thread) > ==2503== 144 bytes in 1 blocks are possibly lost in loss record 18,895 > of 20,196 > ==2503== at 0x4006256: calloc (vg_replace_malloc.c:618) > ==2503== by 0x4D8477B8: allocate_dtv (dl-tls.c:297) > ==2503== by 0x4D847F5E: _dl_allocate_tls (dl-tls.c:461) > ==2503== by 0x4DA1A6A0: pthread_create@@GLIBC_2.1 (allocatestack.c:572) > […] > > So multipleinheritance and std::string have no false positives for me. > Newarray is not as reliable, as each memory block that starts with a > word of 1,2 or 4 will match (if the pointer has offset 4). > Maybe some kind of matching is needed for combinations of pointer offset > and allocating callstack. Yes, we could add a special kind of suppression that would suppress a block to be found via an heuristic if the alloc stack trace matches the "heuristic suppression". This is probably not very difficult to implement but as usual need to look at the balance between additional complexity (and cpu implied) and the provided functionality. A fully "user configurable" heuristic would be to allow the user to specify an expression that should return true. This will then allow very flexible heuristic to be done outside of memcheck. We need a (relatively fast) expression reader and evaluator in memcheck then. Philippe |