Author: njn
Date: 2007-12-04 03:15:23 +0000 (Tue, 04 Dec 2007)
New Revision: 7270
Log:
Two changes:
- Be more robust in the face of malformed stack traces. This avoids some
potential assertion errors (which have affected prior versions of Massif),
but unfortunately reduces the amount of sanity-checking that can be done
on XTrees.
- Get white-space printing right in output file. Non-functional change,
just makes output files easier to read.
Modified:
trunk/massif/ms_main.c
Modified: trunk/massif/ms_main.c
===================================================================
--- trunk/massif/ms_main.c 2007-12-03 21:29:22 UTC (rev 7269)
+++ trunk/massif/ms_main.c 2007-12-04 03:15:23 UTC (rev 7270)
@@ -465,6 +465,9 @@
// | v v |
// child1 child2
//
+// (Note that malformed stack traces can lead to difficulties. See the
+// comment at the bottom of get_XCon.)
+//
// XTrees and XPts are mirrored by SXTrees and SXPts, where the 'S' is short
// for "saved". When the XTree is duplicated for a snapshot, we duplicate
// it as an SXTree, which is similar but omits some things it does not need,
@@ -612,7 +615,7 @@
static SXPt* dup_XTree(XPt* xpt, SizeT total_szB)
{
Int i, n_sig_children, n_insig_children, n_child_sxpts;
- SizeT insig_children_szB, sig_child_threshold_szB;
+ SizeT sig_child_threshold_szB;
SXPt* sxpt;
// Number of XPt children Action for SXPT
@@ -657,21 +660,23 @@
// Create the SXPt's children.
if (n_child_sxpts > 0) {
Int j;
- SizeT sig_children_szB = 0;
+ SizeT sig_children_szB = 0, insig_children_szB = 0;
sxpt->Sig.children = VG_(malloc)(n_child_sxpts * sizeof(SXPt*));
- // Duplicate the significant children.
+ // Duplicate the significant children. (Nb: sig_children_szB +
+ // insig_children_szB doesn't necessarily equal xpt->szB.)
j = 0;
for (i = 0; i < xpt->n_children; i++) {
if (xpt->children[i]->szB >= sig_child_threshold_szB) {
sxpt->Sig.children[j++] = dup_XTree(xpt->children[i], total_szB);
- sig_children_szB += xpt->children[i]->szB;
+ sig_children_szB += xpt->children[i]->szB;
+ } else {
+ insig_children_szB += xpt->children[i]->szB;
}
}
// Create the SXPt for the insignificant children, if any, and put it
// in the last child entry.
- insig_children_szB = sxpt->szB - sig_children_szB;
if (n_insig_children > 0) {
// Nb: We 'n_sxpt_allocs' here because creating an Insig SXPt
// doesn't involve a call to dup_XTree().
@@ -719,8 +724,6 @@
// ms_expensive_sanity_check.
static void sanity_check_XTree(XPt* xpt, XPt* parent)
{
- Int i;
-
tl_assert(xpt != NULL);
// Check back-pointer.
@@ -730,16 +733,8 @@
// Check children counts look sane.
tl_assert(xpt->n_children <= xpt->max_children);
- // Check the sum of any children szBs equals the XPt's szB. Check the
- // children at the same time.
- if (xpt->n_children > 0) {
- SizeT children_sum_szB = 0;
- for (i = 0; i < xpt->n_children; i++) {
- sanity_check_XTree(xpt->children[i], xpt);
- children_sum_szB += xpt->children[i]->szB;
- }
- tl_assert(children_sum_szB == xpt->szB);
- }
+ // Unfortunately, xpt's size is not necessarily equal to the sum of xpt's
+ // children's sizes. See comment at the bottom of get_XCon.
}
// Sanity checking: we check SXTrees (which are in snapshots) after
@@ -756,12 +751,9 @@
switch (sxpt->tag) {
case SigSXPt: {
if (sxpt->Sig.n_children > 0) {
- SizeT children_sum_szB = 0;
for (i = 0; i < sxpt->Sig.n_children; i++) {
sanity_check_SXTree(sxpt->Sig.children[i]);
- children_sum_szB += sxpt->Sig.children[i]->szB;
}
- tl_assert(children_sum_szB == sxpt->szB);
}
break;
}
@@ -893,7 +885,45 @@
}
}
}
- tl_assert(0 == xpt->n_children); // Must be bottom-XPt
+
+ // [Note: several comments refer to this comment. Do not delete it
+ // without updating them.]
+ //
+ // A complication... If all stack traces were well-formed, then the
+ // returned xpt would always be a bottom-XPt. As a consequence, an XPt's
+ // size would always be equal to the sum of its children's sizes, which
+ // is an excellent sanity check.
+ //
+ // Unfortunately, stack traces occasionally are malformed, ie. truncated.
+ // This allows a stack trace to be a sub-trace of another, eg. a/b/c is a
+ // sub-trace of a/b/c/d. So we can't assume this xpt is a bottom-XPt;
+ // nor can we do sanity check an XPt's size against its children's sizes.
+ // This is annoying, but must be dealt with. (Older versions of Massif
+ // had this assertion in, and it was reported to fail by real users a
+ // couple of times.) Even more annoyingly, I can't come up with a simple
+ // test case that exhibit such a malformed stack trace, so I can't
+ // regression test it. Sigh.
+ //
+ // However, we can print a warning, so that if it happens (unexpectedly)
+ // in existing regression tests we'll know. Also, it warns users that
+ // the output snapshots may not add up the way they might expect.
+ //
+ //tl_assert(0 == xpt->n_children); // Must be bottom-XPt
+ if (0 != xpt->n_children) {
+ static Int n_moans = 0;
+ if (n_moans < 3) {
+ VG_(message)(Vg_UserMsg,
+ "Warning: Malformed stack trace detected. In Massif's output,");
+ VG_(message)(Vg_UserMsg,
+ "Warning: the size of an entry's child entries may not sum up");
+ VG_(message)(Vg_UserMsg,
+ "Warning: to the entry's size as they normally do.");
+ n_moans++;
+ if (3 == n_moans)
+ VG_(message)(Vg_UserMsg,
+ "Warning: (And Massif now won't warn about this again.)");
+ }
+ }
return xpt;
}
@@ -902,7 +932,6 @@
{
tl_assert(True == clo_heap);
tl_assert(NULL != xpt);
- tl_assert(0 == xpt->n_children); // must be bottom-XPt
if (0 == space_delta)
return;
@@ -1925,11 +1954,12 @@
// Ok, print the child.
pp_snapshot_SXPt(fd, child, depth+1, depth_str, depth_str_len,
snapshot_heap_szB, snapshot_total_szB);
+ }
- // Unindent.
- depth_str[depth+0] = '\0';
- depth_str[depth+1] = '\0';
- }
+ // Unindent.
+ depth_str[depth+0] = '\0';
+ depth_str[depth+1] = '\0';
+
// There should be 0 or 1 Insig children SXPts.
tl_assert(n_insig_children_sxpts <= 1);
break;
-------------------------------------------------------------------------
SF.Net email is sponsored by: The Future of Linux Business White Paper
from Novell. From the desktop to the data center, Linux is going
mainstream. Let it simplify your IT future.
http://altfarm.mediaplex.com/ad/ck/8857-50307-18918-4
_______________________________________________
Valgrind-developers mailing list
Valgrind-developers@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/valgrind-developers
