Bug 68141 - [n]stricteq code is bogus in JSValue32_64 JIT
Summary: [n]stricteq code is bogus in JSValue32_64 JIT
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
Depends on:
Reported: 2011-09-14 21:50 PDT by Gavin Barraclough
Modified: 2011-09-14 22:16 PDT (History)
1 user (show)

See Also:

The patch (3.36 KB, patch)
2011-09-14 21:52 PDT, Gavin Barraclough
sam: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Gavin Barraclough 2011-09-14 21:50:45 PDT
The code tries to check for both ints or cells, but this check also catches cases where values that are undefined, null, etc (probably was incorrectly assuming cell was the 2nd highest tag?).
Also, there is no need not to handle int on the fast path.
stricteq is just a case of comparing the payloads, if we:

* handle cases of differing tags on a slow path
* handle doubles a slow path
* handle both-are-string on a slow path
Comment 1 Gavin Barraclough 2011-09-14 21:52:32 PDT
Created attachment 107453 [details]
The patch
Comment 2 WebKit Review Bot 2011-09-14 21:55:26 PDT
Attachment 107453 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1

Source/JavaScriptCore/ChangeLog:14:  Need whitespace between colon and description  [changelog/filechangedescriptionwhitespace] [5]
Source/JavaScriptCore/ChangeLog:15:  Need whitespace between colon and description  [changelog/filechangedescriptionwhitespace] [5]
Source/JavaScriptCore/ChangeLog:16:  Need whitespace between colon and description  [changelog/filechangedescriptionwhitespace] [5]
Total errors found: 3 in 2 files

If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Sam Weinig 2011-09-14 21:56:28 PDT
Comment on attachment 107453 [details]
The patch

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

> Source/JavaScriptCore/jit/JITOpcodes32_64.cpp:1011
> +    // Simply compare the patyloads.

No patyloads here! Tyop!
Comment 4 Gavin Barraclough 2011-09-14 22:16:27 PDT
Fixed in r95168