[PATCH] Static analysis checker for catch handler inversion

[Jonathan Roelofs]

On 11/13/14 10:33 AM, Aaron Ballman wrote:
> On Wed, Nov 12, 2014 at 5:05 PM, Richard Smith <richard at metafoo.co.uk> wrote:
>> I think it's not too hard to avoid the quadratic performance here. Algorithm
>> outline:
>>
>> Start with empty map of type -> handlers
>> For each catch handler for a type T:
>>    For each base class U of T (including T itself), most-derived-first:
>>      If we have a handler for U in our map, warn, break
>>    Add handler for T to our map
>>
>> (This is still quadratic if you have struct A<n> : A<n-1>, and catch A<0>,
>> A<1>, ..., A<n> in that order, but in that case I think your approach is
>> cubic (because IsDerivedFrom is itself linear in the number of base
>> classes.)
>
> I've reworked the patch to implement your algorithm. Thanks!
>
>>
>> Also, I'm not sure you're correctly handling this case:
>>
>>    try { ... }
>>    catch (Base *) {}
>>    catch (Derived &) {}
>>
>> We shouldn't warn here, but I think you might, because you strip off a
>> top-level * or & quite early. I'd also like to see some test coverage for
>> catch parameters with cv-qualifiers.

Aaron,

I remember adding some tests to libcxxabi which exhaustively check a few of the 
cv-requirements for catch handlers... maybe those are a useful starting point 
for this? Those tests are in libcxxabi/test/catch_pointer_reference.cpp


Cheers,

Jon

>
> Correct, that was handled incorrectly before, but has been fixed in
> this patch. I've added some further tests to make sure of it.
>
> Thanks!
>
> ~Aaron
>
>>

-- 
Jon Roelofs
jonathan at codesourcery.com
CodeSourcery / Mentor Embedded