public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Alexandre Oliva <oliva@adacore.com>
To: gcc-patches@gcc.gnu.org
Subject: [00/13] check hash table counts
Date: Tue, 27 Dec 2022 01:07:35 -0300	[thread overview]
Message-ID: <or5ydxh4l4.fsf@lxoliva.fsfla.org> (raw)


While looking into another issue that corrupted hash tables, I added
code to check consistency of element counts, and hit numerous issues
that were concerning, of three kinds: insertion of entries that seem
empty, dangling insertions, and lookups during insertions.

These problems may all have the effect of replacing a deleted entry
with one that seems empty, which may disconnect double-hashing chains
involving that entry, and thus cause entries to go missing.

This patch, opening the patch series, only adds code to detect these
problems to the hash table verifier method.  I'm not sure it makes
sense to put it in, but I post it for the record.  Fixes and cheaper
detectors for some of these errors are going to be posted separately.

The number of bugs it revealed tells me that leaving callers in charge
of completing insertions is too error prone.  I found this
verification code a bit too expensive to enable in general.  We could
add check entry count cheaply at expand time and catch some of these
at very low cost, which would likely catch at least some of the
errors, but perhaps a safer interface could avoid them entirely.
WDYT?


I'm going to post a collection of 13 patches a as a followup to this
one, fixing various problems it has detected, or adding code to catch
some of these problems sooner.


for  gcc/ChangeLog

	* hash-table.h (verify): Check elements and deleted counts.
---
 gcc/hash-table.h |   17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/gcc/hash-table.h b/gcc/hash-table.h
index 53507daae26c3..7dbeea05373a2 100644
--- a/gcc/hash-table.h
+++ b/gcc/hash-table.h
@@ -1035,6 +1035,23 @@ hash_table<Descriptor, Lazy, Allocator>
 	  && Descriptor::equal (*entry, comparable))
 	hashtab_chk_error ();
     }
+
+  if (m_size < 2048)
+    {
+      size_t elmts = m_n_elements, dels = m_n_deleted;
+      for (size_t i = 0; i < m_size; i++)
+	{
+	  value_type *entry = &m_entries[i];
+	  if (!is_empty (*entry))
+	    {
+	      elmts--;
+	      if (is_deleted (*entry))
+		dels--;
+	    }
+	}
+      if (elmts || dels)
+	hashtab_chk_error ();
+    }
 }
 
 /* This function deletes an element with the given COMPARABLE value


-- 
Alexandre Oliva, happy hacker                https://FSFLA.org/blogs/lxo/
   Free Software Activist                       GNU Toolchain Engineer
Disinformation flourishes because many people care deeply about injustice
but very few check the facts.  Ask me about <https://stallmansupport.org>

             reply	other threads:[~2022-12-27  4:11 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-27  4:07 Alexandre Oliva [this message]
2022-12-27  4:17 ` [01/13] scoped tables: insert before further lookups Alexandre Oliva
2022-12-27 15:11   ` Jeff Law
2022-12-27  4:18 ` [02/13] varpool: do not add NULL vnodes to referenced Alexandre Oliva
2022-12-27 15:14   ` Jeff Law
2022-12-27  4:19 ` [03/13] tree-inline decl_map: skip mapping NULL to itself Alexandre Oliva
2022-12-27 15:15   ` Jeff Law
2022-12-27  4:21 ` [04/13] [C++] constraint: insert norm entry once Alexandre Oliva
2022-12-27 15:37   ` Jeff Law
2022-12-27  4:22 ` [05/13] ssa-loop-niter: skip caching of null operands Alexandre Oliva
2022-12-27 15:19   ` Jeff Law
2022-12-28  4:03     ` Alexandre Oliva
2022-12-27  4:23 ` [06/13] tree-inline decl_map: skip mapping result's NULL default def Alexandre Oliva
2022-12-27 15:23   ` Jeff Law
2022-12-27  4:24 ` [07/13] postreload-gcse: no insert on mere lookup Alexandre Oliva
2022-12-27 15:11   ` Jeff Law
2022-12-27  4:28 ` [08/13] tm: complete tm_restart insertion Alexandre Oliva
2022-12-27 15:27   ` Jeff Law
2022-12-27  4:30 ` [09/13] [C++] constexpr: request insert iff depth is ok Alexandre Oliva
2022-12-27 15:38   ` Jeff Law
2022-12-27  4:35 ` [10/13] lto: drop dummy partition mapping Alexandre Oliva
2022-12-27 15:34   ` Jeff Law
2022-12-27  4:38 ` [11/13] ada: don't map NULL decl to locus Alexandre Oliva
2022-12-27 15:33   ` Jeff Law
2022-12-27 16:54     ` Arnaud Charlet
2022-12-27  4:38 ` [12/13] hash set: reject attempts to add empty values Alexandre Oliva
2022-12-27 15:30   ` Jeff Law
2022-12-27  4:39 ` [13/13] hash-map: reject empty-looking insertions Alexandre Oliva
2022-12-27 15:31   ` Jeff Law
2022-12-27 17:53   ` David Malcolm
2022-12-28 12:32     ` [15/17] prevent hash set/map insertion of deleted entries Alexandre Oliva
2022-12-29  4:25       ` Jeff Law
2022-12-28  8:50 ` [00/13] check hash table counts Martin Liška
2022-12-28 12:46   ` [16/17] check hash table counts at expand Alexandre Oliva
2023-01-09  7:46     ` Richard Biener
2022-12-28 12:30 ` [14/17] parloops: don't request insert that won't be completed Alexandre Oliva
2022-12-29  2:44   ` Jeff Law
2022-12-28 12:50 ` [17/17] check hash table insertions Alexandre Oliva
2022-12-28 14:20   ` Richard Biener
2022-12-28 23:06     ` Alexandre Oliva
2022-12-29  7:29       ` Richard Biener
2022-12-30  8:53         ` Alexandre Oliva
2022-12-30 11:30           ` Richard Biener
2022-12-30 16:41             ` Alexandre Oliva

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=or5ydxh4l4.fsf@lxoliva.fsfla.org \
    --to=oliva@adacore.com \
    --cc=gcc-patches@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).