Re: [PATCH][RFC] Sanitize equals and hash functions in hash-tables.

["Martin Liška" <mliska@suse.cz>]

[-- Attachment #1: Type: text/plain, Size: 16616 bytes --]

On 6/12/19 10:02 AM, Martin Liška wrote:
> On 6/12/19 9:59 AM, Richard Biener wrote:
>> On Tue, Jun 11, 2019 at 9:02 PM Jason Merrill <jason@redhat.com> wrote:
>>>
>>> On 6/11/19 9:16 AM, Martin Liška wrote:
>>>> On 6/11/19 2:27 PM, Jason Merrill wrote:
>>>>> On 6/11/19 3:41 AM, Martin Liška wrote:
>>>>>> On 6/10/19 8:21 PM, Jason Merrill wrote:
>>>>>>> On Mon, Jun 10, 2019 at 3:08 AM Martin Liška <mliska@suse.cz> wrote:
>>>>>>>> On 6/7/19 11:43 PM, Jason Merrill wrote:
>>>>>>>>> On Fri, Jun 7, 2019 at 8:14 AM Martin Liška <mliska@suse.cz> wrote:
>>>>>>>>>> On 6/7/19 2:09 PM, Richard Biener wrote:
>>>>>>>>>>> On Fri, Jun 7, 2019 at 2:03 PM Martin Liška <mliska@suse.cz> wrote:
>>>>>>>>>>>> On 6/7/19 10:57 AM, Richard Biener wrote:
>>>>>>>>>>>>> On Mon, Jun 3, 2019 at 3:35 PM Martin Liška <mliska@suse.cz> wrote:
>>>>>>>>>>>>>> On 6/1/19 12:06 AM, Jeff Law wrote:
>>>>>>>>>>>>>>> On 5/22/19 3:13 AM, Martin Liška wrote:
>>>>>>>>>>>>>>>> On 5/21/19 1:51 PM, Richard Biener wrote:
>>>>>>>>>>>>>>>>> On Tue, May 21, 2019 at 1:02 PM Martin Liška <mliska@suse.cz> wrote:
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> On 5/21/19 11:38 AM, Richard Biener wrote:
>>>>>>>>>>>>>>>>>>> On Tue, May 21, 2019 at 12:07 AM Jeff Law <law@redhat.com> wrote:
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> On 5/13/19 1:41 AM, Martin Liška wrote:
>>>>>>>>>>>>>>>>>>>>> On 11/8/18 9:56 AM, Martin Liška wrote:
>>>>>>>>>>>>>>>>>>>>>> On 11/7/18 11:23 PM, Jeff Law wrote:
>>>>>>>>>>>>>>>>>>>>>>> On 10/30/18 6:28 AM, Martin Liška wrote:
>>>>>>>>>>>>>>>>>>>>>>>> On 10/30/18 11:03 AM, Jakub Jelinek wrote:
>>>>>>>>>>>>>>>>>>>>>>>>> On Mon, Oct 29, 2018 at 04:14:21PM +0100, Martin Liška wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>> +hashtab_chk_error ()
>>>>>>>>>>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>>>>>>>>>>> +  fprintf (stderr, "hash table checking failed: "
>>>>>>>>>>>>>>>>>>>>>>>>>> +           "equal operator returns true for a pair "
>>>>>>>>>>>>>>>>>>>>>>>>>> +           "of values with a different hash value");
>>>>>>>>>>>>>>>>>>>>>>>>> BTW, either use internal_error here, or at least if using fprintf
>>>>>>>>>>>>>>>>>>>>>>>>> terminate with \n, in your recent mail I saw:
>>>>>>>>>>>>>>>>>>>>>>>>> ...different hash valueduring RTL pass: vartrack
>>>>>>>>>>>>>>>>>>>>>>>>>                       ^^^^^^
>>>>>>>>>>>>>>>>>>>>>>>> Sure, fixed in attached patch.
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>> Martin
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>> +  gcc_unreachable ();
>>>>>>>>>>>>>>>>>>>>>>>>>> +}
>>>>>>>>>>>>>>>>>>>>>>>>>     Jakub
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>> 0001-Sanitize-equals-and-hash-functions-in-hash-tables.patch
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>   From 0d9c979c845580a98767b83c099053d36eb49bb9 Mon Sep 17 00:00:00 2001
>>>>>>>>>>>>>>>>>>>>>>>> From: marxin <mliska@suse.cz>
>>>>>>>>>>>>>>>>>>>>>>>> Date: Mon, 29 Oct 2018 09:38:21 +0100
>>>>>>>>>>>>>>>>>>>>>>>> Subject: [PATCH] Sanitize equals and hash functions in hash-tables.
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>> ---
>>>>>>>>>>>>>>>>>>>>>>>>    gcc/hash-table.h | 40 +++++++++++++++++++++++++++++++++++++++-
>>>>>>>>>>>>>>>>>>>>>>>>    1 file changed, 39 insertions(+), 1 deletion(-)
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>> diff --git a/gcc/hash-table.h b/gcc/hash-table.h
>>>>>>>>>>>>>>>>>>>>>>>> index bd83345c7b8..694eedfc4be 100644
>>>>>>>>>>>>>>>>>>>>>>>> --- a/gcc/hash-table.h
>>>>>>>>>>>>>>>>>>>>>>>> +++ b/gcc/hash-table.h
>>>>>>>>>>>>>>>>>>>>>>>> @@ -503,6 +503,7 @@ private:
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>      value_type *alloc_entries (size_t n CXX_MEM_STAT_INFO) const;
>>>>>>>>>>>>>>>>>>>>>>>>      value_type *find_empty_slot_for_expand (hashval_t);
>>>>>>>>>>>>>>>>>>>>>>>> +  void verify (const compare_type &comparable, hashval_t hash);
>>>>>>>>>>>>>>>>>>>>>>>>      bool too_empty_p (unsigned int);
>>>>>>>>>>>>>>>>>>>>>>>>      void expand ();
>>>>>>>>>>>>>>>>>>>>>>>>      static bool is_deleted (value_type &v)
>>>>>>>>>>>>>>>>>>>>>>>> @@ -882,8 +883,12 @@ hash_table<Descriptor, Allocator>
>>>>>>>>>>>>>>>>>>>>>>>>      if (insert == INSERT && m_size * 3 <= m_n_elements * 4)
>>>>>>>>>>>>>>>>>>>>>>>>        expand ();
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>> -  m_searches++;
>>>>>>>>>>>>>>>>>>>>>>>> +#if ENABLE_EXTRA_CHECKING
>>>>>>>>>>>>>>>>>>>>>>>> +    if (insert == INSERT)
>>>>>>>>>>>>>>>>>>>>>>>> +      verify (comparable, hash);
>>>>>>>>>>>>>>>>>>>>>>>> +#endif
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>> +  m_searches++;
>>>>>>>>>>>>>>>>>>>>>>>>      value_type *first_deleted_slot = NULL;
>>>>>>>>>>>>>>>>>>>>>>>>      hashval_t index = hash_table_mod1 (hash, m_size_prime_index);
>>>>>>>>>>>>>>>>>>>>>>>>      hashval_t hash2 = hash_table_mod2 (hash, m_size_prime_index);
>>>>>>>>>>>>>>>>>>>>>>>> @@ -930,6 +935,39 @@ hash_table<Descriptor, Allocator>
>>>>>>>>>>>>>>>>>>>>>>>>      return &m_entries[index];
>>>>>>>>>>>>>>>>>>>>>>>>    }
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>> +#if ENABLE_EXTRA_CHECKING
>>>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>>> +/* Report a hash table checking error.  */
>>>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>>> +ATTRIBUTE_NORETURN ATTRIBUTE_COLD
>>>>>>>>>>>>>>>>>>>>>>>> +static void
>>>>>>>>>>>>>>>>>>>>>>>> +hashtab_chk_error ()
>>>>>>>>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>>>>>>>>> +  fprintf (stderr, "hash table checking failed: "
>>>>>>>>>>>>>>>>>>>>>>>> +     "equal operator returns true for a pair "
>>>>>>>>>>>>>>>>>>>>>>>> +     "of values with a different hash value\n");
>>>>>>>>>>>>>>>>>>>>>>>> +  gcc_unreachable ();
>>>>>>>>>>>>>>>>>>>>>>>> +}
>>>>>>>>>>>>>>>>>>>>>>> I think an internal_error here is probably still better than a simple
>>>>>>>>>>>>>>>>>>>>>>> fprintf, even if the fprintf is terminated with a \n :-)
>>>>>>>>>>>>>>>>>>>>>> Fully agree with that, but I see a lot of build errors when using internal_error.
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> The question then becomes can we bootstrap with this stuff enabled and
>>>>>>>>>>>>>>>>>>>>>>> if not, are we likely to soon?  It'd be a shame to put it into
>>>>>>>>>>>>>>>>>>>>>>> EXTRA_CHECKING, but then not be able to really use EXTRA_CHECKING
>>>>>>>>>>>>>>>>>>>>>>> because we've got too many bugs to fix.
>>>>>>>>>>>>>>>>>>>>>> Unfortunately it's blocked with these 2 PRs:
>>>>>>>>>>>>>>>>>>>>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87845
>>>>>>>>>>>>>>>>>>>>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87847
>>>>>>>>>>>>>>>>>>>>> Hi.
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> I've just added one more PR:
>>>>>>>>>>>>>>>>>>>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90450
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> I'm sending updated version of the patch that provides a disablement for the 3 PRs
>>>>>>>>>>>>>>>>>>>>> with a new function disable_sanitize_eq_and_hash.
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> With that I can bootstrap and finish tests. However, I've done that with a patch
>>>>>>>>>>>>>>>>>>>>> limits maximal number of checks:
>>>>>>>>>>>>>>>>>>>> So rather than call the disable_sanitize_eq_and_hash, can you have its
>>>>>>>>>>>>>>>>>>>> state set up when you instantiate the object?  It's not a huge deal,
>>>>>>>>>>>>>>>>>>>> just thinking about loud.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> So how do we want to go forward, particularly the EXTRA_EXTRA checking
>>>>>>>>>>>>>>>>>>>> issue :-)
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> There is at least one PR where we have a table where elements _in_ the
>>>>>>>>>>>>>>>>>>> table are never compared against each other but always against another
>>>>>>>>>>>>>>>>>>> object (I guess that's usual even), but the setup is in a way that the
>>>>>>>>>>>>>>>>>>> comparison function only works with those.  With the patch we verify
>>>>>>>>>>>>>>>>>>> hashing/comparison for something that is never used.
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> So - wouldn't it be more "correct" to only verify comparison/hashing
>>>>>>>>>>>>>>>>>>> at lookup time, using the object from the lookup and verify that against
>>>>>>>>>>>>>>>>>>> all other elements?
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> I don't a have problem with that. Apparently this changes fixes
>>>>>>>>>>>>>>>>>> PR90450 and PR87847.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Changes from previous version:
>>>>>>>>>>>>>>>>>> - verification happens only when an element is searched (not inserted)
>>>>>>>>>>>>>>>>>> - new argument 'sanitize_eq_and_hash' added for hash_table::hash_table
>>>>>>>>>>>>>>>>>> - new param has been introduced hash-table-verification-limit in order
>>>>>>>>>>>>>>>>>>     to limit number of elements that are compared within a table
>>>>>>>>>>>>>>>>>> - verification happens only with flag_checking >= 2
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> I've been bootstrapping and testing the patch right now.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Looks like I misremembered the original patch.  The issue isn't
>>>>>>>>>>>>>>>>> comparing random two elements in the table.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> That it fixes PR90450 is because LIM never calls find_slot_with_hash
>>>>>>>>>>>>>>>>> without INSERTing.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> There's updated version of the patch where I check all find operations
>>>>>>>>>>>>>>>> (both w/ and w/o insertion).
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests
>>>>>>>>>>>>>>>> except for:
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> $ ./xgcc -B. /home/marxin/Programming/gcc/gcc/testsuite/gcc.dg/torture/pr63941.c -O2 -c
>>>>>>>>>>>>>>>> hash table checking failed: equal operator returns true for a pair of values with a different hash value
>>>>>>>>>>>>>>>> during GIMPLE pass: lim
>>>>>>>>>>>>>>>> /home/marxin/Programming/gcc/gcc/testsuite/gcc.dg/torture/pr63941.c: In function ‘fn1’:
>>>>>>>>>>>>>>>> /home/marxin/Programming/gcc/gcc/testsuite/gcc.dg/torture/pr63941.c:6:1: internal compiler error: in hashtab_chk_error, at hash-table.h:1019
>>>>>>>>>>>>>>>>       6 | fn1 ()
>>>>>>>>>>>>>>>>         | ^~~
>>>>>>>>>>>>>>>> 0x6c5725 hashtab_chk_error
>>>>>>>>>>>>>>>>        /home/marxin/Programming/gcc/gcc/hash-table.h:1019
>>>>>>>>>>>>>>>> 0xe504ea hash_table<mem_ref_hasher, false, xcallocator>::verify(ao_ref* const&, unsigned int)
>>>>>>>>>>>>>>>>        /home/marxin/Programming/gcc/gcc/hash-table.h:1040
>>>>>>>>>>>>>>>> 0xe504ea hash_table<mem_ref_hasher, false, xcallocator>::find_slot_with_hash(ao_ref* const&, unsigned int, insert_option)
>>>>>>>>>>>>>>>>        /home/marxin/Programming/gcc/gcc/hash-table.h:960
>>>>>>>>>>>>>>>> 0xe504ea gather_mem_refs_stmt
>>>>>>>>>>>>>>>>        /home/marxin/Programming/gcc/gcc/tree-ssa-loop-im.c:1501
>>>>>>>>>>>>>>>> 0xe504ea analyze_memory_references
>>>>>>>>>>>>>>>>        /home/marxin/Programming/gcc/gcc/tree-ssa-loop-im.c:1625
>>>>>>>>>>>>>>>> 0xe504ea tree_ssa_lim
>>>>>>>>>>>>>>>>        /home/marxin/Programming/gcc/gcc/tree-ssa-loop-im.c:2646
>>>>>>>>>>>>>>>> 0xe504ea execute
>>>>>>>>>>>>>>>>        /home/marxin/Programming/gcc/gcc/tree-ssa-loop-im.c:2708
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Richi: it's after your recent patch.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> For some reason I don't see PR87847 issue any longer.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> May I install the patch with disabled sanitization in tree-ssa-loop-im.c ?
>>>>>>>>>>>>>>> Don't we still need to deal with the naked fprintf when there's a
>>>>>>>>>>>>>>> failure.  ie, shouldn't we be raising it with a gcc_assert or somesuch?
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Good point, I've just adjusted that.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Ready to be installed?
>>>>>>>>>>>>>
>>>>>>>>>>>>> Ugh, the cselib one is really bad.  But I don't hold my breath for anyone
>>>>>>>>>>>>> fixing it ...
>>>>>>>>>>>>
>>>>>>>>>>>> Yes :D It's been some time and there's no interest in the PR.
>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> One question - there's unconditional
>>>>>>>>>>>>>
>>>>>>>>>>>>> +         if (m_sanitize_eq_and_hash)
>>>>>>>>>>>>> +           verify (comparable, hash);
>>>>>>>>>>>>>
>>>>>>>>>>>>> which will read a global variable and have (possibly not inline) call
>>>>>>>>>>>>> to verify on a common path even with checking disabled.  So I think
>>>>>>>>>>>>> we want to compile this checking feature out for !CHECKING_P
>>>>>>>>>>>>> or at least make the if __builtin_expect (..., 0), ::verify not
>>>>>>>>>>>>> inlined and marked pure () (thus, !CHECKING_P is simplest ;)).
>>>>>>>>>>>>
>>>>>>>>>>>> Fixed. May I install the patch? The cselib issue can be solved later..
>>>>>>>>>>>
>>>>>>>>>>> You missed the second occurance
>>>>>>>>>>>
>>>>>>>>>>> -  m_searches++;
>>>>>>>>>>> +  if (m_sanitize_eq_and_hash)
>>>>>>>>>>> +    verify (comparable, hash);
>>>>>>>>>>
>>>>>>>>>> Yep ;) I've just install the patch.
>>>>>>>>>
>>>>>>>>> This is breaking my build:
>>>>>>>>>
>>>>>>>>> /home/jason/gt/gcc/hash-map.h:123:71: error: no matching function for
>>>>>>>>> call to ‘hash_table<hash_map<mem_alloc_d\
>>>>>>>>> escription<mem_usage>::mem_location_hash, mem_usage*,
>>>>>>>>> simple_hashmap_traits<default_hash_traits<mem_alloc_desc\
>>>>>>>>> ription<mem_usage>::mem_location_hash>, mem_usage*> >::hash_entry,
>>>>>>>>> false, xcallocator>::hash_table(size_t&, bo\
>>>>>>>>> ol&, bool&, mem_alloc_origin, const char*&, int&, const char*&)’
>>>>>>>>>        : m_table (n, ggc, gather_mem_stats, HASH_MAP_ORIGIN PASS_MEM_STAT) {}
>>>>>>>>>
>>>>>>>>> Looks like this needs to be updated to pass an argument to the new
>>>>>>>>> sanitize_eq_and_hash parameter.
>>>>>>>>>
>>>>>>>>> Jason
>>>>>>>>
>>>>>>>> Sorry for the breakage, I've just fixed that in r272104.
>>>>>>>
>>>>>>> Thanks.  I'm also seeing a massive compile time hit from this:  A
>>>>>>> constexpr testcase that I've been looking at went from compiling in 13
>>>>>>> seconds to 78 seconds, 6 times as long.  I would expect template-heavy
>>>>>>> code to see similar problems when sanitization is enabled for those
>>>>>>> hash tables.  Could we keep the parameter low or 0 by default, and
>>>>>>> just do occasional sanitize runs with it explicitly enabled?
>>>>>>
>>>>>> Makes sense to me. Can you please provide a test-case which I can measure?
>>>>>
>>>>> This is the one I've been looking at:
>>>>>
>>>>>    struct Int {
>>>>>      constexpr Int(int v): v(v) {}
>>>>>      constexpr Int& operator+=(Int b) { this->v += b.v; return *this; }
>>>>>      constexpr Int& operator++() { ++this->v; return *this; }
>>>>>    private:
>>>>>      friend constexpr bool operator<(Int a, Int b) { return a.v < b.v; }
>>>>>      int v;
>>>>>    };
>>>>>    constexpr int f(int n) {
>>>>>      Int i = {0};
>>>>>      Int k = {0};
>>>>>      k = 0;
>>>>>      for (; k<10000; ++k) {
>>>>>        i += k;
>>>>>      }
>>>>>      return n;
>>>>>    }
>>>>>
>>>>>    template<int N> struct S {
>>>>>      static constexpr int sm = S<N-1>::sm+f(N);
>>>>>    };
>>>>>    template<> struct S<0> {
>>>>>      static constexpr int sm = 0;
>>>>>    };
>>>>>    constexpr int r = S<20>::sm;
>>>>>
>>>>> Jason
>>>>
>>>> For the test-case provided I see:
>>>>
>>>> $ time g++ time.cc -c --param hash-table-verification-limit=100
>>>>
>>>> real  0m1.855s
>>>> user  0m1.829s
>>>> sys   0m0.025s
>>>>
>>>> $ time g++ time.cc -c --param hash-table-verification-limit=0
>>>>
>>>> real  0m1.275s
>>>> user  0m1.219s
>>>> sys   0m0.052s
>>>>
>>>> $ time g++-9 time.cc -c
>>>>
>>>> real  0m0.939s
>>>> user  0m0.827s
>>>> sys   0m0.109s
>>>>
>>>> So it's slower, but I can't confirm the huge slowdown you see.
>>>> Is it due to r272144?
>>>
>>> Hmm, I wonder if this is because of the
>>> --enable-gather-detailed-mem-stats hash tables.
>>
>> I wonder if we can reduce the overhead by making
>> hash-tables/maps using predefined traits not perform
>> the checking?  Thus make [the default] whether to check or not
>> to check part of the traits?  Surely the basic pointer-hash/map
>> stuff is OK and needs no extra checking.
> 
> Interesting idea! I can prepare a patch. Right now I'm testing a patch
> that removes sanitization for hash-tables (and maps) that track memory
> allocations.

