Bug 138476 - slow_path_get_direct_pname() needs to be hardened against a constant baseValue
Summary: slow_path_get_direct_pname() needs to be hardened against a constant baseValue
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Lam
URL:
Keywords: InRadar
Depends on: 138483
Blocks:
  Show dependency treegraph
 
Reported: 2014-11-06 14:53 PST by Mark Lam
Modified: 2014-11-07 00:24 PST (History)
8 users (show)

See Also:


Attachments
the patch. (20.88 KB, patch)
2014-11-06 15:21 PST, Mark Lam
msaboff: review-
Details | Formatted Diff | Diff
take 2: minus test refactoring (9.79 KB, patch)
2014-11-06 16:06 PST, Mark Lam
msaboff: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Lam 2014-11-06 14:53:26 PST
CommonSlowPath's slow_path_get_direct_pname() currently assumes that the baseValue is always a non-constant register.  However, this is not always the case like in the following:

    function foo() {
        var o = { a:1 };
        for (var n in o)
            0[n];
    };
    foo();

<rdar://problem/18495847>
Comment 1 Mark Lam 2014-11-06 15:21:13 PST
Created attachment 241135 [details]
the patch.
Comment 2 Michael Saboff 2014-11-06 15:35:50 PST
Comment on attachment 241135 [details]
the patch.

The code change looks fine.  The test changes are change to the test beyond just adding cases for the fix.  Therefore please break out the test enhancements as a separate patch.
Comment 3 Mark Lam 2014-11-06 16:06:30 PST
Created attachment 241142 [details]
take 2: minus test refactoring
Comment 4 Michael Saboff 2014-11-06 16:15:13 PST
Comment on attachment 241142 [details]
take 2: minus test refactoring

r=me
Comment 5 Mark Lam 2014-11-06 16:19:35 PST
Thanks for the review.  Landed in r175724: <http://trac.webkit.org/r175724>.
Comment 6 Simon Fraser (smfr) 2014-11-06 20:39:40 PST
It looks like this broke 3 JSC tests:
https://build.webkit.org/builders/Apple%20Yosemite%2032-bit%20JSC%20%28BuildAndTest%29?numbuilds=100
Comment 7 Mark Lam 2014-11-06 20:48:36 PST
(In reply to comment #6)
> It looks like this broke 3 JSC tests:
> https://build.webkit.org/builders/Apple%20Yosemite%2032-
> bit%20JSC%20%28BuildAndTest%29?numbuilds=100

Iā€™m looking into it.
Comment 8 Mark Lam 2014-11-07 00:24:56 PST
(In reply to comment #6)
> It looks like this broke 3 JSC tests:
> https://build.webkit.org/builders/Apple%20Yosemite%2032-
> bit%20JSC%20%28BuildAndTest%29?numbuilds=100

The issue is due to a latent bug that is uncovered by the new tests.  I'll skip the tests (from the JSC test set) temporarily while I debug the issue.  This work is tracked in https://bugs.webkit.org/show_bug.cgi?id=138497.