improve insertion after growth and add decision docs

Author: SchrodingerZhu

libc/docs/dev/undefined_behavior.rst

@@ -62,3 +62,10 @@ Often the standard will imply an intended behavior through what it states is und
 Ignoring Bug-For-Bug Compatibility
 ----------------------------------
 Any long running implementations will have bugs and deviations from the standard. Hyrum's Law states that “all observable behaviors of your system will be depended on by somebody” which includes these bugs. An example of a long-standing bug is glibc's scanf float parsing behavior. The behavior is specifically defined in the standard, but it isn't adhered to by all libc implementations. There is a longstanding bug in glibc where it incorrectly parses the string 100er and this caused the C standard to add that specific example to the definition for scanf. The intended behavior is for scanf, when parsing a float, to parse the longest possibly valid prefix and then accept it if and only if that complete parsed value is a float. In the case of 100er the longest possibly valid prefix is 100e but the float parsed from that string is only 100. Since there is no number after the e it shouldn't be included in the float, so scanf should return a parsing error. For LLVM's libc it was decided to follow the standard, even though glibc's version is slightly simpler to implement and this edge case is rare. Following the standard must be the first priority, since that's the goal of the library.
+
+Design Decisions
+================
+
+Resizable Tables for hsearch
+----------------------------
+The POSIX.1 standard does not delineate the behavior consequent to invoking hsearch or hdestroy without prior initialization of the hash table via hcreate. Furthermore, the standard does not articulate the outcomes of successive invocations of hsearch absent intervening hdestroy calls. Libraries such as MUSL and Glibc do not incorporate checks for these scenarios, potentially leading to memory corruption or leakage. Conversely, FreeBSD's libc and Bionic adopt a distinct methodology, automatically initializing the hash table to a minimal size if it is found uninitialized, and proceeding to destroy the table only if initialization has occurred. This approach also renders hcreate redundant if an initialized hash table is already present. Given that the hash table commences with a minimal size, resizing becomes necessary to accommodate additional user insertions. LLVM's libc mirrors the approach of FreeBSD's libc and Bionic, owing to its enhanced robustness and user-friendliness. Notably, such resizing behavior aligns with POSIX.1 standards, which explicitly permit implementations to modify the capacity of the hash table.

libc/src/__support/HashTable/table.h

@@ -184,7 +184,7 @@ struct HashTable {
   // Nor does the routine check if the table is full.
   // This is only to be used in grow() where we insert all existing entries
   // into a new table. Hence, the requirements are naturally satisfied.
-  LIBC_INLINE void unsafe_insert(ENTRY item) {
+  LIBC_INLINE ENTRY *unsafe_insert(ENTRY item) {
     uint64_t primary = oneshot_hash(item.key);
     uint8_t secondary = secondary_hash(primary);
     ProbeSequence sequence{static_cast<size_t>(primary), 0, entries_mask};
@@ -199,7 +199,7 @@ struct HashTable {
         entry(index).key = item.key;
         entry(index).data = item.data;
         available_slots--;
-        return;
+        return &entry(index);
       }
     }
   }
@@ -238,10 +238,8 @@ struct HashTable {
       // resized sccuessfully: clean up the old table and use the new one
       deallocate(table);
       table = new_table;
-      // hash also need to be recomputed since the hash state is updated
-      primary = table->oneshot_hash(item.key);
-      index = table->find(item.key, primary);
-      slot = &table->entry(index);
+      // it is still valid to use the fastpath insertion.
+      return table->unsafe_insert(item);
     }
 
     table->set_ctrl(index, secondary_hash(primary));

libc/test/src/__support/HashTable/table_test.cpp

@@ -100,9 +100,11 @@ TEST(LlvmLibcTableTest, Insertion) {
               static_cast<ENTRY *>(nullptr));
   }
 
-  // one more insert should grow successfully.
-  ASSERT_NE(HashTable::insert(table, {keys[CAP].bytes, keys[CAP].bytes}),
-            static_cast<ENTRY *>(nullptr));
+  // One more insert should grow the table successfully. We test the value
+  // here because the grow finishes with a fastpath insertion that is different
+  // from the normal insertion.
+  ASSERT_EQ(HashTable::insert(table, {keys[CAP].bytes, keys[CAP].bytes})->data,
+            static_cast<void *>(keys[CAP].bytes));
 
   for (size_t i = 0; i <= CAP; ++i) {
     ASSERT_EQ(strcmp(table->find(keys[i].bytes)->key, keys[i].bytes), 0);