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

Message ID orilhxe9z1.fsf@lxoliva.fsfla.org
State Accepted
Headers
Series [01/13] scoped tables: insert before further lookups |

Checks

Context Check Description
snail/gcc-patch-check success Github commit url

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)
>
  

Patch

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)