[13/13] hash-map: reject empty-looking insertions

Message ID	orilhxe9z1.fsf@lxoliva.fsfla.org
State	New
Headers	show Return-Path: <gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org> DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 0B6933858C27 To: gcc-patches@gcc.gnu.org Subject: [13/13] hash-map: reject empty-looking insertions Organization: Free thinker, does not speak for AdaCore References: <or5ydxh4l4.fsf@lxoliva.fsfla.org> Date: Tue, 27 Dec 2022 01:39:30 -0300 In-Reply-To: <or5ydxh4l4.fsf@lxoliva.fsfla.org> (Alexandre Oliva's message of "Tue, 27 Dec 2022 01:07:35 -0300") Message-ID: <orilhxe9z1.fsf@lxoliva.fsfla.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain Precedence: list From: Alexandre Oliva via Gcc-patches <gcc-patches@gcc.gnu.org> Reply-To: Alexandre Oliva <oliva@adacore.com> Errors-To: gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org Sender: "Gcc-patches" <gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org>
Series	[01/13] scoped tables: insert before further lookups \| expand [01/13] scoped tables: insert before further lookups [02/13] varpool: do not add NULL vnodes to referenced [03/13] tree-inline decl_map: skip mapping NULL to itself [04/13,C++] constraint: insert norm entry once [05/13] ssa-loop-niter: skip caching of null operands [06/13] tree-inline decl_map: skip mapping result's NULL default def [07/13] postreload-gcse: no insert on mere lookup [08/13] tm: complete tm_restart insertion [09/13,C++] constexpr: request insert iff depth is ok [10/13] lto: drop dummy partition mapping [11/13] ada: don't map NULL decl to locus [12/13] hash set: reject attempts to add empty values [13/13] hash-map: reject empty-looking insertions [14/17] parloops: don't request insert that won't be completed [15/17] prevent hash set/map insertion of deleted entries [16/17] check hash table counts at expand [17/17] check hash table insertions

Message ID

orilhxe9z1.fsf@lxoliva.fsfla.org

State

New

Headers

DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 0B6933858C27
To: gcc-patches@gcc.gnu.org
Subject: [13/13] hash-map: reject empty-looking insertions
Organization: Free thinker, does not speak for AdaCore
References: <or5ydxh4l4.fsf@lxoliva.fsfla.org>
Date: Tue, 27 Dec 2022 01:39:30 -0300
In-Reply-To: <or5ydxh4l4.fsf@lxoliva.fsfla.org> (Alexandre Oliva's message of
 "Tue, 27 Dec 2022 01:07:35 -0300")
Message-ID: <orilhxe9z1.fsf@lxoliva.fsfla.org>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
Precedence: list
From: Alexandre Oliva via Gcc-patches <gcc-patches@gcc.gnu.org>
Reply-To: Alexandre Oliva <oliva@adacore.com>
Errors-To: gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org
Sender: "Gcc-patches"
 <gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org>

Series

[01/13] scoped tables: insert before further lookups | expand

Commit Message

Alexandre Oliva Dec. 27, 2022, 4:39 a.m. UTC

Check, after inserting entries, that they don't look empty.

Regstrapped on x86_64-linux-gnu.  Ok to install?


for  gcc/ChangeLog

	* hash-map.h (put, get_or_insert): Check that entry does not
	look empty after insertion.
---
 gcc/hash-map.h |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Jeff Law Dec. 27, 2022, 3:31 p.m. UTC | #1

On 12/26/22 21:39, Alexandre Oliva via Gcc-patches wrote:
> 
> Check, after inserting entries, that they don't look empty.
> 
> Regstrapped on x86_64-linux-gnu.  Ok to install?
> 
> 
> for  gcc/ChangeLog
> 
> 	* hash-map.h (put, get_or_insert): Check that entry does not
> 	look empty after insertion.
OK once prereqs are installed.
jeff

David Malcolm Dec. 27, 2022, 5:53 p.m. UTC | #2

On Tue, 2022-12-27 at 01:39 -0300, Alexandre Oliva via Gcc-patches
wrote:
> 
> Check, after inserting entries, that they don't look empty.

Thanks for working on this - I often get this stuff wrong.

Would it make sense to also add assertions that such entries aren't
Traits::is_deleted?  (both for hash_map and hash_set)

Dave

> 
> Regstrapped on x86_64-linux-gnu.  Ok to install?
> 
> 
> for  gcc/ChangeLog
> 
>         * hash-map.h (put, get_or_insert): Check that entry does not
>         look empty after insertion.
> ---
>  gcc/hash-map.h |    4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/gcc/hash-map.h b/gcc/hash-map.h
> index 457967f4bf1ae..63fa21cf37c5b 100644
> --- a/gcc/hash-map.h
> +++ b/gcc/hash-map.h
> @@ -169,11 +169,12 @@ public:
>      {
>        hash_entry *e = m_table.find_slot_with_hash (k, Traits::hash
> (k),
>                                                    INSERT);
> -      bool ins = hash_entry::is_empty (*e);
> +      bool ins = Traits::is_empty (*e);
>        if (ins)
>         {
>           e->m_key = k;
>           new ((void *) &e->m_value) Value (v);
> +         gcc_checking_assert (!Traits::is_empty (*e));
>         }
>        else
>         e->m_value = v;
> @@ -203,6 +204,7 @@ public:
>         {
>           e->m_key = k;
>           new ((void *)&e->m_value) Value ();
> +         gcc_checking_assert (!Traits::is_empty (*e));
>         }
>  
>        if (existed != NULL)
>

diff --git a/gcc/hash-map.h b/gcc/hash-map.h
index 457967f4bf1ae..63fa21cf37c5b 100644
--- a/gcc/hash-map.h
+++ b/gcc/hash-map.h
@@ -169,11 +169,12 @@  public:
     {
       hash_entry *e = m_table.find_slot_with_hash (k, Traits::hash (k),
 						   INSERT);
-      bool ins = hash_entry::is_empty (*e);
+      bool ins = Traits::is_empty (*e);
       if (ins)
 	{
 	  e->m_key = k;
 	  new ((void *) &e->m_value) Value (v);
+	  gcc_checking_assert (!Traits::is_empty (*e));
 	}
       else
 	e->m_value = v;
@@ -203,6 +204,7 @@  public:
 	{
 	  e->m_key = k;
 	  new ((void *)&e->m_value) Value ();
+	  gcc_checking_assert (!Traits::is_empty (*e));
 	}
 
       if (existed != NULL)

[13/13] hash-map: reject empty-looking insertions

Commit Message

Comments

Patch