Deleted values not checked for WeakMember in HashTraits |
|||
Issue descriptionThe HashTraits for checking liveness of WeakMember do not consider deleted values [1]. This is problematic whenever we delete single buckets containing WeakMember. E.g. 1. Create data structure (something using WTF::HashTable) with WeakMember. 2. Hold the table alive from the stack, e.g. through an iterator 3. Delete some WeakMember key. 4. Trigger conservative GC and trace the backingstore with kNoWeakHandling from the stack. 5. End up in [2]. 6. Crash somewhere along the path by deref'ing (void*)-1 (== kHashTableDeletedValue). Crash was flushed out by running w/ incremental marking as the write barriers on WTF collections are currently implemented naively by just treating everything as strong. This behavior matches what we would expect from a conservative GC, so it looks like the crash could also happen on ToT. On crash/ we sometimes see crashes with 0xFFFFFFFF which I think can be related to such behavior. [1] https://cs.chromium.org/chromium/src/third_party/WebKit/Source/platform/heap/HeapAllocator.h?q=HeapAllocator.h&dr=CSs&l=807 [2] https://cs.chromium.org/chromium/src/third_party/WebKit/Source/platform/heap/HeapAllocator.h?q=HeapAllocator.h&dr=CSs&l=815
,
Apr 3 2018
The in-place marking in HashTable::Trace had the appropriate checks, so this was only a problem when marking through the backing store (which never happened I guess). Introduced in: https://chromium-review.googlesource.com/c/chromium/src/+/923731 Working on tests and fix now.
,
Apr 3 2018
I have double checked all HashTable (only data structure) related callsites for WeakMember and it looks like they all go through the bottlenecks in TraceTraits which also have the according checks for empty or deleted buckets. So it seems this is only an incremental marking issue.
,
Apr 5 2018
This cannot happen anymore as of df7973c46ecc157e63f710177eac3cdb02bc089c because no individual barriers are emitted on deleted values. |
|||
►
Sign in to add a comment |
|||
Comment 1 by mlippautz@chromium.org
, Apr 3 2018