Changeset 191290 in webkit


Ignore:
Timestamp:
Oct 19, 2015, 9:13:59 AM (10 years ago)
Author:
mark.lam@apple.com
Message:

DoubleRep fails to convert SpecBoolean values.
​https://bugs.webkit.org/show_bug.cgi?id=150313

Reviewed by Geoffrey Garen.

This was uncovered by the op_sub stress test on 32-bit builds. On 32-bit builds,
DoubleRep will erroneously convert 'true' to a 'NaN' instead of a double 1.
On 64-bit, the same issue exists but is masked by another bug in DoubleRep where
boolean values will always erroneously trigger a BadType OSR exit.

The erroneous conversion of 'true' to 'NaN' is because the 'true' case in
compileDoubleRep() is missing a jump to the "done" destination. Instead, it
fall through to the "isUndefined" case where it produces a NaN.

The 64-bit erroneous BadType OSR exit is due to the boolean type check being
implemented incorrectly. It was checking if any bits other than bit 0 were set.
However, boolean JS values always have TagBitBool (the 3rd bit) set. Hence, the
check will always fail if we have a boolean value.

This patch fixes both of these issues.

No new test is needed because these issues are already covered by scenarios in
the op_sub.js stress test. This patch also fixes the op_sub.js test to throw an
exception if any failures are encountered (as expected by the stress test
harness). This patch also re-worked the test code to provide more accurate
descriptions of each test scenario for error reporting.

  • dfg/DFGSpeculativeJIT.cpp:

(JSC::DFG::SpeculativeJIT::compileDoubleRep):

  • tests/stress/op_sub.js:

(generateScenarios):
(func):
(initializeTestCases):
(runTest):
(stringify): Deleted.