I've got 2 patch candidates that will make it happen. It survives build
on --enable-languages=all, but one would need to build all cross-compilers
to make a proper testing.

Do you like the idea of the patch before I'll write a changelog and test it?

Thanks,
Martin

> 
> Martin
> 
>>
>> Richard.
>>
>>> Jason
> 


[-- Attachment #2: 0002-Disable-hash-table-sanitization-for-mem-stats-maps.patch --]
[-- Type: text/x-patch, Size: 2694 bytes --]

From 4888f897e579267abb6ebf94646518f5c9f2da1d Mon Sep 17 00:00:00 2001
From: Martin Liska <mliska@suse.cz>
Date: Wed, 12 Jun 2019 11:04:11 +0200
Subject: [PATCH 2/2] Disable hash-table sanitization for mem stats maps.

---
 gcc/ggc-common.c | 2 +-
 gcc/hash-map.h   | 9 ++++++---
 gcc/mem-stats.h  | 6 +++---
 3 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/gcc/ggc-common.c b/gcc/ggc-common.c
index 2acdb6dc60c..6fb5a3d5ceb 100644
--- a/gcc/ggc-common.c
+++ b/gcc/ggc-common.c
@@ -1014,5 +1014,5 @@ ggc_prune_overhead_list (void)
       (*it).second.first->m_collected += (*it).second.second;
 
   delete ggc_mem_desc.m_reverse_object_map;
-  ggc_mem_desc.m_reverse_object_map = new map_t (13, false, false);
+  ggc_mem_desc.m_reverse_object_map = new map_t (13, false, false, false);
 }
