[PATCH] D30762: [ubsan] Add a nullability sanitizer

[Vedant Kumar via Phabricator via cfe-commits]

vsk added a comment.

One question for Anna (inline). I will update the diff with the documentation/code comments/renaming fixes once I hear back. Thanks again for the comments!



================
Comment at: docs/UndefinedBehaviorSanitizer.rst:101
+     ``-fsanitize=nullability-assign``, and the argument check with
+     ``-fsanitize=nullability-arg``. While violating nullability rules does
+     not result in undefined behavior, it is often unintentional, so UBSan
----------------
zaks.anna wrote:
> Have you investigated if -fsanitize=nullability-arg is too noisy? I think we should somehow "recommend" return and assignment check to most users. This is not clear from the help and options provided. The main concern here is that someone would try -fsanitize=nullability, see a ton of warnings they don't care about and think that the whole check is too noisy.
> 
> I cannot come up with a great solution. Options that come into mind are:
>  - Drop "arg" check completely.
>  - Remove arg from the nullability group.
>  - Have 2 groups for nullability.
> 
> We can also wait for concrete data before making a decision here and address this in subsequent commits.
The nullability-arg check was not noisy on a small sample (n=4) of Apple-internal projects. I only collected 11 diagnostics in total, all of which seemed actionable. I.e the issues were not isolated to system headers: the project code actually did violate arg nullability. Based on what I've seen so far, I'd like to start off with including the arg check in a "nullability-full" group. If arg check noisiness becomes an issue, then I'd like to create a new "nullability-partial" group with just the return/assignment checks.

TL;DR: let's rename -fsanitize=nullability to -fsanitize=nullability-full, keep the arg check, and introduce a nullability-partial group later if we need to. Wdyt?




https://reviews.llvm.org/D30762