[webkit-dev] Smart Pointer Analysis Tool for WebKit

[Jan Korous]

Hi all,

I am an engineer at Security Tools team at Apple responsible for the tooling support for this effort.

> On Sep 23, 2020, at 12:34 PM, Darin Adler <darin at apple.com> wrote:
> 
>> On Sep 23, 2020, at 12:18 PM, Ryosuke Niwa <rniwa at webkit.org> wrote:
>> 
>> There are quite a few cases where data members are references but then those can also be replaced by a simple member function which retrieves the value of the smart pointer member variable and returns a reference.
> 
> I think this should be an explicit recommendation in the project of refactoring to follow these rules.
> 
>> For now, a trivial function is defined as a member function defined in the class declaration whose definition simply returns a member variable (the result of get() or a copy if the member variable is a smart pointer).
> 
> That seems like a rule that’s too narrow. I would not want a function to become non-trivial just because I moved it from being inline within the class definition to an inline below the class definition in the same header.
> 
> This rule worries me a lot right now; it seems like it could result in an explosion of local variable copies of arguments.
> 
>> We probably also need to figure out a way to exempt all lambda functions that never get stored anywhere. We have a bunch of helper functions like WTF::map which just calls lambdas on each item while iterating over an array, etc... and there is no need to create a separate Ref / RefPtr in those cases since lambdas are never stored and re-used later.
> 
> Does seem important. I am pretty sure I have seen this concept in other languages. We often try to use const Function& for one type of lambda argument and Function&& for the other type, but that’s far from complete.

Re: lambda captures - I think we have two ideas here.

1. Allow WeakPtr captures.
This makes sense to do but it implies we have to add the notion of ownership to the rules. The thing is that WeakPtr is safe on its own (and technically reference-counted) but it can’t be used as a safety measure in the way of RefPtr or Ref since it doesn’t own the memory (not even in a shared manner).

2. Allow raw pointer/reference captures.
This makes sense given you use generic algorithms in the codebase. I will implement a new version of the checker - currently it is still based on simple AST analysis and for this kind of reasoning we’ll need to use symbolic execution in Clang Static Analyzer.

Thanks,

Jan

> — Darin