[PATCH] D12301: [PATCH] New checker for UB in handler of a function-try-block

[Aaron Ballman via cfe-commits]

On Tue, Aug 25, 2015 at 8:05 AM, Aaron Ballman <aaron.ballman at gmail.com> wrote:
> On Mon, Aug 24, 2015 at 7:42 PM, Richard Smith <richard at metafoo.co.uk> wrote:
>> On Mon, Aug 24, 2015 at 3:36 PM, Aaron Ballman <aaron.ballman at gmail.com>
>> wrote:
>>>
>>> On Mon, Aug 24, 2015 at 6:29 PM, Richard Smith <richard at metafoo.co.uk>
>>> wrote:
>>> > On Mon, Aug 24, 2015 at 3:23 PM, Aaron Ballman <aaron.ballman at gmail.com>
>>> > wrote:
>>> >>
>>> >> aaron.ballman created this revision.
>>> >> aaron.ballman added reviewers: alexfh, rsmith.
>>> >> aaron.ballman added a subscriber: cfe-commits.
>>> >>
>>> >> Per [except.handle]p10, the handler for a constructor or destructor
>>> >> function-try-block cannot refer to a non-static member of the object
>>> >> under
>>> >> construction. This patch adds a new clang-tidy check that warns the
>>> >> user
>>> >> when they've hit this undefined behavior.
>>> >>
>>> >> Due to how infrequent function-try-blocks appear on constructors and
>>> >> destructors in the wild compared to how often member expressions are
>>> >> encountered, I felt this was more appropriate as a clang-tidy check
>>> >> than as
>>> >> a semantic warning. I was concerned with efficiency of checking whether
>>> >> an
>>> >> arbitrary member expression was referring to the object under
>>> >> construction/destruction within the function-try-block catch handler
>>> >> scope.
>>> >
>>> >
>>> > Seems like this would be very cheap to check in the case where the
>>> > object
>>> > expression is an implicit or explicit CXXThisExpr. It'd be good to have
>>> > a
>>> > frontend warning for that case.
>>>
>>> Are you thinking the check would likely in BuildMemberReferenceExpr()
>>> and I would just have to look at the current scope to determine
>>> whether it's a function-try-block catch handler?
>>
>>
>> Yes, somewhere around there. You might need to wire a Scope* though a couple
>> of layers though.
>>
>>>
>>> When I looked into
>>> doing a frontend warning for this, I seemed to struggle with figuring
>>> out specifically that I was in the catch handler of a
>>> function-try-block of a constructor or destructor.
>>
>>
>> Looks like there's no better method than walking up the Scope stack until
>> you hit a scope with the FnTryCatchScope flag (or leave the function),
>> currently.
>
> That only tells you that you're in a function-try-block, not that
> you're within the handler of a function-try-block, doesn't it? So to
> implement this, it seems like I'd have to:
>
> 1) Thread a Scope * through to BuildMemberReferenceExpr()
> 2) Update the Scope class to have both FnTryScope, FnCatchScope, and
> FnTryCatchScope (that's a bitwise OR of FnTryScope and FnCatchScope)

Ah, perhaps I don't have to do this step. It seems that I should be
able to look for FnTryCatchScope & TryScope to determine if it is a
function-try-block try scope or not.

~Aaron

> 3) Update the parser to create the proper Scope type for a
> function-try-block's handler
> 4) Implement the actual warning
>
> I *think* that should about cover it.
>
> ~Aaron