Closed Bug 531915 Opened 15 years ago Closed 3 years ago

Floating point differences between platforms

Categories

(Core :: JavaScript Engine, defect)

Product:
Core
Component:
JavaScript Engine
Platform:
x86
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
93 Branch
Tracking Flags:
Tracking Status
firefox-esr91 --- fixed
firefox93 --- fixed

People

(Reporter: philip, Assigned: me)

References

(Blocks 1 open bug, Regressed 1 open bug)

Details

(Whiteboard: [fingerprinting])

Attachments

(10 files, 3 obsolete files)

Bug 531915 - part 1 - update fdlibm to import files needed for sin, cos, and tan r?tjr
(deleted), text/x-phabricator-request
RyanVM
: approval-mozilla-esr91+
Details
Bug 531915 - part 2 - update fdlibm local patches 4, 8, and 9 r?tjr
(deleted), text/x-phabricator-request
RyanVM
: approval-mozilla-esr91+
Details
Bug 531915 - part 3 - add back needed rounding helpers and emulate freebsd internal double types ?tjr
(deleted), text/x-phabricator-request
RyanVM
: approval-mozilla-esr91+
Details
Bug 531915 - part 4 - rem pio2 fixups r?tjr
(deleted), text/x-phabricator-request
RyanVM
: approval-mozilla-esr91+
Details
Bug 531915 - part 5 - build and export sin, cos, and tan from fdlibm r?tjr
(deleted), text/x-phabricator-request
RyanVM
: approval-mozilla-esr91+
Details
Bug 531915 - part 6 - optionally use fdlibm's sin, cos, and tan in jsmath r?tjr
(deleted), text/x-phabricator-request
RyanVM
: approval-mozilla-esr91+
Details
graph for benchmarks
(deleted), text/html
Details
graph for kraken
(deleted), text/html
Details
graph for kraken linux subtests
(deleted), text/html
Details
graph for kraken with twice as many linux tests
(deleted), text/html
Details
With a 32-bit Linux build of SM tip: js> Math.exp(10) 22026.465794806725 js> Math.sin(Math.PI) 1.2246063538223773e-16 js> Math.cos(Math.PI/2) 6.123031769111886e-17 js> Math.tan(Math.PI) -1.2246063538223773e-16 js> Math.atan2(1e-310, 2) 5e-311 js> Math.pow(Math.PI, -100) 1.9275814160560332e-50 With a 64-bit Linux build, all the answers are slightly different: js> Math.exp(10) 22026.465794806718 js> Math.sin(Math.PI) 1.2246467991473532e-16 js> Math.cos(Math.PI/2) 6.123233995736766e-17 js> Math.tan(Math.PI) -1.2246467991473532e-16 js> Math.atan2(1e-310, 2) 4.9999999999997e-311 js> Math.pow(Math.PI, -100) 1.9275814160560206e-50 Also I see some bigger differences between Linux and Windows (testing in Firefox 3.5 on both, since I don't have a convenient newer SpiderMonkey on Windows): Math.tan(-1e300) = -4.802497308567303 (Linux), -4.987183803371025 (Windows) (plus similar things with other trig functions) 1/((-0) % 1.5) = -Infinity (Linux), Infinity (Windows) I'd personally like (for embedding) to have identical output for all conceivable inputs across 32- and 64-bit Linux, Windows and OS X (but I don't really care what output, and I could just delete some of the functions if necessary).
Adding our resident IEEE double gurus. The differences look like permitted rounding differences to me (we use the platform's FPU hardware, and its allowed to produce a 1-bit difference as per spec). Waldo, do you concur?
(In reply to comment #1) > 1/((-0) % 1.5) = -Infinity (Linux), Infinity (Windows) That looks like a bug in js_fmod from jslibmath.h
With respect to the function off-by-one differences: ECMAScript never required precise behavior for any of these functions except for when given certain mathematically exact inputs or by specifying some quality of the result when given inputs comparing in a precise way to some mathematically exact boundary (mathematically exact inputs/boundaries: +0, -0, 1, -1, infinities, NaN, etc. but not Math.PI or similar approximate values). None of these inputs here are exact in the necessary manner, so our behavior is permitted by ES3/ES5. It would be nice to have exactly consistent results across architectures, but unless we revert back to a fdlibm-style implementation we use everywhere, I don't see it happening. We're not going to munge FP mode so as to avoid or ensure extra precision for a least-significant digit difference.
I agree with comment 2. (zero % finite) is clearly specified as zero for either zero value.
Yeah looking at #2 right now.
1/((-0) % 1.5) is -Infinity on my windows build with msvc8. Philip what version of ff did you use for testing this?
I was using Firefox 3.5.3. But I just built a new version of the shell from Hg on Windows and can't reproduce the problem - I guess it was fixed in bug 503595, so that's okay. The sin/cos/tan cases are not just least-significant-bit differences in the output, they differ around the 6th decimal digit. I accept they're not violating specs and are not behaving unreasonably, though.
We use libm these days. We used to use fdlibm for cross-platform consistency but it was slow compared to OS library implementations, and the code was crufty. Are the sin/cos/tan differences legit? I crave a numerics-lawyer's opinion ;-). /be
Hm, non-MSB differences are more sadmaking. I don't think we should actually do anything about it for 1e-6-order differences, tho, at least not unless some more satisfactory solution than shipping our own unoptimized libm functions is possible.
Assignee: general → nobody
Whiteboard: [fingerprinting]

:cpeterson fyi:

since FF68

  • windows: four results (two more yet to be added). Note: Tor Browser has there own fingerprint of two results as they use mingw)
  • android: at least seven results
  • linux: two results so far (one more I know of yet to be added)
  • mac: one result

chromium on the other hand is the same across all platforms AFAIK

Flags: needinfo?(cpeterson)

Thanks for the links! Using your example code, I can reproduce the differences in 32-bit and 64-bit Firefox builds on Windows. I mistakenly thought these architecture differences were nullified when Firefox switched to fdlibm in bug 933257.

I was also just reading this related Tor bug: "Math routines are OS fingerprintable" https://gitlab.torproject.org/legacy/trac/-/issues/13018

Flags: needinfo?(cpeterson)

swapping libm libraries should also nullifies entropy in audio fingerprinting - ask Paul Adenot: this has been discussed upstream as a standard
[1] https://gitlab.torproject.org/tpo/applications/tor-browser/-/issues/13017#note_2615311

Assignee: nobody → me
Status: NEW → ASSIGNED
Attachment #9230279 - Attachment is obsolete: true
Attachment #9230280 - Attachment is obsolete: true

Depends on D119420

Depends on D119423

Depends on D119424

Depends on D119425

Depends on D119426

Thank you for working on this!

Have you checked the performance impact by switching to fdlibm sin/cos/tan? (see bug 933257 comment #139 and bug 933257 comment #159 for the previous performance impact)

Flags: needinfo?(me)

(In reply to Tooru Fujisawa [:arai] from comment #22)

Have you checked the performance impact by switching to fdlibm sin/cos/tan? (see bug 933257 comment #139 and bug 933257 comment #159 for the previous performance impact)

We did a few basic experiments early on, like we saw a 10% slowdown on the box2d benchmark in Jetstream. But, now that these patches are beginning to take shape, we are going to do more benchmarking.

Thanks for the references to the comments, I will keep the ni to redo those experiments and post the data here.

sin/cos/tan changes alone won't solve all math entropy, but would seem to nail most of them. There are cbrt, cosh, expm1 and sinh polyfills that have entropy, so assuming the patch works, we'd need a follow up bug

Attachment #9230310 - Attachment description: Bug 531915 - part 7 - add jstest to verify math fingerprint r?tjr → Bug 531915 - part 7 - add browser test to verify math fingerprint r?tjr
Attachment #9230310 - Attachment is obsolete: true
Attachment #9230306 - Attachment description: Bug 531915 - part 3 - create local patch 20 to add back needed rounding helpers r?tjr → WIP: Bug 531915 - part 3 - add back needed rounding helpers and emulate freebsd internal double types ?tjr
Attachment #9230309 - Attachment description: Bug 531915 - part 6 - use fdlibm's sin, cos, and tan in jsmath r?tjr → Bug 531915 - part 6 - optionally use fdlibm's sin, cos, and tan in jsmath r?tjr
Attachment #9230306 - Attachment description: WIP: Bug 531915 - part 3 - add back needed rounding helpers and emulate freebsd internal double types ?tjr → Bug 531915 - part 3 - add back needed rounding helpers and emulate freebsd internal double types ?tjr

What is our actual fingerprinting breakdown here? Is it more than just Win/Mac/Lin/Android?

What is our actual fingerprinting breakdown here? Is it more than just Win/Mac/Lin/Android?

There are differences within each: such as linux distros can differ, android has at least seven results, windows at least four (not counting mingw vs winsdk which should only apply to Tor Browser)

(In reply to sanketh from comment #23)

(In reply to Tooru Fujisawa [:arai] from comment #22)

Have you checked the performance impact by switching to fdlibm sin/cos/tan? (see bug 933257 comment #139 and bug 933257 comment #159 for the previous performance impact)

We did a few basic experiments early on, like we saw a 10% slowdown on the box2d benchmark in Jetstream. But, now that these patches are beginning to take shape, we are going to do more benchmarking.

Thanks for the references to the comments, I will keep the ni to redo those experiments and post the data here.

We did more experiments and the data is included in the email to dev-platform. Following the discussion there and offline, we might do more experiments, and we'll post the details here when we have it.

Flags: needinfo?(me)

(In reply to Simon Mainey from comment #24)

sin/cos/tan changes alone won't solve all math entropy, but would seem to nail most of them. There are cbrt, cosh, expm1 and sinh polyfills that have entropy, so assuming the patch works, we'd need a follow up bug

(Sorry for the late response.) I don't think I follow. Could you expand on how polyfills have entropy? Does the same polyfill on different platforms give different results (which might indicate that some underlying primitive, like say pow, is not the same across machines) or something else?

Flags: needinfo?(simon.mainey)

(In reply to sanketh from comment #28)

Does the same polyfill on different platforms give different results

Different devices it seems, and limited to Android. Look at [1] https://arkenfox.github.io/TZP/tests/mathdata.html and scan the Firefox results F20 to F26 and spot all the blue p1's. These are items that differed from the control test (which was a windows Firefox), not a direct comparison to each other. But you can see the results of each polyfill per Android result, they are clearly not all the same

At the top, see the 55 items that differ (so far) in Firefox tests, the polyfill ones are color coded to stand out with a p1 (ignore the number, that was in case multiple polyfills were used per function). The very first item listed is cbrt 1p1 - cbrt test array item 1 using polyfill. At [2] https://arkenfox.github.io/TZP/tests/math.html , scroll down to data, you can see that cbrt test 1 uses Math.PI

polyfill code at [2] https://github.com/arkenfox/TZP/blob/d7b77267890483c4e111500fc2c3b7c0a7258c6d/tests/math.html#L321

  • crbt uses pow, and the test cbrt1 uses Math.PI
  • cosh, expm1, sinh all exp - and the tests in question all use the value 1

So I guess that kind of simplifies that up a bit. Android only with Math.PI + pow, and exp + 1. Polyfill entropy between desktops vs Android (which we could live with), and the entropy within Android is a different issue (like how Chromium's audio hash can change with a kernel update). I would ignore this and follow up in a different issue once this one is solved

Flags: needinfo?(simon.mainey)
Blocks: 1722181

Simon, (In reply to Simon Mainey from comment #29)

[...]

Thank you for the detailed response. As suggested, I created a new bug Bug 1722181 and we can continue the discussion there once this is resolved.

Following the response to the email to dev-platform, we did further benchmarking using a shippable build (we were using a opt build for the original numbers) and the results show smaller slowdowns.

Method

We benchmarked the patch with the javascript.options.use_fdlibm_for_sin_cos_tan set to true ("fdlibm on") and false ("fdlibm off") on three OSes (OS X, Linux, and Windows) and compared it to moz-central ("current"). We tested on JetStream's Box2d, Kraken's Audio DFT, and SunSpider's Math Partial Sums which aim to simulate real-world workloads and on arai’s microbenchmark of concerning values to infer a fine-grained performance impact.

The data was collected from the following try runs:

Three Benchmarks

On OS X and Linux, we see a small speedup across the board with the average speedup being 2.2% and 3.7% respectively.

On Windows, the impact is less uniform, we see a 26% speedup on Kraken Audio DFT, 7.1% slowdown on SunSpider Math Partial Sums, and 0.8% slowdown on JetStream Box2D.

Arai's Microbenchmark

On OS X and Linux, we see speedups for some tasks and slowdowns for others, with an average speedup of 6.4% and 3.7% respectively.

On Windows, we see slowdowns across the board with an average slowdown of 18%.

I couldn't figure out how to attach images to a bugzilla comment 😬, so I put them on a webpage: https://snkth.com/browser-fingerprinting/fdlibm-vs-native/

Pushed by arai_a@mac.com: https://hg.mozilla.org/integration/autoland/rev/14c95ef3dc44 part 1 - update fdlibm to import files needed for sin, cos, and tan r=arai https://hg.mozilla.org/integration/autoland/rev/5f15204ccca9 part 2 - update fdlibm local patches 4, 8, and 9 r=arai https://hg.mozilla.org/integration/autoland/rev/9ec88b43d7a1 part 3 - add back needed rounding helpers and emulate freebsd internal double types ?tjr r=arai https://hg.mozilla.org/integration/autoland/rev/a4824d5f29fd part 4 - rem pio2 fixups r=arai https://hg.mozilla.org/integration/autoland/rev/c9d70e46dfca part 5 - build and export sin, cos, and tan from fdlibm r=arai https://hg.mozilla.org/integration/autoland/rev/65cf5929d5f2 part 6 - optionally use fdlibm's sin, cos, and tan in jsmath r=tjr,arai

Iulian, thank you so much for the links!

Flags: needinfo?(me)
Attachment #9230306 - Attachment description: Bug 531915 - part 3 - add back needed rounding helpers and emulate freebsd internal double types ?tjr → WIP: Bug 531915 - part 3 - add back needed rounding helpers and emulate freebsd internal double types ?tjr
Attachment #9230306 - Attachment description: WIP: Bug 531915 - part 3 - add back needed rounding helpers and emulate freebsd internal double types ?tjr → Bug 531915 - part 3 - add back needed rounding helpers and emulate freebsd internal double types ?tjr
Pushed by tritter@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/a4aeb9fac1b3 part 1 - update fdlibm to import files needed for sin, cos, and tan r=arai https://hg.mozilla.org/integration/autoland/rev/12a65b83675a part 2 - update fdlibm local patches 4, 8, and 9 r=arai https://hg.mozilla.org/integration/autoland/rev/18fbbce162fc part 3 - add back needed rounding helpers and emulate freebsd internal double types ?tjr r=arai https://hg.mozilla.org/integration/autoland/rev/07b64b719699 part 4 - rem pio2 fixups r=arai https://hg.mozilla.org/integration/autoland/rev/effc44a3a9a1 part 5 - build and export sin, cos, and tan from fdlibm r=arai https://hg.mozilla.org/integration/autoland/rev/a42624b244be part 6 - optionally use fdlibm's sin, cos, and tan in jsmath r=tjr,arai

Comment on attachment 9230309 [details]
Bug 531915 - part 6 - optionally use fdlibm's sin, cos, and tan in jsmath r?tjr

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration: These patches are (mostly) inert in normal browser behavior. (They do add a conditional to some math functions which technically hurts performance, but our tests show this is near-unnoticable.) However it will be used by Tor Browser who will switch to ESR 91 in the coming months.
  • User impact if declined: Tor will have to carry this patchset and potentially rebase it for the lifetime of ESR 91.
  • Fix Landed on Version: 93
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Does nto affect browser behavior unless you have Resist Fingerprinting enabled.
  • String or UUID changes made by this patch:
Attachment #9230309 - Flags: approval-mozilla-esr91?
Attachment #9230303 - Flags: approval-mozilla-esr91?
Attachment #9230305 - Flags: approval-mozilla-esr91?
Attachment #9230306 - Flags: approval-mozilla-esr91?
Attachment #9230307 - Flags: approval-mozilla-esr91?
Attachment #9230308 - Flags: approval-mozilla-esr91?

Comment on attachment 9230303 [details]
Bug 531915 - part 1 - update fdlibm to import files needed for sin, cos, and tan r?tjr

Approved for 91.1esr, though it would be nice if we could standardize around this across the board for consistency's sake.

Attachment #9230303 - Flags: approval-mozilla-esr91? → approval-mozilla-esr91+
Attachment #9230305 - Flags: approval-mozilla-esr91? → approval-mozilla-esr91+
Attachment #9230306 - Flags: approval-mozilla-esr91? → approval-mozilla-esr91+
Attachment #9230307 - Flags: approval-mozilla-esr91? → approval-mozilla-esr91+
Attachment #9230308 - Flags: approval-mozilla-esr91? → approval-mozilla-esr91+
Attachment #9230309 - Flags: approval-mozilla-esr91? → approval-mozilla-esr91+

On 32bit Linux builds I'm getting now:
[ 1841s] 28:33.05 /usr/bin/ccache /usr/bin/g++ -o IntermNodePatternMatcher.o -c -I/home/abuild/rpmbuild/BUILD/obj/dist/stl_wrappers -I/home/abuild/rpmbuild/BUILD/obj/dist/system_wrappers -include /home/abuild/rpmbuild/BUILD/firefox-91.1.0/config/gcc_hidden.h -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -DNDEBUG=1 -DTRIMMED=1 -DANGLE_PLATFORM_EXPORT= -D__NDK_FPABI__= -DANGLE_SKIP_DXGI_1_2_CHECK -DANGLE_ENABLE_KEYEDMUTEX -DANGLE_TRANSLATOR_ESSL_ONLY -DANGLE_ENABLE_ESSL -DANGLE_ENABLE_GLSL -DANGLE_ENABLE_HLSL '-DCR_CLANG_REVISION="llvmorg-12-init-11060-g118c3f3c-1"' -DDYNAMIC_ANNOTATIONS_ENABLED=0 -DNOMINMAX -DUNICODE -DWINVER=0x0A00 -D_ATL_NO_OPENGL -D_CRT_RAND_S -D_CRT_SECURE_NO_DEPRECATE -D_HAS_EXCEPTIONS=0 -D_SCL_SECURE_NO_DEPRECATE -D_SECURE_ATL -D_UNICODE -I/home/abuild/rpmbuild/BUILD/firefox-91.1.0/gfx/angle/targets/translator -I/home/abuild/rpmbuild/BUILD/obj/gfx/angle/targets/translator -I/home/abuild/rpmbuild/BUILD/firefox-91.1.0/gfx/angle/checkout/include -I/home/abuild/rpmbuild/BUILD/firefox-91.1.0/gfx/angle/checkout/out/gen/angle -I/home/abuild/rpmbuild/BUILD/firefox-91.1.0/gfx/angle/checkout/src -I/home/abuild/rpmbuild/BUILD/firefox-91.1.0/gfx/angle/checkout/src/common/third_party/base -I/home/abuild/rpmbuild/BUILD/obj/dist/include -I/usr/include/nspr4 -I/usr/include/nss3 -I/usr/include/nspr4 -I/home/abuild/rpmbuild/BUILD/obj/dist/include/nss -DMOZILLA_CLIENT -include /home/abuild/rpmbuild/BUILD/obj/mozilla-config.h -Wall -Wempty-body -Wignored-qualifiers -Wpointer-arith -Wsign-compare -Wtype-limits -Wunreachable-code -Wno-invalid-offsetof -Wc++2a-compat -Wduplicated-cond -Wimplicit-fallthrough -Wno-error=maybe-uninitialized -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=coverage-mismatch -Wno-error=free-nonheap-object -Wno-multistatement-macros -Wno-error=class-memaccess -Wno-error=deprecated-copy -Wno-error=unused-but-set-variable -Wformat -Wformat-overflow=2 -Wno-psabi -fno-sized-deallocation -fno-aligned-new -fno-exceptions -fno-strict-aliasing -fPIC -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fno-math-errno -pthread -pipe -freorder-blocks -O2 -fomit-frame-pointer -funwind-tables -msse2 -MD -MP -MF .deps/IntermNodePatternMatcher.o.pp /home/abuild/rpmbuild/BUILD/firefox-91.1.0/gfx/angle/checkout/src/compiler/translator/tree_util/IntermNodePatternMatcher.cpp
[ 1841s] 28:33.12 In file included from /home/abuild/rpmbuild/BUILD/firefox-91.1.0/modules/fdlibm/src/e_acos.cpp:44:
[ 1841s] 28:33.12 /home/abuild/rpmbuild/BUILD/firefox-91.1.0/modules/fdlibm/src/math_private.h:34:21: error: conflicting declaration ‘typedef __double_t double_t’
[ 1841s] 28:33.12 34 | typedef __double_t double_t;
[ 1841s] 28:33.12 | ^~~~~~~~
[ 1841s] 28:33.12 In file included from /usr/include/c++/11/cmath:45,
[ 1841s] 28:33.12 from /home/abuild/rpmbuild/BUILD/obj/dist/system_wrappers/cmath:3,
[ 1841s] 28:33.12 from /home/abuild/rpmbuild/BUILD/obj/dist/stl_wrappers/cmath:60,
[ 1841s] 28:33.12 from /home/abuild/rpmbuild/BUILD/firefox-91.1.0/modules/fdlibm/src/e_acos.cpp:41:
[ 1841s] 28:33.12 /usr/include/math.h:156:21: note: previous declaration as ‘typedef long double double_t’
[ 1841s] 28:33.12 156 | typedef long double double_t;
[ 1841s] 28:33.12 | ^~~~~~~~
[ 1841s] 28:33.12 gmake[4]: *** [/home/abuild/rpmbuild/BUILD/firefox-91.1.0/config/rules.mk:676: e_acos.o] Error 1

(In reply to Wolfgang Rosenauer [:wolfiR] from comment #44)

On 32bit Linux builds I'm getting now:
[...]

Thank you for reporting this, do you see this across distros or just on specific distros?

The fix would be to update these lines in math_private.h to go from

typedef double      __double_t;
typedef __double_t  double_t;

to

#if [linux 32-bit & maybe specific distro]
typedef long double      __double_t;
#else
typedef double      __double_t;
#endif
typedef __double_t  double_t;
Flags: needinfo?(mozilla)

I'm building only for openSUSE and there is only one official version left for i386 which is openSUSE Tumbleweed. I cannot tell if that is specific.

The system has glibc 2.33 and apparently it defines __GLIBC_FLT_EVAL_METHOD=2 resulting in typedef long double double_t; in math.h

Flags: needinfo?(mozilla)

(In reply to Wolfgang Rosenauer [:wolfiR] from comment #46)

I'm building only for openSUSE and there is only one official version left for i386 which is openSUSE Tumbleweed. I cannot tell if that is specific.

The system has glibc 2.33 and apparently it defines __GLIBC_FLT_EVAL_METHOD=2 resulting in typedef long double double_t; in math.h

Thank you for the information and for digging into glibc. I am going to tag @arai and @tjr and maybe together we can figure out how to proceed.

For starters, it looks like the autoland job for this patch didn't test Linux 32-bit (it tested Windows 32-bit and Android 32-bit) which is odd (since Firefox provides a Linux 32-bit installer). Earlier, I was worried that if we set double_t to long double it would break on other 32-bit linux distros but now I am not sure if that is true (it will break on 32-bit windows and 32-bit android but it might hold for all 32-bit linux.)

Flags: needinfo?(tom)
Flags: needinfo?(arai.unmht)

given that what we want to do there is just use double, regardless of system's definition (double or long double),
I think we'd better workaround the conflict by not using the double_t or __double_t identifiers.

We can replace double_t and __double_t to some other names, or maybe just use double itself.

Flags: needinfo?(arai.unmht)

anyway, let's do that in new bug, given this bug is already fixed, in previous cycle.

Regressions: 1729459
Flags: needinfo?(tom)
Blocks: 1765276
Blocks: 1827576
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: