[Bro-Dev] newly found segfaults from metrics framework
robin at icir.org
Thu Aug 18 14:11:45 PDT 2011
On Tue, Aug 16, 2011 at 17:47 -0500, you wrote:
> I didn't think it could be sized right in the CompositeHash ctor since
> it's only got Types and not Vals to work with at that point, so I
> changed the computation to allocate and use a buffer on the fly if the
> pre-allocated one from the ctor isn't large enough to hold the key
> computed from the index vals.
I looked at this a bit more now. You're right that it can't know the
size, but in general the code already accounts for that: it
preallocates the buffer only if the size is static independent of the
value, and otherwise allocates on the fly already.
The problem is however that this check-if-static test is not done
right: for strings that are part of records and optional, it may say
it's static when it in fact it's not.
Your patch fixes that by double-checking that the supposedly static
size is indeed correct, but I think I have found a better fix: the
patch below gets rid of the crash for me. It also passes the
test-suite so I'm going to commit. Please put some stress on the
metrics framework everybody to see if we're fine now.
The patch may look a bit odd but the problem is that the semantics of
"v" are overloaded for SingleTypeKeySize(): v==0 can either mean we
are computing the size of a potential static buffer, or we have an
unset optional record field.
@@ -393,7 +393,7 @@ int CompositeHash::SingleTypeKeySize(BroType* bt, const Val* v,
sz = SingleTypeKeySize(rt->FieldType(i),
rv ? rv->Lookup(i) : 0,
- type_check, sz, optional);
+ type_check, sz, v && optional);
if ( ! sz )
Robin Sommer * Phone +1 (510) 722-6541 * robin at icir.org
ICSI/LBNL * Fax +1 (510) 666-2956 * www.icir.org
More information about the bro-dev