Analyzer committed to master (was Re: Analyzer status)

[David Malcolm <dmalcolm at redhat dot com>]

On Tue, 2020-01-14 at 08:55 +0100, Richard Biener wrote:
> On Mon, 13 Jan 2020, David Malcolm wrote:
> 
> > I posted the initial version of the analyzer patch kit on 2019-11-
> > 15,
> > shortly before the close of stage 1.
> > 
> > Jeff reviewed (most of) the latest version of the kit on Friday,
> > and
> > said:
> > 
> > > I'm not going to have time to finish #22 or #37 -- hell, I'm not
> > > even
> > > supposed to be working today :-)
> > > 
> > > I'd suggest committing now and we can iterate on any issues.  The
> > > code
> > > is appropriately conditionalized, so it shouldn't affect anyone
> > > that
> > > doesn't ask for it and it was submitted prior to stage1 close.
> > > 
> > > I would also suggest that we have a fairly loose policy for these
> > > bits
> > > right now.  Again, they're conditionally compiled and it's
> > > effectively
> > > "tech preview", so if we muck something up, the after-effects are
> > > relatively small.
> > 
> > Unfortunately, I didn't resolve the hash_table/hash_map issue
> > referred to here:
> >   https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00734.html
> > where r279139 on 2019-12-09 introduced the assumption that empty
> > hash_table entries and empty hash_map keys are all zeroes.
> > 
> > The analyzer uses hash_map::empty in a single place with a hash_map
> > where the "empty" key value is all-ones.
> > 
> > Unfortunately, without a fix for this issue, the analyzer can hang,
> > leading to DejaGnu hanging, which I clearly don't want to push to
> > master as-is.
> > 
> > The patch here fixes the issue:
> >   https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00776.html
> > but Jakub has expressed concern about the effect on code generated
> > for the compiler.
> > 
> > As noted here:
> >   https://gcc.gnu.org/ml/gcc-patches/2020-01/msg00651.html
> > gcc successfully converts this back to a memset to 0 for hash_table
> > at -O2, but not for hash_map, since it can't "know" that it's OK to
> > clobber the values as well as the keys; it instead generates a loop
> > that zeroes the keys without touching the values.
> > 
> > I've been experimenting with adding template specializations to try
> > to allow for memset to 0 for hash_map, but haven't been successful
> > (e.g. std::is_pod is C++11-only); my closest attempt so far is at:
> >   
> > https://dmalcolm.fedorapeople.org/gcc/2020-01-13/0001-FIXME-experiments-with-fixing-hash_map-empty.patch
> > which at least expresses the memset to 0 without needing
> > optimization for hash_table of *pointer* types, but I wasn't able
> > to
> > do the same for hash_map, today at least.
> > 
> > If the hash_map::empty patch is unacceptable, I've also
> > successfully
> > B&R-ed the following kludge to the analyzer, which avoids using
> > hash_map::empty at the single place where it's problematic.  This
> > kludge is entirely confined to the analyzer.
> > 
> > I'd like to get the analyzer code into gcc 10, but I appreciate
> > it's
> > now stage 4.  Jakub suggested on IRC that with approval before the
> > end of stage 3 (which it was), there may be some flexibility if we
> > get it in soon and I take steps to minimize disruption.
> > 
> > Some options:
> > (a) the patch to fix hash_table::empty, and the analyzer kit
> > (b) the analyzer kit with the following kludge
> > (c) someone with better C++-fu than me figure out a way to get the
> > memset optimization for hash_map with 0-valued empty (or to give me
> > some suggestions)
> > (d) not merge the analyzer for gcc 10
> > 
> > My preferred approach is option (a), but option (b) is confined to
> > the analyzer.
> > 
> > Thoughts?
> 
> (b)
> 
> we can iterate on (a)/(c) if deemed necessary but also this can be
> done
> during next stage1.

Thanks.  Jakub's proposed fix worked, so I've effectively gone with
(c).

I've rebased and squashed the analyzer patch kit and squashed patch 2
of the hash_table fix into it, and re-tested it successfully, so I've
pushed it to master (as 757bf1dff5e8cee34c0a75d06140ca972bfecfa7).

I'm going to work through the various followup patches I had on my
branch and re-test and push to master those that seem appropriate.

Dave