Bug 69721 - JSVALUE32_64 DFG JIT - Bug fix for ConvertThis
Summary: JSVALUE32_64 DFG JIT - Bug fix for ConvertThis
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-10-09 04:40 PDT by Yuqiang Xian
Modified: 2011-10-09 21:37 PDT (History)
3 users (show)

See Also:


Attachments
the patch (1.63 KB, patch)
2011-10-09 04:45 PDT, Yuqiang Xian
fpizlo: review-
fpizlo: commit-queue-
Details | Formatted Diff | Diff
the patch (1.76 KB, patch)
2011-10-09 18:20 PDT, Yuqiang Xian
darin: review+
darin: commit-queue-
Details | Formatted Diff | Diff
patch addressing Darin's comments (1.77 KB, patch)
2011-10-09 19:38 PDT, Yuqiang Xian
barraclough: review+
barraclough: commit-queue+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yuqiang Xian 2011-10-09 04:40:52 PDT
Patch soon.
Comment 1 Yuqiang Xian 2011-10-09 04:45:04 PDT
Created attachment 110298 [details]
the patch
Comment 2 Filip Pizlo 2011-10-09 12:09:58 PDT
Comment on attachment 110298 [details]
the patch

This doesn't look right.  I think you should be catching the case where thisValueTagGPR holds NullTag.  I think that's what this code was trying to do (and failing).
Comment 3 Yuqiang Xian 2011-10-09 18:20:35 PDT
Created attachment 110314 [details]
the patch

oh...
Comment 4 Darin Adler 2011-10-09 19:29:08 PDT
Comment on attachment 110314 [details]
the patch

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

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:1762
> +            ASSERT((JSValue::UndefinedTag + 1 == JSValue::NullTag) && (JSValue::NullTag & 0x1));

As I mentioned in the other bug, this assertion should be:

    COMPILE_ASSERT((JSValue::UndefinedTag | 1) == JSValue::NullTag);
Comment 5 Yuqiang Xian 2011-10-09 19:38:43 PDT
Created attachment 110318 [details]
patch addressing Darin's comments
Comment 6 WebKit Review Bot 2011-10-09 20:10:17 PDT
Comment on attachment 110318 [details]
patch addressing Darin's comments

Clearing flags on attachment: 110318

Committed r97037: <http://trac.webkit.org/changeset/97037>
Comment 7 WebKit Review Bot 2011-10-09 20:10:22 PDT
All reviewed patches have been landed.  Closing bug.