[Webkit-unassigned] [Bug 216172] [JSC] Use symbols as identifiers for class fields computed names storage

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=216172

--- Comment #8 from Caio Lima <ticaiolima at gmail.com> ---
Comment on attachment 411746
  --> https://bugs.webkit.org/attachment.cgi?id=411746
Use symbols as identifiers, v4

View in context: https://bugs.webkit.org/attachment.cgi?id=411746&action=review

> Source/JavaScriptCore/parser/Parser.cpp:2849
> +    auto symbol = Symbol::create(vm, vm.symbolRegistry().privateSymbolForKey(symbolName));

After thinking this in more depth, I'm uncomfortable with the fact we have a short lived heap object to return the uid here. This seems a bad choice for this API, since for a fresh created `SymbolImpl`, the only thing that is handling its ref is the Symbol we created. It means that if GC runs after `getOrCreateSymbolForComputedName`  and before we ever reference SymbolImpl somewhere else, we will end up with an invalid pointer that can cause UAF issues. I think it's not correct relying that such scenario won't happen. Is there a particular reason to have such function as a primitive? 

It sounds more correct if we have a less fine grain API to create/retrieve private identifier on `identifierArena` called `makePrivateIdentifier(VM& vm, unsigned identifier)`. This way, we would be able to avoid a heap allocation (removing `Symbol::create`) and have full control of SymbolImpl lifetime.

> Source/WTF/wtf/text/SymbolRegistry.h:91
> +    HashSet<SymbolRegistryKey> m_privateTable;

I don't think we need to include a `m_privateTable` on SymbolRegistry. I think this concept of private/public is more on the client side of this API than on SymbolRegistry itself. The alternative I'd propose here is to have a new SymbolRegistry on `VM` that store private symbols. This way we would be achieving the same goal we want with current `SymbolRegistry` API we have.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20201020/3dbc2998/attachment-0001.htm>