diff --git a/gcc/hash-map.h b/gcc/hash-map.h
index 151632730e7..77f17275ec3 100644
--- a/gcc/hash-map.h
+++ b/gcc/hash-map.h
@@ -119,16 +119,19 @@ class GTY((user)) hash_map
 
 public:
   explicit hash_map (size_t n = 13, bool ggc = false,
+		     bool sanitize_eq_and_hash = true,
 		     bool gather_mem_stats = GATHER_STATISTICS
 		     CXX_MEM_STAT_INFO)
-    : m_table (n, ggc, true, gather_mem_stats, HASH_MAP_ORIGIN PASS_MEM_STAT)
+    : m_table (n, ggc, sanitize_eq_and_hash, gather_mem_stats,
+	       HASH_MAP_ORIGIN PASS_MEM_STAT)
   {
   }
 
   explicit hash_map (const hash_map &h, bool ggc = false,
+		     bool sanitize_eq_and_hash = true,
 		     bool gather_mem_stats = GATHER_STATISTICS
 		     CXX_MEM_STAT_INFO)
-    : m_table (h.m_table, ggc, true, gather_mem_stats,
+    : m_table (h.m_table, ggc, sanitize_eq_and_hash, gather_mem_stats,
 	       HASH_MAP_ORIGIN PASS_MEM_STAT) {}
 
   /* Create a hash_map in ggc memory.  */
@@ -137,7 +140,7 @@ public:
 			       CXX_MEM_STAT_INFO)
     {
       hash_map *map = ggc_alloc<hash_map> ();
-      new (map) hash_map (size, true, gather_mem_stats PASS_MEM_STAT);
+      new (map) hash_map (size, true, true, gather_mem_stats PASS_MEM_STAT);
       return map;
     }
 
