RESOLVED FIXED 119343
Some cleanup in JSValue::get
https://bugs.webkit.org/show_bug.cgi?id=119343
Summary Some cleanup in JSValue::get
Gavin Barraclough
Reported 2013-07-31 10:44:20 PDT
JSValue::get is implemented to: 1) Check if the value is a cell – if not, synthesize a prototype to search, 2) call getOwnPropertySlot on the cell, 3) if this returns false, cast to JSObject to get the prototype, and walk the prototype chain. By all rights this should crash when passed a string and accessing a property that does not exist, because the string is a cell, getOwnPropertySlot should return false, and the cast to JSObject should be unsafe. To work around this, JSString::getOwnPropertySlot actually implements 'get' functionality - searching the prototype chain, and faking out a return value of undefined if no property is found. This is a huge hazard, since fixing JSString::getOwnPropertySlot or calling getOwnPropertySlot on cells from elsewhere would introduce bugs. Fortunately it is only ever called in this one place. The fix here is to move getOwnPropertySlot onto JSObjecte and end this madness - cells don't have property slots anyway.
Attachments
fix (93.13 KB, patch)
2013-07-31 10:48 PDT, Gavin Barraclough
ggaren: review+
buildbot: commit-queue-
Gavin Barraclough
Comment 1 2013-07-31 10:48:44 PDT
Geoffrey Garen
Comment 2 2013-07-31 10:53:23 PDT
Comment on attachment 207862 [details] fix View in context: https://bugs.webkit.org/attachment.cgi?id=207862&action=review r=me > Source/JavaScriptCore/llint/LLIntSlowPaths.cpp:675 > - > + Please revert.
Build Bot
Comment 3 2013-07-31 11:27:49 PDT
Gavin Barraclough
Comment 4 2013-07-31 13:43:43 PDT
Fixed in r153532, r153537.
Simon Fraser (smfr)
Comment 5 2013-07-31 18:44:39 PDT
Mark Lam
Comment 6 2013-08-01 14:05:56 PDT
(In reply to comment #5) > This broke bindings generation tests; please fix: > http://build.webkit.org/builders/Apple%20MountainLion%20Release%20WK1%20%28Tests%29?numbuilds=200 The fix for the bindings test is at https://bugs.webkit.org/show_bug.cgi?id=119410.
Note You need to log in before you can comment on or make changes to this bug.