[JSC] Do not pass Symbol::privateName() directly to PropertyName without ref-ing it
<rdar://problem/48806130>
Created attachment 364768 [details] Patch
Created attachment 364770 [details] Patch
Comment on attachment 364770 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=364770&action=review r=me with fixes. > Source/JavaScriptCore/ChangeLog:19 > + PropertyName can be accidentally destroyed in the middle of putByVal operation. We should retain PrivateName /middle of putByVal/middle of the putByVal/ > Source/JavaScriptCore/ChangeLog:37 > + 3. We audit similar functions `toPropertyKey(exec)` and `toIdentifier(exec)` necessary exception checks. /necessary exception checks/for needed but missing exception checks/. > Source/JavaScriptCore/runtime/JSONObject.cpp:250 > + auto propertyName = name.toString(exec)->toIdentifier(exec); name.toString() can also throw an exception because it can allocate a string from a name that is a number. So, you'll need another exception check before the toIdentifier() conversion.
Comment on attachment 364770 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=364770&action=review >> Source/JavaScriptCore/ChangeLog:19 >> + PropertyName can be accidentally destroyed in the middle of putByVal operation. We should retain PrivateName > > /middle of putByVal/middle of the putByVal/ Fixed. >> Source/JavaScriptCore/ChangeLog:37 >> + 3. We audit similar functions `toPropertyKey(exec)` and `toIdentifier(exec)` necessary exception checks. > > /necessary exception checks/for needed but missing exception checks/. Fixed. >> Source/JavaScriptCore/runtime/JSONObject.cpp:250 >> + auto propertyName = name.toString(exec)->toIdentifier(exec); > > name.toString() can also throw an exception because it can allocate a string from a name that is a number. So, you'll need another exception check before the toIdentifier() conversion. Nice catch. Fixed.
Committed r242991: <https://trac.webkit.org/changeset/242991>
Comment on attachment 364770 [details] Patch LGTM too. Thanks a lot for solving this bug.
Added a missing exception check in r242999: <http://trac.webkit.org/r242999>.
Committed r243191: <https://trac.webkit.org/changeset/243191>