Location:
trunk/Source/JavaScriptCore
Files:
3 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/ChangeLog

    r191286 r191290  
     12015-10-19  Mark Lam  <mark.lam@apple.com>
     2
     3        DoubleRep fails to convert SpecBoolean values.
     4        https://bugs.webkit.org/show_bug.cgi?id=150313
     5
     6        Reviewed by Geoffrey Garen.
     7
     8        This was uncovered by the op_sub stress test on 32-bit builds.  On 32-bit builds,
     9        DoubleRep will erroneously convert 'true' to a 'NaN' instead of a double 1.
     10        On 64-bit, the same issue exists but is masked by another bug in DoubleRep where
     11        boolean values will always erroneously trigger a BadType OSR exit.
     12
     13        The erroneous conversion of 'true' to 'NaN' is because the 'true' case in
     14        compileDoubleRep() is missing a jump to the "done" destination.  Instead, it
     15        fall through to the "isUndefined" case where it produces a NaN.
     16
     17        The 64-bit erroneous BadType OSR exit is due to the boolean type check being
     18        implemented incorrectly.  It was checking if any bits other than bit 0 were set.
     19        However, boolean JS values always have TagBitBool (the 3rd bit) set.  Hence, the
     20        check will always fail if we have a boolean value.
     21
     22        This patch fixes both of these issues.
     23
     24        No new test is needed because these issues are already covered by scenarios in
     25        the op_sub.js stress test.  This patch also fixes the op_sub.js test to throw an
     26        exception if any failures are encountered (as expected by the stress test
     27        harness).  This patch also re-worked the test code to provide more accurate
     28        descriptions of each test scenario for error reporting.
     29
     30        * dfg/DFGSpeculativeJIT.cpp:
     31        (JSC::DFG::SpeculativeJIT::compileDoubleRep):
     32
     33        * tests/stress/op_sub.js:
     34        (generateScenarios):
     35        (func):
     36        (initializeTestCases):
     37        (runTest):
     38        (stringify): Deleted.
     39
    1402015-10-19  Yusuke Suzuki  <utatane.tea@gmail.com>
    241

  • trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp

    r191224 r191290  
    21972197
    21982198            DFG_TYPE_CHECK(JSValueRegs(op1GPR), node->child1(), ~SpecCell,
    2199                 m_jit.branchTest64(JITCompiler::NonZero, op1GPR, TrustedImm32(static_cast<int32_t>(~1))));
     2199                m_jit.branchTest64(JITCompiler::Zero, op1GPR, TrustedImm32(static_cast<int32_t>(TagBitBool))));
    22002200
    22012201            JITCompiler::Jump isFalse = m_jit.branch64(JITCompiler::Equal, op1GPR, TrustedImm64(ValueFalse));
    22022202            static const double one = 1;
    22032203            m_jit.loadDouble(MacroAssembler::TrustedImmPtr(&one), resultFPR);
     2204            done.append(m_jit.jump());
    22042205            done.append(isFalse);
    22052206
    … …  
    22502251            static const double one = 1;
    22512252            m_jit.loadDouble(MacroAssembler::TrustedImmPtr(&one), resultFPR);
     2253            done.append(m_jit.jump());
    22522254            done.append(isFalse);
    22532255

  • trunk/Source/JavaScriptCore/tests/stress/op_sub.js

    r191224 r191290  
    2525
    2626var set1 = [
    27     o1,
    28     null,
    29     undefined,
    30     NaN,
    31     "abc",
     27    'o1',
     28    'null',
     29    'undefined',
     30    'NaN',
     31    '"abc"',
    3232];
    3333
    3434var set2 = [
    35     10, -10,
    36     2147483647, -2147483647,
    37     4294967296, -4294967296,
    38     100.2, -100.2,
    39     true, false
     35    '10',
     36    '-10',
     37    '2147483647',
     38    '-2147483647',
     39    '4294967296',
     40    '-4294967296',
     41    '100.2',
     42    '-100.2',
     43    'true',
     44    'false',
    4045];
    4146
    … …  
    4752    values.push(set2[i]);
    4853for (var i = 0; i < set2.length; i++)
    49     values.push("" + set2[i]);
    50 
    51 function stringify(value) {
    52     if (typeof value == "string")
    53         return '"' + value + '"';
    54     return "" + value;
    55 }
     54    values.push('"' + set2[i] + '"');
    5655
    5756function generateScenarios(xvalues, yvalues) {
    … …  
    5958    for (var i = 0; i < xvalues.length; i++) {
    6059        for (var j = 0; j < yvalues.length; j++) {
    61             var name = "(" + xvalues[i] + " - " + yvalues[j] + ")";
    62             var x = xvalues[i];
    63             var y = yvalues[j];
    64             var expected = eval(stringify(x) + " - " + stringify(y));
     60            var xStr = xvalues[i];
     61            var yStr = yvalues[j];
     62            var x = eval(xStr);
     63            var y = eval(yStr);
     64            var name = "(" + xStr + " - " + yStr + ")";
     65            var expected = eval("" + xStr + " - " + yStr);
    6566            var scenario = { name: name, x: x, y: y, expected: expected };
    6667
    … …  
    8889        name: "subI32V",
    8990        func: function(x, y) { return 10 - y; },
    90         xvalues: [ 10 ],
     91        xvalues: [ '10' ],
    9192        yvalues: values
    9293    },
    … …  
    9596        func: function(x, y) { return x - 10; },
    9697        xvalues: values,
    97         yvalues: [ 10 ]
     98        yvalues: [ '10' ]
    9899    },
    99100    {
    100101        name: "subI32oV",
    101102        func: function(x, y) { return -2147483647 - y; },
    102         xvalues: [ -2147483647 ],
     103        xvalues: [ '-2147483647' ],
    103104        yvalues: values
    104105    },
    … …  
    107108        func: function(x, y) { return x - 2147483647; },
    108109        xvalues: values,
    109         yvalues: [ 2147483647 ]
     110        yvalues: [ '2147483647' ]
    110111    },
    111112    {
    112113        name: "subI52V",
    113114        func: function(x, y) { return 4294967296 - y; },
    114         xvalues: [ 4294967296 ],
     115        xvalues: [ '4294967296' ],
    115116        yvalues: values
    116117    },
    … …  
    119120        func: function(x, y) { return x - 4294967296; },
    120121        xvalues: values,
    121         yvalues: [ 4294967296 ]
     122        yvalues: [ '4294967296' ]
    122123    },
    123124    {
    124125        name: "subDV",
    125126        func: function(x, y) { return 100.2 - y; },
    126         xvalues: [ 100.2 ],
     127        xvalues: [ '100.2' ],
    127128        yvalues: values
    128129    },
    … …  
    131132        func: function(x, y) { return x - 100.2; },
    132133        xvalues: values,
    133         yvalues: [ 100.2 ]
     134        yvalues: [ '100.2' ]
    134135    },
    135136    {
    136137        name: "subBV",
    137138        func: function(x, y) { return true - y; },
    138         xvalues: [ true ],
     139        xvalues: [ 'true' ],
    139140        yvalues: values
    140141    },
    … …  
    143144        func: function(x, y) { return x - true; },
    144145        xvalues: values,
    145         yvalues: [ true ]
     146        yvalues: [ 'true' ]
    146147    },
    147148    {
    148149        name: "subSi32V",
    149150        func: function(x, y) { return "10" - y; },
    150         xvalues: [ "10" ],
     151        xvalues: [ '"10"' ],
    151152        yvalues: values
    152153    },
    … …  
    155156        func: function(x, y) { return x - "10"; },
    156157        xvalues: values,
    157         yvalues: [ "10" ]
     158        yvalues: [ '"10"' ]
    158159    },
    159160
    … …  
    161162        name: "subSi32oV",
    162163        func: function(x, y) { return "-2147483647" - y; },
    163         xvalues: [ "-2147483647" ],
     164        xvalues: [ '"-2147483647"' ],
    164165        yvalues: values
    165166    },
    … …  
    168169        func: function(x, y) { return x - "2147483647"; },
    169170        xvalues: values,
    170         yvalues: [ "2147483647" ]
     171        yvalues: [ '"2147483647"' ]
    171172    },
    172173    {
    173174        name: "subSi52V",
    174175        func: function(x, y) { return "4294967296" - y; },
    175         xvalues: [ "4294967296" ],
     176        xvalues: [ '"4294967296"' ],
    176177        yvalues: values
    177178    },
    … …  
    180181        func: function(x, y) { return x - "4294967296"; },
    181182        xvalues: values,
    182         yvalues: [ "4294967296" ]
     183        yvalues: [ '"4294967296"' ]
    183184    },
    184185    {
    185186        name: "subSdV",
    186187        func: function(x, y) { return "100.2" - y; },
    187         xvalues: [ "100.2" ],
     188        xvalues: [ '"100.2"' ],
    188189        yvalues: values
    189190    },
    … …  
    192193        func: function(x, y) { return x - "100.2"; },
    193194        xvalues: values,
    194         yvalues: [ "100.2" ]
     195        yvalues: [ '"100.2"' ]
    195196    },
    196197    {
    197198        name: "subSbV",
    198199        func: function(x, y) { return "true" - y; },
    199         xvalues: [ "true" ],
     200        xvalues: [ '"true"' ],
    200201        yvalues: values
    201202    },
    … …  
    204205        func: function(x, y) { return x - "true"; },
    205206        xvalues: values,
    206         yvalues: [ "true" ]
     207        yvalues: [ '"true"' ]
    207208    },
    208209
    … …  
    210211        name: "subSV",
    211212        func: function(x, y) { return "abc" - y; },
    212         xvalues: [ "abc" ],
     213        xvalues: [ '"abc"' ],
    213214        yvalues: values
    214215    },
    … …  
    217218        func: function(x, y) { return x - "abc"; },
    218219        xvalues: values,
    219         yvalues: [ "abc" ]
     220        yvalues: [ '"abc"' ]
    220221    },
    221222    {
    222223        name: "subNV",
    223224        func: function(x, y) { return null - y; },
    224         xvalues: [ null ],
     225        xvalues: [ 'null' ],
    225226        yvalues: values
    226227    },
    … …  
    229230        func: function(x, y) { return x - null; },
    230231        xvalues: values,
    231         yvalues: [ null ]
     232        yvalues: [ 'null' ]
    232233    },
    233234    {
    234235        name: "subOV",
    235236        func: function(x, y) { return o1 - y; },
    236         xvalues: [ o1 ],
     237        xvalues: [ 'o1' ],
    237238        yvalues: values
    238239    },
    … …  
    241242        func: function(x, y) { return x - o1; },
    242243        xvalues: values,
    243         yvalues: [ o1 ]
     244        yvalues: [ 'o1' ]
    244245    },
    245246    {
    246247        name: "subNaNV",
    247248        func: function(x, y) { return NaN - y; },
    248         xvalues: [ NaN ],
     249        xvalues: [ 'NaN' ],
    249250        yvalues: values
    250251    },
    … …  
    253254        func: function(x, y) { return x - NaN; },
    254255        xvalues: values,
    255         yvalues: [ NaN ]
     256        yvalues: [ 'NaN' ]
    256257    },
    257258];
    … …  
    264265}
    265266initializeTestCases();
     267
     268var errorReport = "";
    266269
    267270function runTest(test) {
    … …  
    279282                    continue;
    280283                if (!failedScenario[scenarioID]) {
    281                     print("FAIL: " + test.name + ":" + scenario.name + " started failing on iteration " + i + ": expected " + scenario.expected + ", actual " + result);
     284                    errorReport += "FAIL: " + test.name + ":" + scenario.name + " started failing on iteration " + i + ": expected " + scenario.expected + ", actual " + result + "\n";
    282285                    failedScenario[scenarioID] = scenario;
    283286                }
    … …  
    285288        }
    286289    } catch(e) {
    287         print("Unexpected exception: " + e);
     290        errorReport += "Unexpected exception: " + e + "\n";
    288291    }
    289292}
    … …  
    292295    runTest(test);
    293296
     297if (errorReport !== "")
     298    throw "Error: bad result:\n" + errorReport;
Note: See TracChangeset for help on using the changeset viewer.