diff --git a/gcc/mem-stats.h b/gcc/mem-stats.h
index 63ce8712e2b..77960595753 100644
--- a/gcc/mem-stats.h
+++ b/gcc/mem-stats.h
@@ -559,9 +559,9 @@ template <class T>
 inline
 mem_alloc_description<T>::mem_alloc_description ()
 {
-  m_map = new mem_map_t (13, false, false);
-  m_reverse_map = new reverse_mem_map_t (13, false, false);
-  m_reverse_object_map = new reverse_object_map_t (13, false, false);
+  m_map = new mem_map_t (13, false, false, false);
+  m_reverse_map = new reverse_mem_map_t (13, false, false, false);
+  m_reverse_object_map = new reverse_object_map_t (13, false, false, false);
 }
 
 /* Default destructor.  */
-- 
2.21.0


[-- Attachment #3: 0001-Add-sanitize-into-hash-traits-and-disable-it-for-obv.patch --]
[-- Type: text/x-patch, Size: 12263 bytes --]

From 7b96fb6e6d6c5d0e574440ecf0b842d71d5a8049 Mon Sep 17 00:00:00 2001
From: Martin Liska <mliska@suse.cz>
Date: Wed, 12 Jun 2019 11:03:33 +0200
Subject: [PATCH 1/2] Add ::sanitize into hash-traits and disable it for
 obvious types.

---
 gcc/attribs.c         |  5 +++++
 gcc/cp/cp-tree.h      |  2 ++
 gcc/cp/decl2.c        |  1 +
 gcc/gcov.c            |  2 ++
 gcc/graphite.c        |  1 +
 gcc/hash-map-traits.h | 26 ++++++++++++++++++++++++++
 gcc/hash-map.h        |  1 +
 gcc/hash-table.h      |  5 +++--
 gcc/hash-traits.h     | 25 +++++++++++++++++++++++++
 gcc/ipa-devirt.c      |  2 ++
 gcc/ipa-prop.c        |  3 +++
 gcc/profile.c         |  1 +
 gcc/sanopt.c          |  2 ++
 gcc/tree-hasher.h     |  1 +
 gcc/tree-ssa-sccvn.c  |  1 +
 gcc/tree-vect-slp.c   |  1 +
 16 files changed, 77 insertions(+), 2 deletions(-)

diff --git a/gcc/attribs.c b/gcc/attribs.c
index 4441922543f..06a6b56b634 100644
--- a/gcc/attribs.c
+++ b/gcc/attribs.c
@@ -2057,6 +2057,11 @@ struct excl_hash_traits: typed_noop_remove<excl_pair>
   {
     return !*x.first && !*x.second;
   }
+
+  static bool sanitize ()
+  {
+    return true;
+  }
 };
 
 
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 1f4e1e15554..d0ce3b68693 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -869,6 +869,7 @@ struct named_decl_hash : ggc_remove <tree>
   /* Nothing is deletable.  Everything is insertable.  */
   static bool is_deleted (value_type) { return false; }
   static void mark_deleted (value_type) { gcc_unreachable (); }
