From: Philippe Waroquiers Date: Sat, 27 Oct 2018 18:28:59 +0000 (+0200) Subject: Fix 399301 - Use inlined frames in Massif XTree output. X-Git-Tag: VALGRIND_3_15_0~168 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=66b5a4e9c4d7de79c9396cf5ed117a4e5cfcf386;p=thirdparty%2Fvalgrind.git Fix 399301 - Use inlined frames in Massif XTree output. Author: Nicholas Nethercote Use inlined frames in Massif XTree output. This makes Massif's output much easier to follow. The commit also removes a -1 used on all Massif stack frame addresses. There was a big comment questioning the presence of that -1, and with it gone the addresses now match those produced by DHAT. --- diff --git a/.gitignore b/.gitignore index 7ae8ec4ba3..a427659459 100644 --- a/.gitignore +++ b/.gitignore @@ -762,6 +762,7 @@ /massif/tests/ignored /massif/tests/ignoring /massif/tests/insig +/massif/tests/inlinfomalloc /massif/tests/long-names /massif/tests/long-time /massif/tests/malloc_usable diff --git a/NEWS b/NEWS index 1eb1055dea..987d92857e 100644 --- a/NEWS +++ b/NEWS @@ -12,6 +12,8 @@ support for X86/macOS 10.13, AMD64/macOS 10.13. * ==================== CORE CHANGES =================== +* The XTree Massif output format now makes use of the information obtained + when specifying --read-inline-info=yes. * ================== PLATFORM CHANGES ================= @@ -20,9 +22,13 @@ support for X86/macOS 10.13, AMD64/macOS 10.13. * Callgrind: - - callgrind_annotate now inserts commas in call counts, and + - callgrind_annotate now inserts commas in call counts, and sort the caller/callee lists in the call tree. +* Massif: + - The default value for --read-inline-info is now "yes" on + Linux/Android/Solaris. It is still "no" on other OS. + * ==================== OTHER CHANGES ==================== @@ -39,6 +45,7 @@ To see details of a given bug, visit https://bugs.kde.org/show_bug.cgi?id=XXXXXX where XXXXXX is the bug number as listed below. +399301 Use inlined frames in Massif XTree output. 399322 Improve callgrind_annotate output Release 3.14.0 (9 October 2018) diff --git a/coregrind/m_main.c b/coregrind/m_main.c index bf4a712846..00702fc225 100644 --- a/coregrind/m_main.c +++ b/coregrind/m_main.c @@ -176,9 +176,10 @@ static void usage_NORETURN ( Bool debug_help ) " code found in stacks, for all code, or for all\n" " code except that from file-backed mappings\n" " --read-inline-info=yes|no read debug info about inlined function calls\n" -" and use it to do better stack traces. [yes]\n" -" on Linux/Android/Solaris for Memcheck/Helgrind/DRD\n" -" only. [no] for all other tools and platforms.\n" +" and use it to do better stack traces.\n" +" [yes] on Linux/Android/Solaris for the tools\n" +" Memcheck/Massif/Helgrind/DRD only.\n" +" [no] for all other tools and platforms.\n" " --read-var-info=yes|no read debug info on stack and global variables\n" " and use it to print better error messages in\n" " tools that make use of it (Memcheck, Helgrind,\n" @@ -1426,12 +1427,15 @@ Int valgrind_main ( Int argc, HChar **argv, HChar **envp ) early_process_cmd_line_options(&need_help); // BEGIN HACK + // When changing the logic for the VG_(clo_read_inline_info) default, + // the manual and --help output have to be changed accordingly. vg_assert(VG_(clo_toolname) != NULL); vg_assert(VG_(clo_read_inline_info) == False); # if !defined(VGO_darwin) if (0 == VG_(strcmp)(VG_(clo_toolname), "memcheck") || 0 == VG_(strcmp)(VG_(clo_toolname), "helgrind") || 0 == VG_(strcmp)(VG_(clo_toolname), "drd") + || 0 == VG_(strcmp)(VG_(clo_toolname), "massif") || 0 == VG_(strcmp)(VG_(clo_toolname), "exp-dhat")) { /* Change the default setting. Later on (just below) main_process_cmd_line_options should pick up any diff --git a/coregrind/m_xtree.c b/coregrind/m_xtree.c index 217cad9da5..7dbba7129f 100644 --- a/coregrind/m_xtree.c +++ b/coregrind/m_xtree.c @@ -744,8 +744,14 @@ static void ms_make_groups (UInt depth, Ms_Ec* ms_ec, UInt n_ec, SizeT sig_sz, VG_(ssort)(*groups, *n_groups, sizeof(Ms_Group), ms_group_revcmp_total); } -static void ms_output_group (VgFile* fp, UInt depth, Ms_Group* group, - SizeT sig_sz, double sig_pct_threshold) +/* Output the given group (located in an xtree at the given depth). + indent tells by how much to indent the information output for the group. + indent can be bigger than depth when outputting a group that is made + of one or more inlined calls: all inlined calls are output with the + same depth but with one more indent for each inlined call. */ +static void ms_output_group (VgFile* fp, UInt depth, UInt indent, + Ms_Group* group, SizeT sig_sz, + double sig_pct_threshold) { UInt i; Ms_Group* groups; @@ -756,7 +762,7 @@ static void ms_output_group (VgFile* fp, UInt depth, Ms_Group* group, const HChar* s = ( 1 == group->n_ec? "," : "s, all" ); vg_assert(group->group_ip == 0); FP("%*sn0: %lu in %d place%s below massif's threshold (%.2f%%)\n", - depth+1, "", group->total, group->n_ec, s, sig_pct_threshold); + indent+1, "", group->total, group->n_ec, s, sig_pct_threshold); return; } @@ -777,21 +783,33 @@ static void ms_output_group (VgFile* fp, UInt depth, Ms_Group* group, // Fix not trivial to do, so for the moment, --keep-debuginfo=yes will // have no impact on xtree massif output. - FP("%*s" "n%u: %ld %s\n", - depth + 1, "", - n_groups, - group->total, - VG_(describe_IP)(cur_ep, group->ms_ec->ips[depth] - 1, NULL)); - /* XTREE??? Massif original code removes 1 to get the IP description. I am - wondering if this is not something that predates revision r8818, - which introduced a -1 in the stack unwind (see m_stacktrace.c) - Kept for the moment to allow exact comparison with massif output, but - probably we should remove this, as we very probably end up 2 bytes before - the RA Return Address. */ + Addr cur_ip = group->ms_ec->ips[depth]; + + InlIPCursor *iipc = VG_(new_IIPC)(cur_ep, cur_ip); + + while (True) { + const HChar* buf = VG_(describe_IP)(cur_ep, cur_ip, iipc); + Bool is_inlined = VG_(next_IIPC)(iipc); + + FP("%*s" "n%u: %ld %s\n", + indent + 1, "", + is_inlined ? 1 : n_groups, // Inlined frames always have one child. + group->total, + buf); + + if (!is_inlined) { + break; + } + + indent++; + } + + VG_(delete_IIPC)(iipc); /* Output sub groups of this group. */ for (i = 0; i < n_groups; i++) - ms_output_group(fp, depth+1, &groups[i], sig_sz, sig_pct_threshold); + ms_output_group(fp, depth+1, indent+1, &groups[i], sig_sz, + sig_pct_threshold); VG_(free)(groups); } @@ -963,7 +981,7 @@ void VG_(XT_massif_print) /* Output depth 0 groups. */ DMSG(1, "XT_massif_print outputing %u depth 0 groups\n", n_groups); for (i = 0; i < n_groups; i++) - ms_output_group(fp, 0, &groups[i], sig_sz, header->sig_threshold); + ms_output_group(fp, 0, 0, &groups[i], sig_sz, header->sig_threshold); VG_(free)(groups); VG_(free)(ms_ec); diff --git a/docs/xml/manual-core.xml b/docs/xml/manual-core.xml index 0e787d75bf..e3c2a83afc 100644 --- a/docs/xml/manual-core.xml +++ b/docs/xml/manual-core.xml @@ -1899,10 +1899,10 @@ need to use them. function calls from DWARF3 debug info. This slows Valgrind startup and makes it use more memory (typically for each inlined piece of code, 6 words and space for the function name), but it - results in more descriptive stacktraces. For the 3.10.0 - release, this functionality is enabled by default only for Linux, - Android and Solaris targets and only for the tools Memcheck, Helgrind - and DRD. Here is an example of some stacktraces with + results in more descriptive stacktraces. Currently, + this functionality is enabled by default only for Linux, + Android and Solaris targets and only for the tools Memcheck, Massif, + Helgrind and DRD. Here is an example of some stacktraces with : KCachegrind, which is a KDE/Qt based GUI that makes it easy to navigate the large amount of data that an xtree can contain. + Note that xtree Callgrind Format does not make use of the inline + information even when specifying . + @@ -3005,6 +3008,9 @@ the "Massif Format". a massif output file. See and for more details about visualising Massif Format output files. + Note that xtree Massif Format makes use of the inline + information when specifying . + diff --git a/massif/tests/Makefile.am b/massif/tests/Makefile.am index 29cc9d359a..98c1252c29 100644 --- a/massif/tests/Makefile.am +++ b/massif/tests/Makefile.am @@ -20,6 +20,7 @@ EXTRA_DIST = \ custom_alloc.post.exp custom_alloc.stderr.exp custom_alloc.vgtest \ ignored.post.exp ignored.stderr.exp ignored.vgtest \ ignoring.post.exp ignoring.stderr.exp ignoring.vgtest \ + inlinfomalloc.post.exp inlinfomalloc.stderr.exp inlinfomalloc.vgtest \ insig.post.exp insig.stderr.exp insig.vgtest \ long-names.post.exp long-names.stderr.exp long-names.vgtest \ long-time.post.exp long-time.stderr.exp long-time.vgtest \ @@ -60,6 +61,7 @@ check_PROGRAMS = \ deep \ ignored \ ignoring \ + inlinfomalloc \ insig \ long-names \ long-time \ @@ -75,6 +77,8 @@ check_PROGRAMS = \ thresholds \ zero +inlinfomalloc_CFLAGS = $(AM_CFLAGS) -w + AM_CFLAGS += $(AM_FLAG_M3264_PRI) AM_CXXFLAGS += $(AM_FLAG_M3264_PRI) diff --git a/massif/tests/inlinfomalloc.c b/massif/tests/inlinfomalloc.c new file mode 100644 index 0000000000..1757636a1e --- /dev/null +++ b/massif/tests/inlinfomalloc.c @@ -0,0 +1,79 @@ +#include +#include + +#define INLINE inline __attribute__((always_inline)) + +int alloc (int n) { + (void) malloc (n); + return n; +} + +INLINE int fun_d(int argd) { + static int locd = 0; + if (argd > 0) + locd += argd; + return alloc (locd); +} + +INLINE int fun_c(int argc) { + static int locc = 0; + locc += argc; + return fun_d(locc); +} + +INLINE int fun_b(int argb) { + static int locb = 0; + locb += argb; + return fun_c(locb); +} + +INLINE int fun_a(int arga) { + static int loca = 0; + loca += arga; + return fun_b(loca); +} + +__attribute__((noinline)) +static int fun_noninline_m(int argm) +{ + return fun_d(argm); +} + +__attribute__((noinline)) +static int fun_noninline_o(int argo) +{ + static int loco = 0; + if (argo > 0) + loco += argo; + return alloc (loco); +} + +INLINE int fun_f(int argf) { + static int locf = 0; + locf += argf; + return fun_noninline_o(locf); +} + +INLINE int fun_e(int arge) { + static int loce = 0; + loce += arge; + return fun_f(loce); +} + +__attribute__((noinline)) +static int fun_noninline_n(int argn) +{ + return fun_e(argn); +} + + +int main() { + int result = 100000; + result = fun_a(result); + result += fun_b(result); + result += fun_noninline_m(result); + result += fun_d(result); + result += fun_noninline_n(result); + return 0; +} + diff --git a/massif/tests/inlinfomalloc.post.exp b/massif/tests/inlinfomalloc.post.exp new file mode 100644 index 0000000000..a4da734384 --- /dev/null +++ b/massif/tests/inlinfomalloc.post.exp @@ -0,0 +1,69 @@ +-------------------------------------------------------------------------------- +Command: ./inlinfomalloc +Massif arguments: --stacks=no --heap-admin=0 --time-unit=B --threshold=0 --detailed-freq=6 --massif-out-file=massif.out --ignore-fn=__part_load_locale --ignore-fn=__time_load_locale --ignore-fn=dwarf2_unwind_dyld_add_image_hook --ignore-fn=get_or_create_key_element +ms_print arguments: --threshold=0 massif.out +-------------------------------------------------------------------------------- + + + MB +7.057^ @ + | @ + | @ + | @ + | @ + | @ + | @ + | @ + | @ + | @ + | :::::::::::::::::::::::::::::::::::@ + | : @ + | : @ + | : @ + | : @ + | : @ + | : @ + | :::::::::::::::::::::::: @ + | : : @ + | :::::::::: : @ + 0 +----------------------------------------------------------------------->MB + 0 7.057 + +Number of snapshots: 6 + Detailed snapshots: [5] + +-------------------------------------------------------------------------------- + n time(B) total(B) useful-heap(B) extra-heap(B) stacks(B) +-------------------------------------------------------------------------------- + 0 0 0 0 0 0 + 1 100,000 100,000 100,000 0 0 + 2 500,000 500,000 500,000 0 0 + 3 1,400,000 1,400,000 1,400,000 0 0 + 4 3,700,000 3,700,000 3,700,000 0 0 + 5 7,400,000 7,400,000 7,400,000 0 0 +100.00% (7,400,000B) (heap allocation functions) malloc/new/new[], --alloc-fns, etc. +->100.00% (7,400,000B) 0x........: alloc (inlinfomalloc.c:7) + ->50.00% (3,700,000B) 0x........: fun_noninline_o (inlinfomalloc.c:48) + | ->50.00% (3,700,000B) 0x........: fun_f (inlinfomalloc.c:54) + | ->50.00% (3,700,000B) 0x........: fun_e (inlinfomalloc.c:60) + | ->50.00% (3,700,000B) 0x........: fun_noninline_n (inlinfomalloc.c:66) + | ->50.00% (3,700,000B) 0x........: main (inlinfomalloc.c:76) + | + ->31.08% (2,300,000B) 0x........: fun_d (inlinfomalloc.c:15) + | ->31.08% (2,300,000B) 0x........: main (inlinfomalloc.c:75) + | + ->12.16% (900,000B) 0x........: fun_d (inlinfomalloc.c:15) + | ->12.16% (900,000B) 0x........: fun_noninline_m (inlinfomalloc.c:39) + | ->12.16% (900,000B) 0x........: main (inlinfomalloc.c:74) + | + ->05.41% (400,000B) 0x........: fun_d (inlinfomalloc.c:15) + | ->05.41% (400,000B) 0x........: fun_c (inlinfomalloc.c:21) + | ->05.41% (400,000B) 0x........: fun_b (inlinfomalloc.c:27) + | ->05.41% (400,000B) 0x........: main (inlinfomalloc.c:73) + | + ->01.35% (100,000B) 0x........: fun_d (inlinfomalloc.c:15) + ->01.35% (100,000B) 0x........: fun_c (inlinfomalloc.c:21) + ->01.35% (100,000B) 0x........: fun_b (inlinfomalloc.c:27) + ->01.35% (100,000B) 0x........: fun_a (inlinfomalloc.c:33) + ->01.35% (100,000B) 0x........: main (inlinfomalloc.c:72) + diff --git a/massif/tests/inlinfomalloc.stderr.exp b/massif/tests/inlinfomalloc.stderr.exp new file mode 100644 index 0000000000..e69de29bb2 diff --git a/massif/tests/inlinfomalloc.vgtest b/massif/tests/inlinfomalloc.vgtest new file mode 100644 index 0000000000..9021b926e5 --- /dev/null +++ b/massif/tests/inlinfomalloc.vgtest @@ -0,0 +1,6 @@ +prog: inlinfomalloc +vgopts: --stacks=no --heap-admin=0 --time-unit=B --threshold=0 --detailed-freq=6 --massif-out-file=massif.out +vgopts: --ignore-fn=__part_load_locale --ignore-fn=__time_load_locale --ignore-fn=dwarf2_unwind_dyld_add_image_hook --ignore-fn=get_or_create_key_element +stderr_filter: filter_verbose +post: perl ../../massif/ms_print --threshold=0 massif.out | ../../tests/filter_addresses +cleanup: rm massif.out diff --git a/none/tests/cmdline1.stdout.exp b/none/tests/cmdline1.stdout.exp index bc2eb29bcc..0a7b4fc6c8 100644 --- a/none/tests/cmdline1.stdout.exp +++ b/none/tests/cmdline1.stdout.exp @@ -90,9 +90,10 @@ usage: valgrind [options] prog-and-args code found in stacks, for all code, or for all code except that from file-backed mappings --read-inline-info=yes|no read debug info about inlined function calls - and use it to do better stack traces. [yes] - on Linux/Android/Solaris for Memcheck/Helgrind/DRD - only. [no] for all other tools and platforms. + and use it to do better stack traces. + [yes] on Linux/Android/Solaris for the tools + Memcheck/Massif/Helgrind/DRD only. + [no] for all other tools and platforms. --read-var-info=yes|no read debug info on stack and global variables and use it to print better error messages in tools that make use of it (Memcheck, Helgrind, diff --git a/none/tests/cmdline2.stdout.exp b/none/tests/cmdline2.stdout.exp index fb42abb49a..d312734c43 100644 --- a/none/tests/cmdline2.stdout.exp +++ b/none/tests/cmdline2.stdout.exp @@ -90,9 +90,10 @@ usage: valgrind [options] prog-and-args code found in stacks, for all code, or for all code except that from file-backed mappings --read-inline-info=yes|no read debug info about inlined function calls - and use it to do better stack traces. [yes] - on Linux/Android/Solaris for Memcheck/Helgrind/DRD - only. [no] for all other tools and platforms. + and use it to do better stack traces. + [yes] on Linux/Android/Solaris for the tools + Memcheck/Massif/Helgrind/DRD only. + [no] for all other tools and platforms. --read-var-info=yes|no read debug info on stack and global variables and use it to print better error messages in tools that make use of it (Memcheck, Helgrind,