Memory Leak in find_all()?

Hi,

I've just examined a memory leak in one of our installations and it seems to be caused by find_all() from strings.bif in bro.

Running bro with perftools enabled (cf. [1]), I get leak reports, as soon as my call to find_all() returns a non-empty list.
When changing find_all() in the following way (inspired by code in IRC.cc), the leak is not reported anymore and my scripts still work as expected:

old:
function find_all%(str: string, re: pattern%) : string_set
     %{
     TableVal* a = new TableVal(string_set);

     const u_char* s = str->Bytes();
     const u_char* e = s + str->Len();

     for ( const u_char* t = s; t < e; ++t )
         {
         int n = re->MatchPrefix(t, e - t);
         if ( n >= 0 )
             {
             a->Assign(new StringVal(n, (const char*) t), 0);
             t += n - 1;
             }
         }

     return a;
     %}

new:
function find_all%(str: string, re: pattern%) : string_set
     %{
     TableVal* a = new TableVal(string_set);

     const u_char* s = str->Bytes();
     const u_char* e = s + str->Len();

     for ( const u_char* t = s; t < e; ++t )
         {
         int n = re->MatchPrefix(t, e - t);
         if ( n >= 0 )
             {
             Val* ma = new StringVal(n, (const char*) t);
             a->Assign(ma, 0);
             Unref(ma);
             t += n - 1;
             }
         }

     return a;
     %}

Is my observation correct? If so, I could easily send a patch.
BTW: there seems to be another instance of the same problem in IRC.cc

Best regards,

Dirk

1) https://www.bro.org/development/howtos/leaks.html

It is, good catch. Assign() take ownership of the value (0 in this
case), but not of the index. Yes, please send a patch for this one and
other instances you find. Thanks,

Robin

Ok,

here we go (cf. attachment).

With my older state (2.3.XXX) of bro, all tests still are green after the patch.

The patch applies cleanly also to the current git head, but there I didn't execute the test suite.

Best regards,

Dirk

0001-fix-memory-leaks-in-find_all-and-IRC-analyzer.patch (1.33 KB)

Mind filing this as a ticket? That ensures it doesn't get lost.

Robin

Sure: https://bro-tracker.atlassian.net/projects/BIT/issues/BIT-1532