+  inline static bool sanitize () { return true; }
 };
 
 /* Simplified unique_ptr clone to release a tree vec on exit.  */
@@ -1815,6 +1816,7 @@ struct named_label_hash : ggc_remove <named_label_entry *>
   /* Nothing is deletable.  Everything is insertable.  */
   inline static bool is_deleted (value_type) { return false; }
   inline static void mark_deleted (value_type) { gcc_unreachable (); }
+  inline static bool sanitize () { return true; }
 };
 
 /* Global state pertinent to the current function.  */
diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c
index 338db4ab6de..f4f5e878eb7 100644
--- a/gcc/cp/decl2.c
+++ b/gcc/cp/decl2.c
@@ -131,6 +131,7 @@ struct mangled_decl_hash : ggc_remove <tree>
   {
     e = reinterpret_cast <value_type> (1);
   }
+  inline static bool sanitize () { return true; }
 };
 
 /* A hash table of decls keyed by mangled name.  Used to figure out if
diff --git a/gcc/gcov.c b/gcc/gcov.c
index b06a6714c2e..015cac6247a 100644
--- a/gcc/gcov.c
+++ b/gcc/gcov.c
@@ -1234,6 +1234,8 @@ struct function_start_pair_hash : typed_noop_remove <function_start>
   {
     return ref.start_line == ~2U;
   }
+
+  inline static bool sanitize () { return true; }
 };
 
 /* Process a single input file.  */
