From 645de069956aa667b924026d2125433461766e4d Mon Sep 17 00:00:00 2001 From: Paul Floyd Date: Sat, 6 May 2023 07:45:47 +0200 Subject: [PATCH] Bug 469146 - massif --ignore-fn does not ignore inlined functions --- .gitignore | 1 + NEWS | 2 +- coregrind/m_debuginfo/debuginfo.c | 26 ++++++++++++++++++++ include/pub_tool_debuginfo.h | 4 ++- massif/docs/ms-manual.xml | 11 +++++++++ massif/ms_main.c | 31 +++++++++++++++++++---- massif/tests/Makefile.am | 4 +++ massif/tests/bug469146.cpp | 41 +++++++++++++++++++++++++++++++ massif/tests/bug469146.post.exp | 38 ++++++++++++++++++++++++++++ massif/tests/bug469146.stderr.exp | 2 ++ massif/tests/bug469146.vgtest | 8 ++++++ 11 files changed, 161 insertions(+), 7 deletions(-) create mode 100644 massif/tests/bug469146.cpp create mode 100644 massif/tests/bug469146.post.exp create mode 100644 massif/tests/bug469146.stderr.exp create mode 100644 massif/tests/bug469146.vgtest diff --git a/.gitignore b/.gitignore index 6155f8f355..076e168ded 100644 --- a/.gitignore +++ b/.gitignore @@ -772,6 +772,7 @@ /massif/tests/basic /massif/tests/basic_malloc /massif/tests/big-alloc +/massif/tests/bug469146 /massif/tests/culling1 /massif/tests/culling2 /massif/tests/custom_alloc diff --git a/NEWS b/NEWS index e269d128b9..f44cfde1dc 100644 --- a/NEWS +++ b/NEWS @@ -26,7 +26,7 @@ bugzilla (https://bugs.kde.org/enter_bug.cgi?product=valgrind) rather than mailing the developers (or mailing lists) directly -- bugs that are not entered into bugzilla tend to get forgotten about or ignored. - +469146 massif --ignore-fn does not ignore inlined functions To see details of a given bug, visit https://bugs.kde.org/show_bug.cgi?id=XXXXXX diff --git a/coregrind/m_debuginfo/debuginfo.c b/coregrind/m_debuginfo/debuginfo.c index 2d2accc999..22b41def21 100644 --- a/coregrind/m_debuginfo/debuginfo.c +++ b/coregrind/m_debuginfo/debuginfo.c @@ -2289,6 +2289,32 @@ Bool VG_(get_fnname) ( DiEpoch ep, Addr a, const HChar** buf ) /*offsetP*/NULL ); } + +Bool VG_(get_fnname_inl) ( DiEpoch ep, Addr a, const HChar** buf, + const InlIPCursor* iipc ) +{ + if (iipc) { + vg_assert(is_DI_valid_for_epoch(iipc->di, ep)); + } + + if (is_bottom(iipc)) { + return get_sym_name ( /*C++-demangle*/True, /*Z-demangle*/True, + /*below-main-renaming*/True, + ep, a, buf, + /*match_anywhere_in_fun*/True, + /*show offset?*/False, + /*text sym*/True, + /*offsetP*/NULL ); + } else { + const DiInlLoc *next_inl = iipc && iipc->next_inltab >= 0 + ? & iipc->di->inltab[iipc->next_inltab] + : NULL; + vg_assert (next_inl); + *buf = next_inl->inlinedfn; + return True; + } +} + /* This is available to tools... always demangle C++ names, match anywhere in function, and show offset if nonzero. NOTE: See IMPORTANT COMMENT above about persistence and ownership diff --git a/include/pub_tool_debuginfo.h b/include/pub_tool_debuginfo.h index 078c562b8e..7631ff67a6 100644 --- a/include/pub_tool_debuginfo.h +++ b/include/pub_tool_debuginfo.h @@ -210,7 +210,9 @@ extern Bool VG_(next_IIPC)(InlIPCursor *iipc); /* Free all memory associated with iipc. */ extern void VG_(delete_IIPC)(InlIPCursor *iipc); - +/* Similar to VG_(get_fnname) but uses InlIPCursor and handles inline functions */ +extern Bool VG_(get_fnname_inl) ( DiEpoch ep, Addr a, const HChar** fnname, + const InlIPCursor* iipc ); /* Get an XArray of StackBlock which describe the stack (auto) blocks for this ip. The caller is expected to free the XArray at some diff --git a/massif/docs/ms-manual.xml b/massif/docs/ms-manual.xml index b589071556..3dff259952 100644 --- a/massif/docs/ms-manual.xml +++ b/massif/docs/ms-manual.xml @@ -760,6 +760,17 @@ various places online. + Arguments of type size_t need to be replaced + with unsigned long on 64bit platforms and unsigned + on 32bit platforms. + + + will work with inline functions. + Inline function names are not mangled, which means that you only need + to provide the function name and not the argument list. + + + does not support wildcards. diff --git a/massif/ms_main.c b/massif/ms_main.c index f3500c367d..1040ad53f4 100644 --- a/massif/ms_main.c +++ b/massif/ms_main.c @@ -506,6 +506,8 @@ void filter_IPs (Addr* ips, Int n_ips, { Int i; Bool top_has_fnname = False; + Bool is_alloc_fn = False; + Bool is_inline_fn = False; const HChar *fnname; *top = 0; @@ -519,9 +521,21 @@ void filter_IPs (Addr* ips, Int n_ips, // 0x1 0x2 0x3 alloc func1 main // became 0x1 0x2 0x3 func1 main const DiEpoch ep = VG_(current_DiEpoch)(); - for (i = *top; i < n_ips; i++) { - top_has_fnname = VG_(get_fnname)(ep, ips[*top], &fnname); - if (top_has_fnname && VG_(strIsMemberXA)(alloc_fns, fnname)) { + InlIPCursor *iipc = NULL; + + for (i = *top; i < n_ips; ++i) { + iipc = VG_(new_IIPC)(ep, ips[i]); + do { + top_has_fnname = VG_(get_fnname_inl)(ep, ips[i], &fnname, iipc); + is_alloc_fn = top_has_fnname && VG_(strIsMemberXA)(alloc_fns, fnname); + is_inline_fn = VG_(next_IIPC)(iipc); + if (is_alloc_fn && is_inline_fn) { + VERB(4, "filtering inline alloc fn %s\n", fnname); + } + } while (is_alloc_fn && is_inline_fn); + VG_(delete_IIPC)(iipc); + + if (is_alloc_fn) { VERB(4, "filtering alloc fn %s\n", fnname); (*top)++; (*n_ips_sel)--; @@ -534,8 +548,15 @@ void filter_IPs (Addr* ips, Int n_ips, if (*n_ips_sel > 0 && VG_(sizeXA)(ignore_fns) > 0) { if (!top_has_fnname) { // top has no fnname => search for the first entry that has a fnname - for (i = *top; i < n_ips && !top_has_fnname; i++) { - top_has_fnname = VG_(get_fnname)(ep, ips[i], &fnname); + for (i = *top; i < n_ips && !top_has_fnname; ++i) { + iipc = VG_(new_IIPC)(ep, ips[i]); + do { + top_has_fnname = VG_(get_fnname_inl)(ep, ips[i], &fnname, iipc); + if (top_has_fnname) { + break; + } + } while (VG_(next_IIPC)(iipc)); + VG_(delete_IIPC)(iipc); } } if (top_has_fnname && VG_(strIsMemberXA)(ignore_fns, fnname)) { diff --git a/massif/tests/Makefile.am b/massif/tests/Makefile.am index 84c9b1273a..2f3a84ecc7 100644 --- a/massif/tests/Makefile.am +++ b/massif/tests/Makefile.am @@ -11,6 +11,7 @@ EXTRA_DIST = \ big-alloc.post.exp big-alloc.post.exp-64bit big-alloc.post.exp-ppc64 \ big-alloc.stderr.exp big-alloc.vgtest \ big-alloc.post.exp-x86-freebsd \ + bug469146.post.exp bug469146.stderr.exp bug469146.vgtest \ deep-A.post.exp deep-A.stderr.exp deep-A.vgtest \ deep-B.post.exp deep-B.stderr.exp deep-B.vgtest \ deep-C.post.exp deep-C.stderr.exp deep-C.vgtest \ @@ -59,6 +60,7 @@ check_PROGRAMS = \ alloc-fns \ basic \ big-alloc \ + bug469146 \ culling1 culling2 \ custom_alloc \ deep \ @@ -86,6 +88,8 @@ AM_CFLAGS += $(AM_FLAG_M3264_PRI) AM_CXXFLAGS += $(AM_FLAG_M3264_PRI) # C++ tests +bug469146_SOURCES = bug469146.cpp +bug469146_CXXFLAGS = $(AM_CFLAGS) -O2 new_cpp_SOURCES = new-cpp.cpp overloaded_new_SOURCES = overloaded-new.cpp # pre C++11 compilers don't have exception specs diff --git a/massif/tests/bug469146.cpp b/massif/tests/bug469146.cpp new file mode 100644 index 0000000000..268f07c0a8 --- /dev/null +++ b/massif/tests/bug469146.cpp @@ -0,0 +1,41 @@ +#include + +// this is inline so it filters as "filter_function1" +static inline int* filter_function1(std::size_t size) +{ + return new int[size]; +} + +// this is out of line C++ +// int is deliberately used here instead of size_t +// so that it is 32/b4 bit portable +// this filters as "filter_function2(int)" +int* __attribute__((optnone)) __attribute__((noinline)) filter_function2(int size) +{ + return new int[static_cast(size)]; +} + +// finally extern "C" +// this filters as "filter_function3" +extern "C" +int* __attribute__((optnone)) __attribute__((noinline)) filter_function3(std::size_t size) +{ + return new int[size]; +} + +size_t func() +{ + int * mem1 = filter_function1(1000U); + int * mem2 = filter_function2(1000); + int * mem3 = filter_function3(1000U); + delete [] mem1; + delete [] mem2; + delete [] mem3; + + return (size_t)mem1/2 + (size_t)mem2/2 + (size_t)mem3/2; +} + +int main() +{ + return func(); +} diff --git a/massif/tests/bug469146.post.exp b/massif/tests/bug469146.post.exp new file mode 100644 index 0000000000..1e9936cbe1 --- /dev/null +++ b/massif/tests/bug469146.post.exp @@ -0,0 +1,38 @@ +-------------------------------------------------------------------------------- +Command: ./bug469146 +Massif arguments: --stacks=no --time-unit=B --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 --ignore-fn=_GLOBAL__sub_I_eh_alloc.cc --ignore-fn=call_init.part.0 --ignore-fn=call_init --ignore-fn=filter_function1 --ignore-fn=filter_function2(int) --ignore-fn=filter_function3 +ms_print arguments: massif.out +-------------------------------------------------------------------------------- + + + B + 1^ + | + | + | + | + | + | + | + | + | + | + | + | + | + | + | + | + | + | + | + 0 +----------------------------------------------------------------------->B + 0 1 + +Number of snapshots: 1 + Detailed snapshots: [] + +-------------------------------------------------------------------------------- + n time(B) total(B) useful-heap(B) extra-heap(B) stacks(B) +-------------------------------------------------------------------------------- + 0 0 0 0 0 0 diff --git a/massif/tests/bug469146.stderr.exp b/massif/tests/bug469146.stderr.exp new file mode 100644 index 0000000000..139597f9cb --- /dev/null +++ b/massif/tests/bug469146.stderr.exp @@ -0,0 +1,2 @@ + + diff --git a/massif/tests/bug469146.vgtest b/massif/tests/bug469146.vgtest new file mode 100644 index 0000000000..68585a4514 --- /dev/null +++ b/massif/tests/bug469146.vgtest @@ -0,0 +1,8 @@ +prog: bug469146 +vgopts: --stacks=no --time-unit=B --massif-out-file=massif.out +vgopts: --ignore-fn=__part_load_locale --ignore-fn=__time_load_locale --ignore-fn=dwarf2_unwind_dyld_add_image_hook +vgopts: --ignore-fn=get_or_create_key_element --ignore-fn=_GLOBAL__sub_I_eh_alloc.cc --ignore-fn=call_init.part.0 +vgopts: --ignore-fn=call_init +vgopts: --ignore-fn=filter_function1 --ignore-fn="filter_function2(int)" --ignore-fn=filter_function3 +post: perl ../../massif/ms_print massif.out | sed 's/gcc[0-9]*/gcc/' | ../../tests/filter_addresses +cleanup: rm massif.out -- 2.47.2