The Dyninst destructors miss cleaning up some allocated memory

[wcohen]

Intention I am using dyninst to analyze binaries for systemtap. Once done with the analysis phase would like to clean up the memory used because it can be quite substantial for larger binaries such as the Linux kernel. The current work on this is at https://sourceware.org/git/?p=systemtap.git;a=shortlog;h=refs/heads/wcohen/memory . I expect the destructors to release memory when they are called., but I noticed the memory lost on the heap when reviewing the results of valgrind memcheck on the stap binary.

Describe the bug

The systemtap analysis code caches the dyninst SymtabCodeSource and CodeObject generated as the analysis may need to be done multiple times on a binary and it is very expensive to parse entire binaries multiple times. When finished with the anaysis the destructors are explicitly called for the SymtabCodeSource and CodeObject objects. However, it appears that some of dyninst data structures don't actually get cleaned up when destructors and SymtabAPI::Symtab::closeSymtab(i.second.symtab); run in flush_analysis_caches.

To Reproduce

On a Fedora 35 machine:

sudo dnf install valgrind
sudo dnf builddep systemtap
git clone git://sourceware.org/git/systemtap.git
git checkout wcohen/memory
./configure --without-python2-probes
make
sudo make install
sudo ./stap-prep
valgrind  --leak-check=full  ../install/bin/stap -g -p4 -v --example keyhack.stp >& leaks.log

Note at the end of leaks.log there are a couple large dyninst releated leaks:

==472747== 55,990,004 (1,986,816 direct, 54,003,188 indirect) bytes in 9,552 blocks are definitely lost in loss record 5,011 of 5,027
==472747==    at 0x4844FF5: operator new(unsigned long) (vg_replace_malloc.c:422)
==472747==    by 0x4E065B9: Dyninst::InsnAdapter::IA_IAPI::makePlatformIA_IAPI(Dyninst::Architecture, Dyninst::InstructionAPI::InstructionDecoder, unsigned long, Dyninst::ParseAPI::CodeObject*, Dyninst::ParseAPI::CodeRegion*, Dyninst::InstructionSource*, Dyninst::ParseAPI::Block*) (IA_IAPI.C:117)
==472747==    by 0x4DDAFFB: Dyninst::ParseAPI::Parser::parse_frame_one_iteration(Dyninst::ParseAPI::ParseFrame&, bool) (Parser.C:1722)
==472747==    by 0x4DE889C: UnknownInlinedFun (Parser.C:1378)
==472747==    by 0x4DE889C: UnknownInlinedFun (Parser.C:455)
==472747==    by 0x4DE889C: UnknownInlinedFun (Parser.C:632)
==472747==    by 0x4DE889C: Dyninst::ParseAPI::Parser::LaunchWork(LockFreeQueueItem<Dyninst::ParseAPI::ParseFrame*>*, bool) [clone ._omp_fn.0] [clone .lto_priv.0] (Parser.C:620)
==472747==    by 0x6067947: GOMP_task (task.c:442)
==472747==    by 0x4DD4711: Dyninst::ParseAPI::Parser::LaunchWork(LockFreeQueueItem<Dyninst::ParseAPI::ParseFrame*>*, bool) (Parser.C:619)
==472747==    by 0x6067947: GOMP_task (task.c:442)
==472747==    by 0x4DD4711: Dyninst::ParseAPI::Parser::LaunchWork(LockFreeQueueItem<Dyninst::ParseAPI::ParseFrame*>*, bool) (Parser.C:619)
==472747==    by 0x6063325: GOMP_parallel (parallel.c:178)
==472747==    by 0x4DD4D68: UnknownInlinedFun (Parser.C:656)
==472747==    by 0x4DD4D68: Dyninst::ParseAPI::Parser::parse_frames(LockFreeQueue<Dyninst::ParseAPI::ParseFrame*>&, bool) (Parser.C:668)
==472747==    by 0x4DD8063: UnknownInlinedFun (Parser.C:331)
==472747==    by 0x4DD8063: Dyninst::ParseAPI::Parser::parse() (Parser.C:180)
==472747==    by 0x4DEFE32: Dyninst::ParseAPI::CodeObject::parse() (CodeObject.C:186)
==472747== 
==472747== 139,535,467 (5,109,728 direct, 134,425,739 indirect) bytes in 24,566 blocks are definitely lost in loss record 5,020 of 5,027
==472747==    at 0x4844FF5: operator new(unsigned long) (vg_replace_malloc.c:422)
==472747==    by 0x4E065B9: Dyninst::InsnAdapter::IA_IAPI::makePlatformIA_IAPI(Dyninst::Architecture, Dyninst::InstructionAPI::InstructionDecoder, unsigned long, Dyninst::ParseAPI::CodeObject*, Dyninst::ParseAPI::CodeRegion*, Dyninst::InstructionSource*, Dyninst::ParseAPI::Block*) (IA_IAPI.C:117)
==472747==    by 0x4DDAFFB: Dyninst::ParseAPI::Parser::parse_frame_one_iteration(Dyninst::ParseAPI::ParseFrame&, bool) (Parser.C:1722)
==472747==    by 0x4DE889C: UnknownInlinedFun (Parser.C:1378)
==472747==    by 0x4DE889C: UnknownInlinedFun (Parser.C:455)
==472747==    by 0x4DE889C: UnknownInlinedFun (Parser.C:632)
==472747==    by 0x4DE889C: Dyninst::ParseAPI::Parser::LaunchWork(LockFreeQueueItem<Dyninst::ParseAPI::ParseFrame*>*, bool) [clone ._omp_fn.0] [clone .lto_priv.0] (Parser.C:620)
==472747==    by 0x60660DA: gomp_barrier_handle_tasks (task.c:1439)
==472747==    by 0x606E817: gomp_team_barrier_wait_end (bar.c:116)
==472747==    by 0x606BC81: gomp_thread_start (team.c:126)
==472747==    by 0x4990B19: start_thread (pthread_create.c:443)
==472747==    by 0x4A148E3: clone (clone.S:100)
==472747== 
==472747== 268,442,954 bytes in 951 blocks are possibly lost in loss record 5,025 of 5,027
==472747==    at 0x484486F: malloc (vg_replace_malloc.c:381)
==472747==    by 0x4B8B115: __libdw_allocate (libdw_alloc.c:123)
==472747==    by 0x4B85BF0: read_srclines (dwarf_getsrclines.c:1051)
==472747==    by 0x4B85F95: __libdw_getsrclines (dwarf_getsrclines.c:1155)
==472747==    by 0x4B8627D: dwarf_getsrclines (dwarf_getsrclines.c:1245)
==472747==    by 0x4B868D1: dwarf_getsrcfiles (dwarf_getsrcfiles.c:92)
==472747==    by 0x514DB9F: Dyninst::SymtabAPI::DwarfWalker::buildSrcFiles(Dwarf*, Dwarf_Die, boost::shared_ptr<Dyninst::SymtabAPI::StringTable>) [clone .constprop.0] [clone .isra.0] (dwarfWalker.C:310)
==472747==    by 0x50EBD83: UnknownInlinedFun (Object-elf.C:2412)
==472747==    by 0x50EBD83: Dyninst::SymtabAPI::Object::load_object(bool) (Object-elf.C:1585)
==472747==    by 0x51349FC: Dyninst::SymtabAPI::Object::Object(MappedFile*, bool, void (*)(char const*), bool, Dyninst::SymtabAPI::Symtab*) [clone .constprop.0] (Object-elf.C:2805)
==472747==    by 0x50B1E70: Dyninst::SymtabAPI::Symtab::Symtab(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, bool, bool&) (Symtab.C:1121)
==472747==    by 0x50B5AB4: Dyninst::SymtabAPI::Symtab::openFile(Dyninst::SymtabAPI::Symtab*&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, Dyninst::SymtabAPI::Symtab::def_t) (Symtab.C:1799)
==472747==    by 0x62701A: analysis::analysis(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >) (analysis.cxx:75)
==472747== 