diff --git a/gcc/graphite.c b/gcc/graphite.c
index 67202e276e2..c85f88e9e34 100644
--- a/gcc/graphite.c
+++ b/gcc/graphite.c
@@ -237,6 +237,7 @@ struct sese_scev_hash : typed_noop_remove <seir_cache_key>
   static void mark_empty (seir_cache_key &key) { key.entry_dest = 0; }
   static bool is_deleted (const seir_cache_key &key) { return !key.expr; }
   static bool is_empty (const seir_cache_key &key) { return key.entry_dest == 0; }
+  static bool sanitize () { return true; }
 };
 
 static hash_map<sese_scev_hash, tree> *seir_cache;
diff --git a/gcc/hash-map-traits.h b/gcc/hash-map-traits.h
index af66018e92c..a52c272d7e2 100644
--- a/gcc/hash-map-traits.h
+++ b/gcc/hash-map-traits.h
@@ -40,6 +40,7 @@ struct simple_hashmap_traits
   template <typename T> static inline bool is_deleted (const T &);
   template <typename T> static inline void mark_empty (T &);
   template <typename T> static inline void mark_deleted (T &);
+  static inline bool sanitize ();
 };
 
 template <typename H, typename Value>
@@ -98,6 +99,13 @@ simple_hashmap_traits <H, Value>::mark_deleted (T &entry)
   H::mark_deleted (entry.m_key);
 }
 
+template <typename H, typename Value>
+inline bool
+simple_hashmap_traits <H, Value>::sanitize ()
+{
+  return H::sanitize ();
+}
+
 template <typename H, typename Value>
 struct simple_cache_map_traits: public simple_hashmap_traits<H,Value>
 {
@@ -117,6 +125,7 @@ struct unbounded_hashmap_traits
   template <typename T> static inline bool is_deleted (const T &);
   template <typename T> static inline void mark_empty (T &);
   template <typename T> static inline void mark_deleted (T &);
+  template <typename T> static inline bool sanitize ();
 };
 
 template <typename Value>
