[PATCH] Static analysis checker for catch handler inversion

[Aaron Ballman]

On Fri, Nov 14, 2014 at 6:22 PM, Jonathan Roelofs
<jonathan at codesourcery.com> wrote:
>
>
> 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

Thank you for the good suggestion, I've used it to do some local
testing, but nothing automated as I think the cases are already
sensibly covered. However, it was a good list to double-check against.

~Aaron