Closed Bug 1527007 Opened 9 months ago Closed 9 months ago

Perma tests\jit-test\jit-test\tests\basic\testDivModWithIntMin.js | Z:\task_1549888723\build\tests\jit-test\jit-test\tests\basic\testDivModWithIntMin.js:8:5 Error: Assertion failed: got 0, expected -0 (code 3, args "") [2.3 s]

Categories

(Core :: JavaScript Engine, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla67
Tracking Status
firefox-esr60 66+ fixed
firefox65 --- wontfix
firefox66 + fixed
firefox67 + fixed

People

(Reporter: intermittent-bug-filer, Assigned: tcampbell)

References

Details

(Keywords: intermittent-failure, regression, Whiteboard: [stockwell needswork:owner])

Attachments

(2 files)

#[markdown(off)]
Filed by: cbrindusan [at] mozilla.com

https://treeherder.mozilla.org/logviewer.html#?job_id=227599112&repo=mozilla-central

https://queue.taskcluster.net/v1/task/dse6HTD1Qhe1kJjEE2G9lA/runs/0/artifacts/public/logs/live_backing.log

13:11:01 INFO - TEST-PASS | tests\jit-test\jit-test\tests\basic\testDestructuringVarInsideWith.js | Success (code 0, args "--ion-eager --ion-offthread-compile=off --more-compartments") [2.4 s]
13:11:01 INFO - {"action": "test_start", "jitflags": "--ion-eager --ion-offthread-compile=off --more-compartments", "pid": 8548, "source": "jittests", "test": "basic\\testDestructuringVarInsideWith.js", "thread": "main", "time": 1549890659.084}
13:11:01 INFO - {"action": "test_end", "extra": {"jitflags": "--ion-eager --ion-offthread-compile=off --more-compartments"}, "jitflags": "--ion-eager --ion-offthread-compile=off --more-compartments", "message": "Success", "pid": 8548, "source": "jittests", "status": "PASS", "test": "basic\\testDestructuringVarInsideWith.js", "thread": "main", "time": 1549890661.46}
13:11:01 INFO - TEST-PASS | tests\jit-test\jit-test\tests\basic\testDestructuringFormalError.js | Success (code 3, args "--baseline-eager") [2.4 s]
13:11:01 INFO - {"action": "test_start", "jitflags": "--baseline-eager", "pid": 8548, "source": "jittests", "test": "basic\\testDestructuringFormalError.js", "thread": "main", "time": 1549890659.083}
13:11:01 INFO - {"action": "test_end", "extra": {"jitflags": "--baseline-eager"}, "jitflags": "--baseline-eager", "message": "Success", "pid": 8548, "source": "jittests", "status": "PASS", "test": "basic\\testDestructuringFormalError.js", "thread": "main", "time": 1549890661.46}
13:11:01 INFO - TEST-PASS | tests\jit-test\jit-test\tests\basic\testDestructuringFormalError.js | Success (code 3, args "--ion-eager --ion-offthread-compile=off --more-compartments") [2.4 s]
13:11:01 INFO - {"action": "test_start", "jitflags": "--ion-eager --ion-offthread-compile=off --more-compartments", "pid": 8548, "source": "jittests", "test": "basic\\testDestructuringFormalError.js", "thread": "main", "time": 1549890659.079}
13:11:01 INFO - {"action": "test_end", "extra": {"jitflags": "--ion-eager --ion-offthread-compile=off --more-compartments"}, "jitflags": "--ion-eager --ion-offthread-compile=off --more-compartments", "message": "Success", "pid": 8548, "source": "jittests", "status": "PASS", "test": "basic\\testDestructuringFormalError.js", "thread": "main", "time": 1549890661.46}
13:11:03 INFO - TEST-PASS | tests\jit-test\jit-test\tests\basic\testDifferingArgc.js | Success (code 0, args "--ion-eager --ion-offthread-compile=off --more-compartments") [2.3 s]
13:11:03 INFO - {"action": "test_start", "jitflags": "--ion-eager --ion-offthread-compile=off --more-compartments", "pid": 8548, "source": "jittests", "test": "basic\\testDifferingArgc.js", "thread": "main", "time": 1549890661.421}
13:11:03 INFO - {"action": "test_end", "extra": {"jitflags": "--ion-eager --ion-offthread-compile=off --more-compartments"}, "jitflags": "--ion-eager --ion-offthread-compile=off --more-compartments", "message": "Success", "pid": 8548, "source": "jittests", "status": "PASS", "test": "basic\\testDifferingArgc.js", "thread": "main", "time": 1549890663.689}
13:11:03 INFO - Z:\task_1549888723\build\tests\jit-test\jit-test\tests\basic\testDivModWithIntMin.js:8:5 Error: Assertion failed: got 0, expected -0
13:11:03 INFO - Stack:
13:11:03 INFO - testModNegativeZero@Z:\task_1549888723\build\tests\jit-test\jit-test\tests\basic\testDivModWithIntMin.js:8:5
13:11:03 INFO - @Z:\task_1549888723\build\tests\jit-test\jit-test\tests\basic\testDivModWithIntMin.js:18:1
13:11:03 INFO - Exit code: 3
13:11:03 INFO - FAIL - basic\testDivModWithIntMin.js
13:11:03 WARNING - TEST-UNEXPECTED-FAIL | tests\jit-test\jit-test\tests\basic\testDivModWithIntMin.js | Z:\task_1549888723\build\tests\jit-test\jit-test\tests\basic\testDivModWithIntMin.js:8:5 Error: Assertion failed: got 0, expected -0 (code 3, args "") [2.3 s]
13:11:03 INFO - {"action": "test_start", "jitflags": "", "pid": 8548, "source": "jittests", "test": "basic\\testDivModWithIntMin.js", "thread": "main", "time": 1549890661.429}
13:11:03 INFO - {"action": "test_end", "extra": {"jitflags": ""}, "jitflags": "", "message": "Z:\\task_1549888723\\build\\tests\\jit-test\\jit-test\\tests\\basic\\testDivModWithIntMin.js:8:5 Error: Assertion failed: got 0, expected -0", "pid": 8548, "source": "jittests", "status": "FAIL", "test": "basic\\testDivModWithIntMin.js", "thread": "main", "time": 1549890663.695}
13:11:03 INFO - INFO exit-status : 3
13:11:03 INFO - INFO timed-out : False
13:11:03 INFO - INFO stderr 2> Z:\task_1549888723\build\tests\jit-test\jit-test\tests\basic\testDivModWithIntMin.js:8:5 Error: Assertion failed: got 0, expected -0
13:11:03 INFO - INFO stderr 2> Stack:
13:11:03 INFO - INFO stderr 2> testModNegativeZero@Z:\task_1549888723\build\tests\jit-test\jit-test\tests\basic\testDivModWithIntMin.js:8:5
13:11:03 INFO - INFO stderr 2> @Z:\task_1549888723\build\tests\jit-test\jit-test\tests\basic\testDivModWithIntMin.js:18:1
13:11:03 INFO - TEST-PASS | tests\jit-test\jit-test\tests\basic\testDifferingArgc.js | Success (code 0, args "") [2.3 s]
13:11:03 INFO - {"action": "test_start", "jitflags": "", "pid": 8548, "source": "jittests", "test": "basic\\testDifferingArgc.js", "thread": "main", "time": 1549890661.433}
13:11:03 INFO - {"action": "test_end", "extra": {"jitflags": ""}, "jitflags": "", "message": "Success", "pid": 8548, "source": "jittests", "status": "PASS", "test": "basic\\testDifferingArgc.js", "thread": "main", "time": 1549890663.701}
13:11:03 INFO - TEST-PASS | tests\jit-test\jit-test\tests\basic\testDifferingArgc.js | Success (code 0, args "--baseline-eager") [2.3 s]
13:11:03 INFO - {"action": "test_start", "jitflags": "--baseline-eager", "pid": 8548, "source": "jittests", "test": "basic\\testDifferingArgc.js", "thread": "main", "time": 1549890661.457}
13:11:03 INFO - {"action": "test_end", "extra": {"jitflags": "--baseline-eager"}, "jitflags": "--baseline-eager", "message": "Success", "pid": 8548, "source": "jittests", "status": "PASS", "test": "basic\\testDifferingArgc.js", "thread": "main", "time": 1549890663.726}
13:11:03 INFO - TEST-PASS | tests\jit-test\jit-test\tests\basic\testDestructuringVarInsideWith.js | Success (code 0, args "--no-baseline --no-ion --more-compartments") [2.3 s]
13:11:03 INFO - {"action": "test_start", "jitflags": "--no-baseline --no-ion --more-compartments", "pid": 8548, "source": "jittests", "test": "basic\\testDestructuringVarInsideWith.js", "thread": "main", "time": 1549890661.451}
13:11:03 INFO - {"action": "test_end", "extra": {"jitflags": "--no-baseline --no-ion --more-compartments"}, "jitflags": "--no-baseline --no-ion --more-compartments", "message": "Success", "pid": 8548, "source": "jittests", "status": "PASS", "test": "basic\\testDestructuringVarInsideWith.js", "thread": "main", "time": 1549890663.727}
13:11:03 INFO - TEST-PASS | tests\jit-test\jit-test\tests\basic\testDifferingArgc.js | Success (code 0, args "--ion-eager --ion-offthread-compile=off --ion-check-range-analysis --ion-extra-checks --no-sse3 --no-threads") [2.3 s]
13:11:03 INFO - {"action": "test_start", "jitflags": "--ion-eager --ion-offthread-compile=off --ion-check-range-analysis --ion-extra-checks --no-sse3 --no-threads", "pid": 8548, "source": "jittests", "test": "basic\\testDifferingArgc.js", "thread": "main", "time": 1549890661.457}
13:11:03 INFO - {"action": "test_end", "extra": {"jitflags": "--ion-eager --ion-offthread-compile=off --ion-check-range-analysis --ion-extra-checks --no-sse3 --no-threads"}, "jitflags": "--ion-eager --ion-offthread-compile=off --ion-check-range-analysis --ion-extra-checks --no-sse3 --no-threads", "message": "Success", "pid": 8548, "source": "jittests", "status": "PASS", "test": "basic\\testDifferingArgc.js", "thread": "main", "time": 1549890663.729}
13:11:03 INFO - Z:\task_1549888723\build\tests\jit-test\jit-test\tests\basic\testDivModWithIntMin.js:8:5 Error: Assertion failed: got 0, expected -0

Summary: Intermittent tests\jit-test\jit-test\tests\basic\testDivModWithIntMin.js | Z:\task_1549888723\build\tests\jit-test\jit-test\tests\basic\testDivModWithIntMin.js:8:5 Error: Assertion failed: got 0, expected -0 (code 3, args "") [2.3 s] → Perma ccov [tier 2] tests\jit-test\jit-test\tests\basic\testDivModWithIntMin.js | Z:\task_1549888723\build\tests\jit-test\jit-test\tests\basic\testDivModWithIntMin.js:8:5 Error: Assertion failed: got 0, expected -0 (code 3, args "") [2.3 s]

(In reply to Andreea Pavel [:apavel] from comment #1)

Jan, could it be from bug 1524752?

Maybe, but it's unlikely.

This looks like a compiler/platform issue. I wonder if it's from the Windows 10 1803 update. I'll look into this a bit today.

(In reply to Jan de Mooij [:jandem] from comment #2)

This looks like a compiler/platform issue. I wonder if it's from the Windows 10 1803 update. I'll look into this a bit today.

The test that fails is doing this:

assertEq(intMin % -1, -0);

It fails because we have a +0 instead of -0. Maybe fmod broke?

It fails because we have a +0 instead of -0. Maybe fmod broke?

Here's what's going into and out of fmod on my 1809 machine.

ucrtbase!fmod:
00007ffb`d3898050 4881ecb8000000 sub rsp,0B8h

0:000> .formats @xmm0
Evaluate expression:
Hex: c1e00000`00000000
Decimal: -4476578029606273024
Octal: 1407400000000000000000
Binary: 11000001 11100000 00000000 00000000 00000000 00000000 00000000 00000000
Chars: ........
Time: ***** Invalid FILETIME
Float: low 0 high -28
Double: -2.14748e+009

0:000> .formats @xmm1
Evaluate expression:
Hex: bff00000`00000000
Decimal: -4616189618054758400
Octal: 1377600000000000000000
Binary: 10111111 11110000 00000000 00000000 00000000 00000000 00000000 00000000
Chars: ........
Time: ***** Invalid FILETIME
Float: low 0 high -1.875
Double: -1

0:000> gu
js+0x22493e:
00007ff6`4109493e 660f6ff0 movdqa xmm6,xmm0

0:000> .formats @xmm0
Evaluate expression:
Hex: 00000000`00000000
Decimal: 0
Octal: 0000000000000000000000
Binary: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
Chars: ........
Time: unavailable
Float: low 0 high 0
Double: 0

https://support.microsoft.com/en-us/help/4346783/windows-10-update-kb4346783 says

Applies to: Windows 10, version 1803

Addresses an issue in the Universal CRT that sometimes causes the AMD64-specific implementation of FMOD to return an incorrect result when given very large inputs.

But I wonder why it still fails for me on 1809? Did this update cause the problem or fix it?

Do we have a way to tell whether the CI machines have this KB?

By the way, this would be a tier-1 failure, if we actually ran it on more platforms. Why aren't we running jit-tests on regular Windows builds? https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&searchStr=jit&group_state=expanded

jittests are run only when js code is changed, so it will not show up every push; Now why these don't run on windows, I have no idea- I spent some time looking into this and am confused- it looks like it should be scheduled, but it isn't.

I see for win10/opt we schedule windows-tests:
https://searchfox.org/mozilla-central/source/taskcluster/ci/test/test-platforms.yml#203

^ same for win10-ccov which gets scheduled

windows-tests as a test-set is defined here:
https://searchfox.org/mozilla-central/source/taskcluster/ci/test/test-sets.yml#216

and it references jittest, which is defined here:
https://searchfox.org/mozilla-central/source/taskcluster/ci/test/compiled.yml#54

and in jittest I found what I was looking for:
https://searchfox.org/mozilla-central/source/taskcluster/ci/test/compiled.yml#64

        # Turn off for all linux and windows platforms, since it is
        # redundant with SM(...) jobs -- except for code coverage builds,
        # which are still needed since ccov doesn't yet work with
        # spidermonkey jobs.
        #
        # The regex is essentially /linux.*/ but with a negative match for
        # the last 5 characters being "-ccov". The terminal ..... is needed
        # because otherwise the .* could match the full string and the
        # empty string would fail to match /(?!-ccov)/ so the whole thing
        # would succeed. The beginning /?=linux/ is needed because the
        # platform "linux64" doesn't have enough characters for both the
        # beginning /linux/ and the final /...../.
        #
        # Additionally, platforms contain suffixes like "/opt" or "/debug".
        (?=linux).*(?!-ccov)...../.*: []  # redundant with SM(...)
        (?=windows).*(?!-ccov)...../.*: []  # redundant with SM(p)

the CI machine is basically the stock laptop which is 1803 + who knows what KB version. I assume that is what you would have locally.

        # Turn off for all linux and windows platforms, since it is
        # redundant with SM(...) jobs -- except for code coverage builds,

SM jobs only run on 32-bit builds, so we missed this 64-bit specific issue. If we can't run both, we really ought to favor 64-bit builds. (I bet a cookie that there's already a bug on this detailing the horrifying experience of the last person who tried.)

the CI machine is basically the stock laptop which is 1803 + who knows what
KB version. I assume that is what you would have locally.

Well, the machine I tried on had 1809, which I assume would include all the previous updates to 1803. I'm half wondering if, maybe the KB traded one bug for another, fixing some other fmod issue and in the process caused the regression that we're seeing?

:egao, :gbrown, as you both have laptops locally, could you see what version you have and if this test passes/fails?

Flags: needinfo?(gbrown)
Flags: needinfo?(egao)

Steve, see comment 8. Should SM(p) and SM(cgc) run on Win64 (too)?

Flags: needinfo?(jdemooij) → needinfo?(sphink)

Joel, just to double check, since you mentioned stock laptop -- we're talking about ccov x86_64 builds, right?

sorry, I was referring to windows/aarch64 on a laptop. windows ccov is run in VMs in a data center.

FWIW, I've seen this failure locally running an x86_64 shell on Windows 10 r1809. I figured it was probably known but I guess I should have reported it.

(In reply to Joel Maher ( :jmaher ) (UTC-4) from comment #9)

:egao, :gbrown, as you both have laptops locally, could you see what version you have and if this test passes/fails?

We have aarch64 laptops, so probably not relevant to this discussion.

Flags: needinfo?(gbrown)
Flags: needinfo?(egao)

(In reply to Jan de Mooij [:jandem] from comment #10)

Steve, see comment 8. Should SM(p) and SM(cgc) run on Win64 (too)?

Uh, yeah. I guess I was kind of assuming that that's where they were running. Filed bug 1527777.

Flags: needinfo?(sphink)

This is now a tier1 and bug 1528169 was filed.

See Also: → 1528169

Looks like we have to work around this platform bug on Win64. I'll take a look.

Flags: needinfo?(jdemooij)
Duplicate of this bug: 1528169
Summary: Perma ccov [tier 2] tests\jit-test\jit-test\tests\basic\testDivModWithIntMin.js | Z:\task_1549888723\build\tests\jit-test\jit-test\tests\basic\testDivModWithIntMin.js:8:5 Error: Assertion failed: got 0, expected -0 (code 3, args "") [2.3 s] → Perma tests\jit-test\jit-test\tests\basic\testDivModWithIntMin.js | Z:\task_1549888723\build\tests\jit-test\jit-test\tests\basic\testDivModWithIntMin.js:8:5 Error: Assertion failed: got 0, expected -0 (code 3, args "") [2.3 s]

dmajor, do you know if this only fails when RHS is -1 and LHS is INT32_MIN? Or is one of these conditions sufficient? Once we know more about what inputs are affected we can probably work around it in NumberMod (and report it to MS...)

Flags: needinfo?(jdemooij) → needinfo?(dmajor)

It's possible they're optimizing this with int32 IDIV on x64 so they have to check for the INT32_MIN / -1 case because that's UB. In that case this would be the only failure case. I'll add some more tests to confirm on Try.

I tested this on Try: https://hg.mozilla.org/try/rev/50e1ca5b8e78e9411c964592e176c1924876a2f9

Unfortunately now the |assertEq(intMin % -2, -0);| test I added fails so it's not just INT32_MIN % -1 :/

I'm not seeing any integer divisions in the disassembly.

I'm assuming your try push covers what you wanted me to test, but let me know if you have other requests.

Flags: needinfo?(dmajor)

Over the last 7 days there are 40 failures present on this bug. These happen only on windows10-64-ccov.

Here is the latest failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=230474659&repo=mozilla-central&lineNumber=12903

05:57:38 INFO - {"action": "test_start", "jitflags": "", "pid": 10064, "source": "jittests", "test": "basic\testDifferingArgc.js", "thread": "main", "time": 1551160656.577}
05:57:38 INFO - {"action": "test_end", "extra": {"jitflags": ""}, "jitflags": "", "message": "Success", "pid": 10064, "source": "jittests", "status": "PASS", "test": "basic\testDifferingArgc.js", "thread": "main", "time": 1551160658.659}
05:57:38 INFO - TEST-PASS | tests\jit-test\jit-test\tests\basic\testDifferingArgc.js | Success (code 0, args "--ion-eager --ion-offthread-compile=off --more-compartments") [2.1 s]
05:57:38 INFO - {"action": "test_start", "jitflags": "--ion-eager --ion-offthread-compile=off --more-compartments", "pid": 10064, "source": "jittests", "test": "basic\testDifferingArgc.js", "thread": "main", "time": 1551160656.737}
05:57:38 INFO - {"action": "test_end", "extra": {"jitflags": "--ion-eager --ion-offthread-compile=off --more-compartments"}, "jitflags": "--ion-eager --ion-offthread-compile=off --more-compartments", "message": "Success", "pid": 10064, "source": "jittests", "status": "PASS", "test": "basic\testDifferingArgc.js", "thread": "main", "time": 1551160658.859}
05:57:39 INFO - TEST-PASS | tests\jit-test\jit-test\tests\basic\testDifferingArgc.js | Success (code 0, args "--ion-eager --ion-offthread-compile=off --ion-check-range-analysis --ion-extra-checks --no-sse3 --no-threads") [2.3 s]
05:57:39 INFO - {"action": "test_start", "jitflags": "--ion-eager --ion-offthread-compile=off --ion-check-range-analysis --ion-extra-checks --no-sse3 --no-threads", "pid": 10064, "source": "jittests", "test": "basic\testDifferingArgc.js", "thread": "main", "time": 1551160657.148}
05:57:39 INFO - {"action": "test_end", "extra": {"jitflags": "--ion-eager --ion-offthread-compile=off --ion-check-range-analysis --ion-extra-checks --no-sse3 --no-threads"}, "jitflags": "--ion-eager --ion-offthread-compile=off --ion-check-range-analysis --ion-extra-checks --no-sse3 --no-threads", "message": "Success", "pid": 10064, "source": "jittests", "status": "PASS", "test": "basic\testDifferingArgc.js", "thread": "main", "time": 1551160659.494}
05:57:39 INFO - TEST-PASS | tests\jit-test\jit-test\tests\basic\testDifferingArgc.js | Success (code 0, args "--no-baseline --no-ion --more-compartments") [2.3 s]
05:57:39 INFO - {"action": "test_start", "jitflags": "--no-baseline --no-ion --more-compartments", "pid": 10064, "source": "jittests", "test": "basic\testDifferingArgc.js", "thread": "main", "time": 1551160657.226}
05:57:39 INFO - {"action": "test_end", "extra": {"jitflags": "--no-baseline --no-ion --more-compartments"}, "jitflags": "--no-baseline --no-ion --more-compartments", "message": "Success", "pid": 10064, "source": "jittests", "status": "PASS", "test": "basic\testDifferingArgc.js", "thread": "main", "time": 1551160659.494}
05:57:39 INFO - TEST-PASS | tests\jit-test\jit-test\tests\basic\testDifferingArgc.js | Success (code 0, args "--baseline-eager") [2.3 s]
05:57:39 INFO - {"action": "test_start", "jitflags": "--baseline-eager", "pid": 10064, "source": "jittests", "test": "basic\testDifferingArgc.js", "thread": "main", "time": 1551160657.232}
05:57:39 INFO - {"action": "test_end", "extra": {"jitflags": "--baseline-eager"}, "jitflags": "--baseline-eager", "message": "Success", "pid": 10064, "source": "jittests", "status": "PASS", "test": "basic\testDifferingArgc.js", "thread": "main", "time": 1551160659.529}
05:57:39 INFO - Z:\task_1551158909\build\tests\jit-test\jit-test\tests\basic\testDivModWithIntMin.js:8:5 Error: Assertion failed: got 0, expected -0
05:57:39 INFO - Stack:
05:57:39 INFO - testModNegativeZero@Z:\task_1551158909\build\tests\jit-test\jit-test\tests\basic\testDivModWithIntMin.js:8:5
05:57:39 INFO - @Z:\task_1551158909\build\tests\jit-test\jit-test\tests\basic\testDivModWithIntMin.js:18:1
05:57:39 INFO - Exit code: 3
05:57:39 INFO - FAIL - basic\testDivModWithIntMin.js
05:57:39 WARNING - TEST-UNEXPECTED-FAIL | tests\jit-test\jit-test\tests\basic\testDivModWithIntMin.js | Z:\task_1551158909\build\tests\jit-test\jit-test\tests\basic\testDivModWithIntMin.js:8:5 Error: Assertion failed: got 0, expected -0 (code 3, args "") [2.2 s]
05:57:39 INFO - {"action": "test_start", "jitflags": "", "pid": 10064, "source": "jittests", "test": "basic\testDivModWithIntMin.js", "thread": "main", "time": 1551160657.3100002}
05:57:39 INFO - {"action": "test_end", "extra": {"jitflags": ""}, "jitflags": "", "message": "Z:\task_1551158909\build\tests\jit-test\jit-test\tests\basic\testDivModWithIntMin.js:8:5 Error: Assertion failed: got 0, expected -0", "pid": 10064, "source": "jittests", "status": "FAIL", "test": "basic\testDivModWithIntMin.js", "thread": "main", "time": 1551160659.548}
05:57:39 INFO - INFO exit-status : 3
05:57:39 INFO - INFO timed-out : False
05:57:39 INFO - INFO stderr 2> Z:\task_1551158909\build\tests\jit-test\jit-test\tests\basic\testDivModWithIntMin.js:8:5 Error: Assertion failed: got 0, expected -0
05:57:39 INFO - INFO stderr 2> Stack:
05:57:39 INFO - INFO stderr 2> testModNegativeZero@Z:\task_1551158909\build\tests\jit-test\jit-test\tests\basic\testDivModWithIntMin.js:8:5
05:57:39 INFO - INFO stderr 2> @Z:\task_1551158909\build\tests\jit-test\jit-test\tests\basic\testDivModWithIntMin.js:18:1
05:57:39 INFO - Z:\task_1551158909\build\tests\jit-test\jit-test\tests\basic\testDivModWithIntMin.js:8:5 Error: Assertion failed: got 0, expected -0
05:57:39 INFO - Stack:
05:57:39 INFO - testModNegativeZero@Z:\task_1551158909\build\tests\jit-test\jit-test\tests\basic\testDivModWithIntMin.js:8:5
05:57:39 INFO - @Z:\task_1551158909\build\tests\jit-test\jit-test\tests\basic\testDivModWithIntMin.js:18:1
05:57:39 INFO - Exit code: 3
05:57:39 INFO - FAIL - basic\testDivModWithIntMin.js
05:57:39 WARNING - TEST-UNEXPECTED-FAIL | tests\jit-test\jit-test\tests\basic\testDivModWithIntMin.js | Z:\task_1551158909\build\tests\jit-test\jit-test\tests\basic\testDivModWithIntMin.js:8:5 Error: Assertion failed: got 0, expected -0 (code 3, args "--ion-eager --ion-offthread-compile=off --ion-check-range-analysis --ion-extra-checks --no-sse3 --no-threads") [2.2 s]
05:57:39 INFO - {"action": "test_start", "jitflags": "--ion-eager --ion-offthread-compile=off --ion-check-range-analysis --ion-extra-checks --no-sse3 --no-threads", "pid": 10064, "source": "jittests", "test": "basic\testDivModWithIntMin.js", "thread": "main", "time": 1551160657.366}
05:57:39 INFO - {"action": "test_end", "extra": {"jitflags": "--ion-eager --ion-offthread-compile=off --ion-check-range-analysis --ion-extra-checks --no-sse3 --no-threads"}, "jitflags": "--ion-eager --ion-offthread-compile=off --ion-check-range-analysis --ion-extra-checks --no-sse3 --no-threads", "message": "Z:\task_1551158909\build\tests\jit-test\jit-test\tests\basic\testDivModWithIntMin.js:8:5 Error: Assertion failed: got 0, expected -0", "pid": 10064, "source": "jittests", "status": "FAIL", "test": "basic\testDivModWithIntMin.js", "thread": "main", "time": 1551160659.615}
05:57:39 INFO - INFO exit-status : 3
05:57:39 INFO - INFO timed-out : False
05:57:39 INFO - INFO stderr 2> Z:\task_1551158909\build\tests\jit-test\jit-test\tests\basic\testDivModWithIntMin.js:8:5 Error: Assertion failed: got 0, expected -0
05:57:39 INFO - INFO stderr 2> Stack:
05:57:39 INFO - INFO stderr 2> testModNegativeZero@Z:\task_1551158909\build\tests\jit-test\jit-test\tests\basic\testDivModWithIntMin.js:8:5
05:57:39 INFO - INFO stderr 2> @Z:\task_1551158909\build\tests\jit-test\jit-test\tests\basic\testDivModWithIntMin.js:18:1
05:57:39 INFO - Z:\task_1551158909\build\tests\jit-test\jit-test\tests\basic\testDivModWithIntMin.js:8:5 Error: Assertion failed: got 0, expected -0

Flags: needinfo?(sdetar)

Ted and I talked about this last week and he'll take a look because he's better set up to deal with Windows-specific issues.

Flags: needinfo?(sdetar) → needinfo?(tcampbell)
Blocks: 1527777

Well this sucks. I get incorrect values for negative-int % negative-int in general when the result should be -0. Chrome and Linux both report -0, but Windows Firefox reports +0. Still digging..

Using a simple C++ example on MSVC, I see that sign is computed incorrectly when result is 0 for any negative input. This applies to floating and integer values (-4.5 % 1.5 is wrong, for example). The same sample works correctly on 32-bit builds in the exact same setup.

The host platform is wrong here, but we'll need to ship our own work around for this..

#include <iostream>
#include <math.h>

int main()
{
double x = -4.5;
double y = +1.5;

double z = fmod(x, y);

std::cout << x << " % " << y << " -> " << z << std::endl;
// |0| on Win-64 (wrong)
// |-0| on 32-bit compile on same Win64 machine
}

Priority: P5 → P1

ARM64 (on v1803 and v1809) does not appear to be affected either by this bug. This patch fixes AMD64 for me on v1809. X86 continues to work for me on v1809.

Try run in progress: https://treeherder.mozilla.org/#/jobs?repo=try&revision=fd2091d8c0d50fdce56323938f296f2672ca8112

Flags: needinfo?(tcampbell)

Should the patch be limited to defined(_M_AMD64)?

I've been wondering about that... Is it better to have have platform variations (window = work-around, linux = normal), or more targetted fixes. Win64 is dominant platform, so I was leaning towards doing the same thing for all Windows, but I could be swayed by arguments to have more targeted fixes.

I suppose it wouldn't be hugely surprising if there exist versions of Windows that we haven't tested that are broken. Maybe it's better to cover them all.

Assignee: nobody → tcampbell
Pushed by tcampbell@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/865ffd54922d
Work around Windows fmod bugs r=jandem
Status: NEW → RESOLVED
Closed: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67

Comment on attachment 9047134 [details]
Bug 1527007 - Work around Windows fmod bugs

Beta/Release Uplift Approval Request

  • Feature/Bug causing the regression: None
  • User impact if declined: Windows 10 landed a regression in 1803 / 1809 updates that broke their implementation of |fmod| in C standard library. These users will get incorrect results for % operator in JavaScript when result should be -0.0. Eg |-4 % -2| should be |-0| by spec, but is reported as |+0|.
    Existing test suites have perma-fails due to this windows update and that is probably best reason to fix.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Low risk change for a non-performance-sensitive code path.
  • String changes made/needed:
Attachment #9047134 - Flags: approval-mozilla-beta?

Comment on attachment 9047134 [details]
Bug 1527007 - Work around Windows fmod bugs

Update beta tests for Win 1809; sounds good to me.

Attachment #9047134 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

One question though, what happens if they fix this in monthly updates to 1809?

Flags: needinfo?(tcampbell)

When they fix this, the code branch that I added will never run and we will use the fmod result. The condition we check for would be impossible if their implementation wasn't buggy.

When we are happy their fix is there we can revert the patch, but I'm not sure there is any hurry to do that.

This hasn't come up in the wild, so I don't think there is any reason to touch ESR.

Flags: needinfo?(tcampbell)

(In reply to Ted Campbell [:tcampbell] from comment #39)

This hasn't come up in the wild, so I don't think there is any reason to
touch ESR.

It's burning tests there too, though. Please nominate.

Flags: needinfo?(tcampbell)

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration: A Windows 10 update broke the 'fmod' function in the C++ standard library. This is causing a few tests in automation to perma-fail.
  • User impact if declined: Test suite failures is primary concern.

Incorrect computation of |-4 % -2| which has so far not been reported as causing problems in the wild.

  • Fix Landed on Version: 66/67
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Patch has been stable on nightly and beta. The added code only covers the case that should never happen. If Microsoft fixes their end, the code that I've added becomes dormant and the branch condition is never activated.
  • String or UUID changes made by this patch:
Flags: needinfo?(tcampbell)
Attachment #9048237 - Flags: approval-mozilla-esr60?
Comment on attachment 9048237 [details] [diff] [review]
ESR60 - Bug 1527007 - Work around Windows fmod bugs

I've verified this patch locally on Win 10 v1809 in a jsshell build (with and without patch). The patch is the same as on central, but the diff-context didn't merge correctly so I did a separate patch here.
Attachment #9048237 - Attachment description: 0001-Bug-1527007-Work-around-Windows-fmod-bugs.-r-jandem.patch → ESR60 - Bug 1527007 - Work around Windows fmod bugs
Comment on attachment 9048237 [details] [diff] [review]
ESR60 - Bug 1527007 - Work around Windows fmod bugs

Fixes permafailing tests in CI since the Win10 1803 update. Code is only active on buggy Windows versions and a no-op if/when MS fixes the bug on their end. Approved for 60.6esr.
Attachment #9048237 - Flags: approval-mozilla-esr60? → approval-mozilla-esr60+
You need to log in before you can comment on or make changes to this bug.