@@ -159,6 +168,14 @@ unbounded_hashmap_traits <Value>::mark_deleted (T &entry)
   default_hash_traits <Value>::mark_deleted (entry.m_value);
 }
 
+template <typename Value>
+template <typename T>
+inline bool
+unbounded_hashmap_traits <Value>::sanitize ()
+{
+  return T::sanitize ();
+}
+
 /* Implement traits for a hash_map from integer type Key to Value in
    cases where Key has no spare values for recording empty and deleted
    slots.  */
@@ -169,6 +186,7 @@ struct unbounded_int_hashmap_traits : unbounded_hashmap_traits <Value>
   typedef Key key_type;
   static inline hashval_t hash (Key);
   static inline bool equal_keys (Key, Key);
+  static inline bool sanitize ();
 };
 
 template <typename Key, typename Value>
@@ -185,4 +203,12 @@ unbounded_int_hashmap_traits <Key, Value>::equal_keys (Key k1, Key k2)
   return k1 == k2;
 }
 
+template <typename Key, typename Value>
+inline bool
+unbounded_int_hashmap_traits <Key, Value>::sanitize ()
+{
+  return false;
+}
+
+
 #endif // HASH_MAP_TRAITS_H
diff --git a/gcc/hash-map.h b/gcc/hash-map.h
index a8eb42d5a03..151632730e7 100644
--- a/gcc/hash-map.h
+++ b/gcc/hash-map.h
@@ -55,6 +55,7 @@ class GTY((user)) hash_map
 
     static void mark_empty (hash_entry &e) { Traits::mark_empty (e); }
     static bool is_empty (const hash_entry &e) { return Traits::is_empty (e); }
+    static bool sanitize () { return Traits::sanitize (); }
 
     static void ggc_mx (hash_entry &e)
       {
diff --git a/gcc/hash-table.h b/gcc/hash-table.h
index 4f5e150a0ac..de5ebc8ba6d 100644
--- a/gcc/hash-table.h
+++ b/gcc/hash-table.h
@@ -600,7 +600,8 @@ hash_table<Descriptor, Lazy, Allocator>::hash_table (size_t size, bool ggc,
 						     mem_alloc_origin origin
 						     MEM_STAT_DECL) :
   m_n_elements (0), m_n_deleted (0), m_searches (0), m_collisions (0),
-  m_ggc (ggc), m_sanitize_eq_and_hash (sanitize_eq_and_hash)
+  m_ggc (ggc),
+  m_sanitize_eq_and_hash (sanitize_eq_and_hash && Descriptor::sanitize ())
 #if GATHER_STATISTICS
   , m_gather_mem_stats (gather_mem_stats)
 #endif
@@ -633,7 +634,7 @@ hash_table<Descriptor, Lazy, Allocator>::hash_table (const hash_table &h,
 						     MEM_STAT_DECL) :
   m_n_elements (h.m_n_elements), m_n_deleted (h.m_n_deleted),
   m_searches (0), m_collisions (0), m_ggc (ggc),
-  m_sanitize_eq_and_hash (sanitize_eq_and_hash)
+  m_sanitize_eq_and_hash (sanitize_eq_and_hash && Descriptor::sanitize ())
 #if GATHER_STATISTICS
   , m_gather_mem_stats (gather_mem_stats)
 #endif
diff --git a/gcc/hash-traits.h b/gcc/hash-traits.h
index 2d17e2c982a..37a97c31491 100644
--- a/gcc/hash-traits.h
+++ b/gcc/hash-traits.h
@@ -91,6 +91,7 @@ struct int_hash : typed_noop_remove <Type>
   static inline void mark_empty (Type &);
   static inline bool is_deleted (Type);
   static inline bool is_empty (Type);
+  static inline bool sanitize ();
 };
 
 template <typename Type, Type Empty, Type Deleted>
@@ -136,6 +137,13 @@ int_hash <Type, Empty, Deleted>::is_empty (Type x)
   return x == Empty;
 }
 
+template <typename Type, Type Empty, Type Deleted>
+inline bool
+int_hash <Type, Empty, Deleted>::sanitize ()
+{
+  return false;
+}
+
 /* Pointer hasher based on pointer equality.  Other types of pointer hash
    can inherit this and override the hash and equal functions with some
    other form of equality (such as string equality).  */
@@ -153,6 +161,7 @@ struct pointer_hash
   static inline void mark_empty (Type *&);
   static inline bool is_deleted (Type *);
   static inline bool is_empty (Type *);
+  static inline bool sanitize ();
 };
 
 template <typename Type>
@@ -200,6 +209,13 @@ pointer_hash <Type>::is_empty (Type *e)
   return e == NULL;
 }
 
+template <typename Type>
+inline bool
+pointer_hash <Type>::sanitize ()
+{
+  return true;
+}
+
 /* Hasher for "const char *" strings, using string rather than pointer
    equality.  */
 
