[PATCH 1/1] StaticAnalyzer: fix memleak in CFRefCount

Clang Frontend
1

CFRefCount doesn't free alocated bug types. Do this in the destructor
if we allocated them in CherkerRegister.

Note that this approach is racy. If somebody holds the old immutable
map and iterates over that, they will encounter freed memory. The
question is whether we want is race-free?

Valgrind otherwise complains like:
172 (40 direct, 132 indirect) bytes in 1 blocks are definitely lost in loss record 27 of 27
    at 0x4C26337: operator new(unsigned long) (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
    by 0x16DF42E: (anonymous namespace)::CFRefCount::RegisterChecks(clang::ento::ExprEngine&) (in /l/latest/r
    by 0x1711DC3: clang::ento::ExprEngine::ExprEngine(clang::ento::AnalysisManager&, clang::ento::TransferFun
    by 0x16323BC: ActionExprEngine((anonymous namespace)::AnalysisConsumer&, clang::ento::AnalysisManager&, c
...

Signed-off-by: Jiri Slaby <jirislaby@gmail.com>

2

Hi Jiri,

I think what you are seeing is code in transition.

BugTypes were previously deallocated by the BugReporter class. Now they aren't, because Checkers can outlive a BugReporter (since Checkers are now managed by CheckerManager). I think the right approach is to actually reference count BugTypes if we are concerned about BugReporter objects outliving a Checker object. Alternatively, we can have checkers own them, managing them with an llvm::OwningPtr<>. Note that this leak is not actually a big deal right now since Checkers stay persistent for the for the analysis of an entire translation unit (so we're not leaking on every function analyzed), but I agree we should clean this up.

Ted

3

Hi, thanks for the feedback. I'll look if I'm able to fix this up myself.

thanks,

4

Great. The lifetime of these objects are gradually evolving, so an using an IntrusiveRefCntPtr may be the best way to go.