Re: [PATCH 2/2] add -Wdangling-pointer [PR #63272]

[Martin Sebor <msebor@gmail.com>]

On 11/2/21 1:40 AM, Eric Gallager wrote:
> On Mon, Nov 1, 2021 at 6:20 PM Martin Sebor via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
>>
>> Patch 2 in this series adds support for detecting the uses of
>> dangling pointers: those to auto objects that have gone out of
>> scope.  Like patch 1, to minimize false positives this detection
>> is very simplistic.  However, thanks to the more deterministic
>> nature of the problem (all local objects go out of scope) is able
>> to detect more instances of it.  The approach I used is to simply
>> search the IL for clobbers that dominate uses of pointers to
>> the clobbered objects.  If such a use is found that's not
>> followed by a clobber of the same object the warning triggers.
>> Similar to -Wuse-after-free, the new -Wdangling-pointer option
>> has multiple levels: level 1 to detect unconditional uses and
>> level 2 to flag conditional ones.  Unlike with -Wuse-after-free
>> there is no use case for testing dangling pointers for
>> equality, so there is no level 3.
>>
>> Tested on x86_64-linux and  by building Glibc and Binutils/GDB.
>> It found no problems outside of the GCC test suite.
>>
>> As with the first patch in this series, the tests contain a number
>> of xfails due to known limitations marked with pr??????.  I'll
>> open bugs for them before committing the patch if I don't resolve
>> them first in a followup.
>>
>> Martin
> 
> So, I'd just like to take this chance to re-state my preference (as a
> user) for having separate named options for warnings instead of having
> a single option with multiple levels, so that users can toggle just
> one but not the other. With the numerical levels, one can detect only
> unconditional uses, and not conditional ones, by using level one, but
> they can't do it the other way around (i.e. detecting conditional
> uses, but not unconditional ones), though. I think having a split like
> the one that exists between -Wuninitialized and -Wmaybe-uninitialized
> would make sense here.

I agree that separate options are preferable for warnings with
different design strategies.  That said, I can't think of a use
case for enabling the "higher" level of either of any of our
warnings without also enabling the former.  Their sole purpose
is to control the S/R ratio, or perhaps also the cost in terms
of the compile-time resources spent on the analysis, though
I'm not aware of any measurements backing this up.

The distinction between levels 1 and 2 in the proposed warnings
is fairly subtle, and the higher level is by design exceedingly
unlikely to result in enough false positives to make lowering
it worthwhile.  It may not even be worth exposing a difference
between them via an option.   In fact, I think it might make
sense to merge them (perhaps keeping the "may" phrasing) unless
a compelling argument turns up in favor of keeping them distinct.

Martin