@@ -326,6 +342,7 @@ struct pair_hash
   static inline void mark_empty (value_type &);
   static inline bool is_deleted (const value_type &);
   static inline bool is_empty (const value_type &);
+  static inline bool sanitize ();
 };
 
 template <typename T1, typename T2>
@@ -378,6 +395,14 @@ pair_hash <T1, T2>::is_empty (const value_type &x)
   return T1::is_empty (x.first);
 }
 
+template <typename T1, typename T2>
+inline bool
+pair_hash <T1, T2>::sanitize ()
+{
+  return T1::sanitize ();
+}
+
+
 template <typename T> struct default_hash_traits : T {};
 
 template <typename T>
diff --git a/gcc/ipa-devirt.c b/gcc/ipa-devirt.c
index a4c8b0de86f..26f0d332e00 100644
--- a/gcc/ipa-devirt.c
+++ b/gcc/ipa-devirt.c
@@ -170,6 +170,8 @@ struct default_hash_traits <type_pair>
     {
       e.first = NULL;
     }
+
+  static bool sanitize () { return true; }
 };
 
 static bool odr_types_equivalent_p (tree, tree, bool, bool *,
diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
index d86c2f3db55..57cd864b74d 100644
--- a/gcc/ipa-prop.c
+++ b/gcc/ipa-prop.c
@@ -99,6 +99,8 @@ struct ipa_bit_ggc_hash_traits : public ggc_cache_remove <ipa_bits *>
     {
       p = reinterpret_cast<ipa_bits *> (1);
     }
+
+  static bool sanitize () { return true; }
 };
 
 /* Hash table for avoid repeated allocations of equal ipa_bits.  */
@@ -144,6 +146,7 @@ struct ipa_vr_ggc_hash_traits : public ggc_cache_remove <value_range_base *>
     {
       p = reinterpret_cast<value_range_base *> (1);
     }
+  inline static bool sanitize () { return true; }
 };
 
 /* Hash table for avoid repeated allocations of equal value_ranges.  */
diff --git a/gcc/profile.c b/gcc/profile.c
index 9aff9ef2b21..668888f89ab 100644
--- a/gcc/profile.c
+++ b/gcc/profile.c
@@ -884,6 +884,7 @@ struct location_triplet_hash : typed_noop_remove <location_triplet>
   {
     return ref.lineno == -2;
   }
+  inline static bool sanitize () { return true; }
 };
 
 
diff --git a/gcc/sanopt.c b/gcc/sanopt.c
index 5cb98e1b50e..d2f9ba5a56c 100644
--- a/gcc/sanopt.c
+++ b/gcc/sanopt.c
@@ -147,6 +147,7 @@ struct sanopt_tree_triplet_hash : typed_noop_remove <sanopt_tree_triplet>
   {
     return ref.t1 == NULL;
   }
+  inline static bool sanitize () { return true; }
 };
 
 /* Tree couple for ptr_check_map.  */
@@ -202,6 +203,7 @@ struct sanopt_tree_couple_hash : typed_noop_remove <sanopt_tree_couple>
   {
     return ref.ptr == NULL;
   }
+  inline static bool sanitize () { return true; }
 };
 
 /* This is used to carry various hash maps and variables used
diff --git a/gcc/tree-hasher.h b/gcc/tree-hasher.h
index a64f297fb2a..24da6c0a416 100644
--- a/gcc/tree-hasher.h
+++ b/gcc/tree-hasher.h
@@ -42,6 +42,7 @@ struct int_tree_hasher
   static bool is_empty (const value_type &v) { return v.to == NULL; }
   static void mark_empty (value_type &v) { v.to = NULL; }
   static void remove (value_type &) {}
+  inline static bool sanitize () { return true; }
 };
 
 /* Hash a UID in a int_tree_map.  */
diff --git a/gcc/tree-ssa-sccvn.c b/gcc/tree-ssa-sccvn.c
index b4f626000dd..69d365dc5d2 100644
--- a/gcc/tree-ssa-sccvn.c
+++ b/gcc/tree-ssa-sccvn.c
@@ -335,6 +335,7 @@ struct vn_ssa_aux_hasher : typed_noop_remove <vn_ssa_aux_t>
   static inline void mark_empty (value_type &e) { e = NULL; }
   static inline bool is_deleted (value_type &) { return false; }
   static inline bool is_empty (value_type &e) { return e == NULL; }
+  inline static bool sanitize () { return true; }
 };
 
 hashval_t
diff --git a/gcc/tree-vect-slp.c b/gcc/tree-vect-slp.c
index 930cd79784e..10ad0632853 100644
--- a/gcc/tree-vect-slp.c
+++ b/gcc/tree-vect-slp.c
@@ -1023,6 +1023,7 @@ struct bst_traits
   static inline void mark_empty (value_type &x) { x.release (); }
   static inline void mark_deleted (value_type &x) { x.release (); }
   static inline void remove (value_type &x) { x.release (); }
+  inline static bool sanitize () { return true; }
 };
 inline hashval_t
 bst_traits::hash (value_type x)
-- 
2.21.0