Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Potential memory leak in clang triggered by findScope #16121

Closed
1 task done
pcanal opened this issue Jul 26, 2024 · 5 comments · Fixed by #16150
Closed
1 task done

Potential memory leak in clang triggered by findScope #16121

pcanal opened this issue Jul 26, 2024 · 5 comments · Fixed by #16150
Assignees
Milestone

Comments

@pcanal
Copy link
Member

pcanal commented Jul 26, 2024

Check duplicate issues.

  • Checked for duplicates

Description

As reported in https://root-forum.cern.ch/t/mem-leak-even-with-suppression-file/60330, we just simple executable:
CreateTreeTestv2.tar.gz, we get definitively lost report by valgrind:

==44592==    at 0x483A809: malloc (vg_replace_malloc.c:309)
==44592==    by 0x898CD15: clang::Parser::AnnotateTemplateIdToken(clang::OpaquePtr<clang::TemplateName>, clang::TemplateNameKind, clang::CXXScopeSpec&, clang::SourceLocation, clang::UnqualifiedId&, bool, bool) (in /export/home/jb242989/root_v6.32.02/lib/libCling.so.6.32.02)
==44592==    by 0x8921B63: clang::Parser::ParseOptionalCXXScopeSpecifier(clang::CXXScopeSpec&, clang::OpaquePtr<clang::QualType>, bool, bool, bool*, bool, clang::IdentifierInfo**, bool, bool) (in /export/home/jb242989/root_v6.32.02/lib/libCling.so.6.32.02)
==44592==    by 0x899A53C: clang::Parser::TryAnnotateCXXScopeToken(bool) (in /export/home/jb242989/root_v6.32.02/lib/libCling.so.6.32.02)
==44592==    by 0x80E5246: cling::LookupHelper::findScope(llvm::StringRef, cling::LookupHelper::DiagSetting, clang::Type const**, bool) const (in /export/home/jb242989/root_v6.32.02/lib/libCling.so.6.32.02)
==44592==    by 0x801074B: TCling::GetClassSharedLibs(char const*) (in /export/home/jb242989/root_v6.32.02/lib/libCling.so.6.32.02)
==44592==    by 0x801507A: TClingLookupHelper__ExistingTypeCheck(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&) (in /export/home/jb242989/root_v6.32.02/lib/libCling.so.6.32.02)
==44592==    by 0x4AB8C52: TClassEdit::TSplitType::ShortType(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, int) (in /export/home/jb242989/root_v6.32.02/lib/libCore.so.6.32.02)
==44592==    by 0x4AB98C0: TClassEdit::GetNormalizedName(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, std::basic_string_view<char, ...

Are those a real leak (they accumulate and could have been deleted), hoarding (they accumulate, are in use and can not be delete until the end) or one time (they are just not cleaned-up at the end?)

Reproducer

CreateTreeTestv2.tar.gz

make
valgrind --suppression=$ROOTSYS/etc/valgrind-root.supp createTree

ROOT version

Moving from 6.26/06 to 6.28/04 we’ve still a very large increase of mem leaks and it is still present in v6.32.02

Installation method

unknown

Operating system

Linux

Additional context

#13130

@pcanal
Copy link
Member Author

pcanal commented Jul 26, 2024

In a similar issue: #14199, the problem was avoided by reducing the number of (indirect) calls to findScope (with the side-effect of reducing overall work needed).

@ferdymercury
Copy link
Contributor

A smaller reproducer here. It seems the problem is triggered by TTree::Write

#include "TMemFile.h"
#include "TTree.h"

int main(int argc, char** argv)
{
    TMemFile *f = new TMemFile("f.root","RECREATE");
    TTree *t = new TTree("t","t");
    t->Write(); // If I comment this line, leak disappears
    f->Close();
    delete f;
}

@ferdymercury
Copy link
Contributor

And a oneliner reproducer here:

valgrind --leak-check=full --suppressions=$ROOTSYS/etc/valgrind-root.supp root.exe -n -l -b -q -e 'TMemFile f("f.root", "RECREATE"); TTree t; t.Write(); f.Close()'

@ferdymercury ferdymercury changed the title Potential memory leak in clang triggered by findScope. Potential memory leak in clang triggered by findScope Jul 26, 2024
@devajithvs
Copy link
Contributor

A smaller reproducer here. It seems the problem is triggered by TTree::Write

#include "TMemFile.h"
#include "TTree.h"

int main(int argc, char** argv)
{
    TMemFile *f = new TMemFile("f.root","RECREATE");
    TTree *t = new TTree("t","t");
    t->Write(); // If I comment this line, leak disappears
    f->Close();
    delete f;
}

I ran the above code in a for loop to see if the leak is recurring. I increased the iteration count by a significant factor, but the leak remained constant and did not increase with the iteration count. So maybe we just need to add this to the suppression file?

hahnjo added a commit to hahnjo/root that referenced this issue Aug 1, 2024
Upstream Clang keeps TemplateIdAnnotations around "if they might
still be in the token stream." See upstream commit for more details:
llvm/llvm-project@6163aa9
(included in Clang 11, in ROOT since the upgrade to LLVM 13)

This reasoning doesn't apply when we fully reset the Parser state in
ParserStateRAII's destructor, and we expect the swapped out vector of
TemplateIdAnnotations to be empty in order to not leak.

Fixes root-project#16121
@hahnjo hahnjo self-assigned this Aug 1, 2024
@hahnjo
Copy link
Member

hahnjo commented Aug 1, 2024

I think I agree with Valgrind's complaint here, I propose a fix in #16150.

@hahnjo hahnjo closed this as completed in e6bae6a Aug 2, 2024
silverweed pushed a commit to silverweed/root that referenced this issue Aug 14, 2024
Upstream Clang keeps TemplateIdAnnotations around "if they might
still be in the token stream." See upstream commit for more details:
llvm/llvm-project@6163aa9
(included in Clang 11, in ROOT since the upgrade to LLVM 13)

This reasoning doesn't apply when we fully reset the Parser state in
ParserStateRAII's destructor, and we expect the swapped out vector of
TemplateIdAnnotations to be empty in order to not leak.

Fixes root-project#16121
silverweed pushed a commit to silverweed/root that referenced this issue Aug 19, 2024
Upstream Clang keeps TemplateIdAnnotations around "if they might
still be in the token stream." See upstream commit for more details:
llvm/llvm-project@6163aa9
(included in Clang 11, in ROOT since the upgrade to LLVM 13)

This reasoning doesn't apply when we fully reset the Parser state in
ParserStateRAII's destructor, and we expect the swapped out vector of
TemplateIdAnnotations to be empty in order to not leak.

Fixes root-project#16121
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Issues
Development

Successfully merging a pull request may close this issue.

5 participants