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? I also have a series of followup patches to the analyzer (and diagnostic subsystem, for diagnostic_path-handling) to fix bugs found since I posted the kit, which I've been posting to gcc-patches since November with an "analyzer: " prefix in the subject line. I wonder if it's OK for me to take Jeff's message about "fairly loose policy" above as blanket pre-approval to make bug fixes to the analyzer subdirectory? (See also: https://gcc.gnu.org/wiki/DavidMalcolm/StaticAnalyzer ) Here's the analyzer-specific kludge for the hash_map::empty issue: --- gcc/analyzer/program-state.cc | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/gcc/analyzer/program-state.cc b/gcc/analyzer/program-state.cc index 04346ae9dc8..4c6c82cdf5d 100644 --- a/gcc/analyzer/program-state.cc +++ b/gcc/analyzer/program-state.cc @@ -405,7 +405,19 @@ sm_state_map::remap_svalue_ids (const svalue_id_map &map) } /* Clear the existing values. */ + /* FIXME: hash_map::empty and hash_table::empty make the assumption that + the empty value for the key is all zeroes, which isn't the case for + this hash_map. See + https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00776.html + and + https://gcc.gnu.org/ml/gcc-patches/2020-01/msg00651.html */ +#if 0 m_map.empty (); +#else + /* Workaround: manually remove each element. */ + while (!m_map.is_empty ()) + m_map.remove ((*m_map.begin ()).first); +#endif /* Copy over from intermediate map. */ for (typename map_t::iterator iter = tmp_map.begin (); -- 2.21.0
