[PATCH] D122143: [clang][dataflow] Add support for disabling warnings on smart pointers.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added a comment.

In D122143#3396615 <https://reviews.llvm.org/D122143#3396615>, @xazax.hun wrote:

> Are smart pointers special? I would expect to see similar problems with containers (or even a nested optional).

No, they're not. You're quite right -- containers in general are a problem. We happen to have support (landing soon) for nested optionals, but the general point still holds.

> I wonder whether an allowlist instead of a denylist approach is better here. E.g., instead of disabling the modeling for smart pointers, we could enable it for cases that we actually support.

I like this idea, although we'll need to work out how to describe the set of things we track effectively.  My argument against would be that it moves to "unsafe" as the default. But, given that we know we'll *always* report (right or wrong) in these situations, the safety argument isn't tht strong, because we're simply not adding anything of value in our reports -- users could just as well be told to always double check optionals in containers, etc.

So, do you mean to add a FIXME to move to allowlist, or do you mean to hold off until we've switched? I have a short-term interest in getting this through for a particular usecase, but I understand if you feel it just not a good idea. Regardless, I'm going to get started exploring an allowlist approach.

> (or alternatively, we could add a confidence value to the unsafe access). Usually, these checks are pretty robust when we deal with objects on the stack of the analyzed function (locals, parameters), but it is really hard to reason about objects from the outside (e.g., when a reference to an object is acquired from a container or smart pointer) unless we have explicit modeling for the APIs. The confidence approach might be useful as we are unlikely to cover all the custom smart pointers the users have.

This idea sounds useful, but I'm not really sure how it would play out. I suppose we'd then let the user set a confidence level for diagnostics (like logging level?). Regardless, I think for a first attempt, I'd rather go with binary yes/no. But interested in exploring this approach.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122143/new/

https://reviews.llvm.org/D122143