Bug 154416

Summary: JSObject::getPropertySlot - index-as-propertyname, override on prototype, & shadow
Product: WebKit Reporter: Gavin Barraclough <barraclough>
Component: JavaScriptCoreAssignee: Gavin Barraclough <barraclough>
Status: RESOLVED FIXED    
Severity: Normal CC: buildbot, commit-queue, keith_miller, mark.lam, msaboff, rniwa, saam
Priority: P2    
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Fix
none
Fix
buildbot: commit-queue-
Archive of layout-test-results from ews101 for mac-yosemite
none
Archive of layout-test-results from ews105 for mac-yosemite-wk2
none
Archive of layout-test-results from ews113 for mac-yosemite
none
Fix
buildbot: commit-queue-
Archive of layout-test-results from ews107 for mac-yosemite-wk2
none
Archive of layout-test-results from ews103 for mac-yosemite
none
Archive of layout-test-results from ews116 for mac-yosemite
none
Fix ggaren: review+

Description Gavin Barraclough 2016-02-18 14:17:22 PST
Here's the bug. Suppose you call JSObject::getOwnProperty and:
 * PropertyName contains an index
 * An object on the prototype chain overrides getOwnPropertySlot, and has that index property
 * The base of the access (or another object on the prototype chain) shadows that property

JSObject::getPropertySlot is written assuming the common case is that propertyName is not an
index, and as such walks up the prototype chain looking for non-index properties before it
tries calling parseIndex.

At the point we reach an object on the prototype chain overriding getOwnPropertySlot (which
would potentially return the property) we may have already skipped over non-overriding
objects that contain the property in index storage.
Comment 1 Gavin Barraclough 2016-02-18 14:27:20 PST
Created attachment 271696 [details]
Fix
Comment 2 Gavin Barraclough 2016-02-18 14:44:55 PST
Created attachment 271697 [details]
Fix
Comment 3 WebKit Commit Bot 2016-02-18 14:46:12 PST
Attachment 271697 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/ChangeLog:9:  Need whitespace between colon and description  [changelog/filechangedescriptionwhitespace] [5]
ERROR: Source/JavaScriptCore/ChangeLog:10:  Need whitespace between colon and description  [changelog/filechangedescriptionwhitespace] [5]
ERROR: Source/JavaScriptCore/ChangeLog:11:  Need whitespace between colon and description  [changelog/filechangedescriptionwhitespace] [5]
ERROR: Source/JavaScriptCore/runtime/JSObject.h:861:  The parameter name "propertyName" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 4 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Build Bot 2016-02-18 15:40:15 PST
Comment on attachment 271697 [details]
Fix

Attachment 271697 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/851606

New failing tests:
streams/reference-implementation/abstract-ops.html
Comment 5 Build Bot 2016-02-18 15:40:18 PST
Created attachment 271704 [details]
Archive of layout-test-results from ews101 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 6 Build Bot 2016-02-18 15:43:03 PST
Comment on attachment 271697 [details]
Fix

Attachment 271697 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/851608

New failing tests:
streams/reference-implementation/abstract-ops.html
Comment 7 Build Bot 2016-02-18 15:43:05 PST
Created attachment 271706 [details]
Archive of layout-test-results from ews105 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5
Comment 8 Build Bot 2016-02-18 15:52:16 PST
Comment on attachment 271697 [details]
Fix

Attachment 271697 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/851626

New failing tests:
streams/reference-implementation/abstract-ops.html
Comment 9 Build Bot 2016-02-18 15:52:19 PST
Created attachment 271708 [details]
Archive of layout-test-results from ews113 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews113  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 10 Gavin Barraclough 2016-02-19 11:20:47 PST
Created attachment 271766 [details]
Fix
Comment 11 Build Bot 2016-02-19 12:16:51 PST
Comment on attachment 271766 [details]
Fix

Attachment 271766 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/855539

Number of test failures exceeded the failure limit.
Comment 12 Build Bot 2016-02-19 12:16:55 PST
Created attachment 271774 [details]
Archive of layout-test-results from ews107 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5
Comment 13 Build Bot 2016-02-19 12:18:01 PST
Comment on attachment 271766 [details]
Fix

Attachment 271766 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/855552

Number of test failures exceeded the failure limit.
Comment 14 Build Bot 2016-02-19 12:18:04 PST
Created attachment 271775 [details]
Archive of layout-test-results from ews103 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 15 Build Bot 2016-02-19 12:30:56 PST
Comment on attachment 271766 [details]
Fix

Attachment 271766 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/855569

Number of test failures exceeded the failure limit.
Comment 16 Build Bot 2016-02-19 12:31:00 PST
Created attachment 271777 [details]
Archive of layout-test-results from ews116 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews116  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 17 Gavin Barraclough 2016-02-19 16:46:29 PST
Created attachment 271819 [details]
Fix
Comment 18 Geoffrey Garen 2016-02-19 17:26:16 PST
Comment on attachment 271819 [details]
Fix

r=me
Comment 19 Gavin Barraclough 2016-02-19 17:51:54 PST
Committed revision 196849.