Bug 54387 - V8 3.1.4 changed value of Math.LOG10E
Summary: V8 3.1.4 changed value of Math.LOG10E
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-02-14 04:53 PST by Søren Gjesse
Modified: 2011-02-24 03:13 PST (History)
5 users (show)

See Also:


Attachments
Patch (3.25 KB, patch)
2011-02-15 04:55 PST, Søren Gjesse
no flags Details | Formatted Diff | Diff
Patch (4.74 KB, patch)
2011-02-17 03:55 PST, Søren Gjesse
no flags Details | Formatted Diff | Diff
Patch (4.82 KB, patch)
2011-02-21 14:18 PST, Søren Gjesse
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Søren Gjesse 2011-02-14 04:53:19 PST
Math.LOG10E is now 0.4342944819032518, which matches Firefox and IE.

See http://code.google.com/p/chromium/issues/detail?id=72555

Will update LayoutTests/platform/chromium/test_expectations.txt when V8 3.1.4 has landed in chromium.
Comment 1 Søren Gjesse 2011-02-14 04:55:40 PST
The layout test failing due to the change to Math.LOG10E is fast/js/kde/math.html.
Comment 2 Peter Kasting 2011-02-14 18:30:18 PST
You added an override in the Chromium-side src/webkit/tools/layout_tests/test_expectations.txt file.  Please move that to the WebKit side or fix the test or its expectations as soon as possible.

Thanks!
Comment 3 Søren Gjesse 2011-02-15 04:55:32 PST
Created attachment 82439 [details]
Patch
Comment 4 Peter Kasting 2011-02-15 11:52:49 PST
This patch is incomplete.  In order for this to work, the test will have to be changed in order to expect a different value on V8 versus JSC.
Comment 5 Søren Gjesse 2011-02-16 01:37:38 PST
The patch adds the file

  LayoutTests/platform/chromium/fast/js/kde/math-expected.txt

which only differs from

  LayoutTests/fast/js/kde/math-expected.txt

in the result of Math.LOG10E is

  0.4342944819032518

in the first and

  0.43429448190325176

in the second.

Should that not cover the difference in the value of Math.LOG10E between V8 and JSC?

There is another bug on this, https://bugs.webkit.org/show_bug.cgi?id=41033. According to ES5 section 15.8.1.5 LOG10E has a value of approximately 0.4342944819032518.
Comment 6 Peter Kasting 2011-02-16 10:19:38 PST
(In reply to comment #5)
> The patch adds the file
> 
>   LayoutTests/platform/chromium/fast/js/kde/math-expected.txt
> 
> which only differs from
> 
>   LayoutTests/fast/js/kde/math-expected.txt
> 
> in the result of Math.LOG10E is
> 
>   0.4342944819032518
> 
> in the first and
> 
>   0.43429448190325176
> 
> in the second.
> 
> Should that not cover the difference in the value of Math.LOG10E between V8 and JSC?

No, because the test explicitly checks the value against 0.43429448190325176.  That's why the current output is "FAIL" + explanation instead of the output you attached.  Please see the actual test source code at http://trac.webkit.org/browser/trunk/LayoutTests/fast/js/kde/script-tests/math.js.

The best patch might be one that changes the test to expect the more-correct value, changes the global expectation to expect failure, and adds a Chromium expectation identical to the one you supplied.
Comment 7 Søren Gjesse 2011-02-17 03:55:01 PST
Created attachment 82782 [details]
Patch
Comment 8 Søren Gjesse 2011-02-17 03:57:02 PST
Sorry for making such a bogus patch. I have now uploaded a patch where both the JSC and the V8 value of LOF10E are present and with different expectations.
Comment 9 Peter Kasting 2011-02-17 12:57:50 PST
Comment on attachment 82782 [details]
Patch

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

> LayoutTests/fast/js/kde/script-tests/math.js:17
> +// JSC and V8 has different values for Math.LOG10E

Nit: has -> return.  You might want to add to the end that this is the more accurate value.

> LayoutTests/fast/js/kde/script-tests/math.js:19
>  shouldBe("String()+Math.LOG10E", "'0.43429448190325176'");

Please remove this line.  We should only expect one result.  It is OK for JSC to have an expected-fail result for just the first value.
Comment 10 Eric Seidel (no email) 2011-02-18 13:10:10 PST
JSC folks might want to see this go by.  Seems JSC might want to change here too.
Comment 11 Søren Gjesse 2011-02-21 14:18:55 PST
Created attachment 83214 [details]
Patch
Comment 12 Kent Tamura 2011-02-21 15:27:46 PST
http://trac.webkit.org/changeset/79246
JSC's value has been changed.  Is this patch needed?
Comment 13 Peter Kasting 2011-02-21 15:48:44 PST
(In reply to comment #12)
> http://trac.webkit.org/changeset/79246
> JSC's value has been changed.  Is this patch needed?

No.  Someone should fix the Chromium expectations to note that we expect to pass this test.
Comment 14 Ryosuke Niwa 2011-02-21 17:14:39 PST
http://trac.webkit.org/changeset/79246/ aligned its behavior to V8.