[15/17] prevent hash set/map insertion of deleted entries

Message ID or5ydvemiz.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. 28, 2022, 12:32 p.m. UTC
  On Dec 27, 2022, David Malcolm <dmalcolm@redhat.com> wrote:

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

Yeah, I guess so.  I've come up with something for hash-table proper
too, coming up in 17/17.


Just like the recently-added checks for empty entries, add checks for
deleted entries as well.  This didn't catch any problems, but it might
prevent future accidents.  Suggested by David Malcolm.

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


for  gcc/ChangeLog

	* hash-map.h (put, get_or_insert): Check that added entry
	doesn't look deleted either.
	& hash-set.h (add): Likewise.
---
 gcc/hash-map.h |    8 +++++---
 gcc/hash-set.h |    3 ++-
 2 files changed, 7 insertions(+), 4 deletions(-)
  

Comments

Jeff Law Dec. 29, 2022, 4:25 a.m. UTC | #1
On 12/28/22 05:32, Alexandre Oliva via Gcc-patches wrote:
> On Dec 27, 2022, David Malcolm <dmalcolm@redhat.com> wrote:
> 
>> Would it make sense to also add assertions that such entries aren't
>> Traits::is_deleted?  (both for hash_map and hash_set)
> 
> Yeah, I guess so.  I've come up with something for hash-table proper
> too, coming up in 17/17.
> 
> 
> Just like the recently-added checks for empty entries, add checks for
> deleted entries as well.  This didn't catch any problems, but it might
> prevent future accidents.  Suggested by David Malcolm.
> 
> Regstrapped on x86_64-linux-gnu.  Ok to install?
> 
> 
> for  gcc/ChangeLog
> 
> 	* hash-map.h (put, get_or_insert): Check that added entry
> 	doesn't look deleted either.
> 	& hash-set.h (add): Likewise.
OK
jeff
  

Patch

diff --git a/gcc/hash-map.h b/gcc/hash-map.h
index 63fa21cf37c5b..e6ca9cf5e6429 100644
--- a/gcc/hash-map.h
+++ b/gcc/hash-map.h
@@ -173,8 +173,9 @@  public:
       if (ins)
 	{
 	  e->m_key = k;
-	  new ((void *) &e->m_value) Value (v);
-	  gcc_checking_assert (!Traits::is_empty (*e));
+	  new ((void *)&e->m_value) Value (v);
+	  gcc_checking_assert (!Traits::is_empty (*e)
+			       && !Traits::is_deleted (*e));
 	}
       else
 	e->m_value = v;
@@ -204,7 +205,8 @@  public:
 	{
 	  e->m_key = k;
 	  new ((void *)&e->m_value) Value ();
-	  gcc_checking_assert (!Traits::is_empty (*e));
+	  gcc_checking_assert (!Traits::is_empty (*e)
+			       && !Traits::is_deleted (*e));
 	}
 
       if (existed != NULL)
diff --git a/gcc/hash-set.h b/gcc/hash-set.h
index a98121a060eed..08e1851d5118d 100644
--- a/gcc/hash-set.h
+++ b/gcc/hash-set.h
@@ -61,7 +61,8 @@  public:
 	{
 	  new (e) Key (k);
 	  // Catch attempts to insert e.g. a NULL pointer.
-	  gcc_checking_assert (!Traits::is_empty (*e));
+	  gcc_checking_assert (!Traits::is_empty (*e)
+			       && !Traits::is_deleted (*e));
 	}
 
       return existed;