Help Troubleshooting a Perftools Memleak

Development
1

The MySQL analyzer is ready to go, apart from one issue: a memleak btest that I wrote is failing on some of Bro’s regex code.

@TEST-EXEC: HEAP_CHECK_DUMP_DIRECTORY=. HEAPCHECK=local btest-bg-run bro bro -b -m -r $TRACES/mysql/mysql.trace %INPUT

@load base/protocols/mysql

This results in:

Leak check net_run detected leaks of 203 bytes in 4 objects

The 4 largest leaks:
Leak of 72 bytes in 1 objects allocated from:
@ 53e92d

Leak of 56 bytes in 1 objects allocated from:
@ 52fb66
@ 83f663

Leak of 56 bytes in 1 objects allocated from:
@ 52fd52
@ 20014

Leak of 19 bytes in 1 objects allocated from:
@ 53e9b6

Digging a little deeper shows that two of these leaks are in RE_parse (re-parse.y:110 and re-parse.y:133), one is in re__scan_buffer (re-scan.cc:2035), and one is in re__scan_bytes (re-scan.cc::2084).

The only regular expression that I have in the analyzer is: type NUL_String = RE/[^\0]*/;

I’m pretty sure that this isn’t really an issue, but can anyone help with figuring out how to get the btest to pass? I’d really like to have a memleak test for this.

Thanks,

–Vlad

2

I'm pretty sure I know where that's coming from: normally regexp
compilation is happening before Bro's main loop, and hence not
accounted for by the leak checking. BinPAC however delays
initialization until the first time it tries to match something.

Unfortunately the obvious fix doesn't work. I'm pasting it in below,
but that change lets Bro crash because it now depends on the order in
which global constructors run. If anybody has a better idea, let me
know.

Robin

diff --git a/lib/binpac_regex.h b/lib/binpac_regex.h
index b41e6db..7acc2c6 100644
--- a/lib/binpac_regex.h
+++ b/lib/binpac_regex.h
@@ -14,7 +14,8 @@ public:
        RegExMatcher(const char *pattern)
                : pattern_(pattern)
                {
- re_matcher_ = 0;
+ re_matcher_ = new RE_Matcher(pattern_.c_str());
+ re_matcher_->Compile();
                }

        ~RegExMatcher()
@@ -25,11 +26,6 @@ public:
        // Returns the length of longest match, or -1 on mismatch.
        int MatchPrefix(const_byteptr data, int len)
                {
- if ( ! re_matcher_ )
- {
- re_matcher_ = new RE_Matcher(pattern_.c_str());
- re_matcher_->Compile();
- }
                return re_matcher_->MatchPrefix(data, len);
                }

3

Extending your idea a bit and probably hacking it into a place originally intended for something else. It may be a breaking change for other applications besides Bro, depending on how they define their RE_Matcher::Compile(), but not hard to fix.

diff --git a/lib/binpac_regex.h b/lib/binpac_regex.h
index b41e6db..ce5e7e3 100644
--- a/lib/binpac_regex.h
+++ b/lib/binpac_regex.h
@@ -14,7 +14,8 @@ public:
        RegExMatcher(const char *pattern)
                : pattern_(pattern)
                {
- re_matcher_ = 0;
+ re_matcher_ = new RE_Matcher(pattern_.c_str());
+ re_matcher_->Compile(1);
                }

        ~RegExMatcher()
@@ -25,11 +26,6 @@ public:
        // Returns the length of longest match, or -1 on mismatch.
        int MatchPrefix(const_byteptr data, int len)
                {
- if ( ! re_matcher_ )
- {
- re_matcher_ = new RE_Matcher(pattern_.c_str());
- re_matcher_->Compile();
- }
                return re_matcher_->MatchPrefix(data, len);
                }

diff --git a/src/RE.cc b/src/RE.cc
index 4855b0e..8aecbed 100644
--- a/src/RE.cc
+++ b/src/RE.cc
@@ -426,8 +426,27 @@ void RE_Matcher::AddPat(const char* new_pat)
        re_exact->AddPat(new_pat);
        }

+std::vector<RE_Matcher*> uncompiled_re_matchers;

4

I'm using your code but calling it slightly different via a new
binpac::init() function. Not sure it's much better, but a tiny bit
maybe. :slight_smile: I've pushed the change into master but have actually not
tried yet if it indeed fixes the reported memleak.

Robin

5

Thanks, Robin. I tested it and perftools is no longer reporting any leaks.

  --Vlad