[webkit-changes] [WebKit/WebKit] f41084: Actually enable key validation for HashMap

[Chris Dumez]

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: f4108421a9758cca2331a9e68371afa0ed4ea317
      https://github.com/WebKit/WebKit/commit/f4108421a9758cca2331a9e68371afa0ed4ea317
  Author: Chris Dumez <cdumez at apple.com>
  Date:   2025-01-03 (Fri, 03 Jan 2025)

  Changed paths:
    M Source/WTF/wtf/HashMap.h
    M Source/WTF/wtf/HashSet.h
    M Source/WTF/wtf/HashTable.h
    M Source/WTF/wtf/RobinHoodHashTable.h

  Log Message:
  -----------
  Actually enable key validation for HashMap
https://bugs.webkit.org/show_bug.cgi?id=285359

Reviewed by Darin Adler.

We recently added made a change where HashMap was supposed to validate
the key when calling `add() / get() / remove() / take()` to crash safely
if the key is the special empty/deleted value instead of corrupting the
data structure. The original behavior was meant to be maintained by
UncheckedKeyHashMap for places where performance is too critical to
afford the extra validation.

However, the `shouldValidateKey` flag was never actually being passed
from the HashMap to its internal HashTable. As a result, the flag had
no effect and neither HashMap or UncheckedKeyHashMap were doing any
validation in release builds. This patch addresses the issue so that
we finally get the security benefits.

I have verified that this is performance-neutral on Speedometer 3.

* Source/WTF/wtf/HashMap.h:
* Source/WTF/wtf/HashSet.h:
* Source/WTF/wtf/HashTable.h:
(WTF::shouldValidateKey>::checkKey):
Drop the static assertion that enforces that checkKey() is actually able
to validate the key when `shouldValidateKey` is true (i.e. HashMap is used
instead of UncheckedKeyHashMap). While this seemed like a good idea at the
time, it is too restricting. For a `HashMap<String>` for example, we're able
to do key validation in general. However, if one call site was using a hash
translator (e.g. StringViewHashTranslator) as an optimization, checkKey()
would unnecessarily hit the static assertion. Note that no key validation
is actually needed when using `StringViewHashTranslator` as far as I can
tell. There is a no way to use this translator to generate an invalid
String key.

* Source/WTF/wtf/RobinHoodHashTable.h:

Canonical link: https://commits.webkit.org/288423@main



To unsubscribe from these emails, change your notification settings at https://github.com/WebKit/WebKit/settings/notifications