Expected behavior

With the clean up in flush_analysis_caches:analysis.cxx calling the destructors would not expect dyninst to still have 190+ MB of stuff still on the heap.

System (please complete the following information):

• x86, x86_64
• dyninst-11.0.1-3.fc35.x86_64
• glibc-2.34-21.fc35.x86_64

Additional context

Might be able to reproduce similar issue on a smaller scale with some of the examples in https://github.com/dyninst/examples . However, they don't look to actually attempt to free up things at the end of their runs.

[hainest]

Dyninst has a loose interpretation of memory management. Explicitly calling a destructor may or may not do anything useful as many of the member variables are shared pointers. Moreover, there are (too) many data structures with file scope and internal linkage that own heap memory (e.g., static std::map<...>). The DSOs for DyninstAPI and ParseAPI are each >100MB. The ones for SymtabAPI, InstructionAPI, and ProcControl are tens of MB each.

As an aside, I'm surprised that deleteing a Symtab* worked. When I've tested making one an automatic variable, its destructor always segfaulted.

[wcohen]

Maybe this issue should have been an enhancement rather than a bug report. I kind of figured that there might be limitations to how much can be cleaned up in Dyninst due to the complicated sharing of data structures. However, any improvement in how Dyninst handle memory use would be appreciated.

The code used SymtabAPI::Symtab::closeSymtab(i.second.symtab) rather than a delete on a Symtab * to attempt to reduce the ref count and attempt to clean things up.

I did have a question about data sharing. Does SymtabCodeSource(char *) uses the symbol table info cached from openFile(Symtab *&obj, std::string filename, def_t defensive_binary = NotDefensive)? I noticed there wasn't any change in runtime or space when switched to using the SymtabCodeSource(SymtabAPI::Symtab *).