1578031 - -Werror=int-in-bool-context: fails on js/src/wasm/WasmTypes.h

Status: RESOLVED FIXED

(Core :: JavaScript: WebAssembly, defect, P1)

Description

[Sylvestre Ledru [:Sylvestre]]

                 from /root/firefox-gcc-last/obj-x86_64-pc-linux-gnu/js/src/jit/Unified_cpp_js_src_jit10.cpp:2:
/root/firefox-gcc-last/js/src/wasm/WasmTypes.h: In function 'constexpr bool js::wasm::GetOffsetGuardLimit(bool)':
/root/firefox-gcc-last/js/src/wasm/WasmTypes.h:2335:21: error: ?: using integer constants in boolean context, the expression will always evaluate to 'true' [-Werror=int-in-bool-context]
   return hugeMemory ? HugeOffsetGuardLimit : OffsetGuardLimit;
          ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

static constexpr bool GetOffsetGuardLimit(bool hugeMemory) {
#ifdef WASM_SUPPORTS_HUGE_MEMORY
  return hugeMemory ? HugeOffsetGuardLimit : OffsetGuardLimit;
#else
  return OffsetGuardLimit;
#endif
}

Comment 1

[Sylvestre Ledru [:Sylvestre]]

I am a bit confused as hugeMemory is a bool

Comment 2

[Ryan Hunt [:rhunt]]

The problem is that GetOffsetGuardLimit is returning a bool when it should be returning size_t. OffsetGuardLimit is being coerced to a bool which will always be true.

I am saddened that this is legal..

Comment 3

[Ryan Hunt [:rhunt]]

Attachment: Bug 1578031 - Correct return type for `GetOffsetGuardLimit`. r?lth

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5cc570283ec2
Correct return type for `GetOffsetGuardLimit`. r=lth

Comment 5

[Bogdan Tara[:bogdan_tara | bogdant]]

Backed out changeset 5cc570283ec2 (Bug 1578031) for SM bustages complaining about OffsetGuardLimit

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&fromchange=5cc570283ec269c9415a87b8e2a2a1d070c88b0e&tochange=7a62d5742e64ee0927f488f9f6732f9a4587b492&selectedJob=264528771

Backout link: https://hg.mozilla.org/integration/autoland/rev/81799bad5b25f42b32ea10710435adfa7ff62026

Failure logs:

1: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=264528771&repo=autoland&lineNumber=12227

[task 2019-09-02T10:01:37.692Z] TEST-PASS | js/src/jit-test/tests/wasm/basic.js | Success (code 0, args "--disable-wasm-huge-memory") [0.6 s]
[task 2019-09-02T10:01:37.804Z] TEST-PASS | js/src/jit-test/tests/v8-v5/check-raytrace.js | Success (code 0, args "") [12.8 s]
[task 2019-09-02T10:01:38.356Z] Assertion failure: offset < wasm::OffsetGuardLimit, at /builds/worker/workspace/build/src/js/src/jit/arm64/MacroAssembler-arm64.cpp:298
[task 2019-09-02T10:01:38.356Z] Exit code: -11
[task 2019-09-02T10:01:38.356Z] FAIL - wasm/bce.js
[task 2019-09-02T10:01:38.356Z] TEST-UNEXPECTED-FAIL | js/src/jit-test/tests/wasm/bce.js | Assertion failure: offset < wasm::OffsetGuardLimit, at /builds/worker/workspace/build/src/js/src/jit/arm64/MacroAssembler-arm64.cpp:298 (code -11, args "") [1.0 s]
[task 2019-09-02T10:01:38.356Z] INFO exit-status : -11
[task 2019-09-02T10:01:38.356Z] INFO timed-out : False
[task 2019-09-02T10:01:38.357Z] INFO stderr 2> Assertion failure: offset < wasm::OffsetGuardLimit, at /builds/worker/workspace/build/src/js/src/jit/arm64/MacroAssembler-arm64.cpp:298
[task 2019-09-02T10:01:38.552Z] TEST-PASS | js/src/jit-test/tests/wasm/big-resize.js | Success (code 0, args "") [0.2 s]
[task 2019-09-02T10:01:38.592Z] Assertion failure: offset < wasm::OffsetGuardLimit, at /builds/worker/workspace/build/src/js/src/jit/arm64/MacroAssembler-arm64.cpp:298
[task 2019-09-02T10:01:38.592Z] Exit code: -11
[task 2019-09-02T10:01:38.592Z] FAIL - wasm/bce.js
[task 2019-09-02T10:01:38.592Z] TEST-UNEXPECTED-FAIL | js/src/jit-test/tests/wasm/bce.js | Assertion failure: offset < wasm::OffsetGuardLimit, at /builds/worker/workspace/build/src/js/src/jit/arm64/MacroAssembler-arm64.cpp:298 (code -11, args "--wasm-compiler=baseline") [1.1 s]
[task 2019-09-02T10:01:38.593Z] INFO exit-status : -11
[task 2019-09-02T10:01:38.593Z] INFO timed-out : False
[task 2019-09-02T10:01:38.593Z] INFO stderr 2> Assertion failure: offset < wasm::OffsetGuardLimit, at /builds/worker/workspace/build/src/js/src/jit/arm64/MacroAssembler-arm64.cpp:298
[task 2019-09-02T10:01:38.736Z] TEST-PASS | js/src/jit-test/tests/wasm/big-resize.js | Success (code 0, args "--wasm-compiler=ion") [0.2 s]
[task 2019-09-02T10:01:38.788Z] Assertion failure: offset < wasm::OffsetGuardLimit, at /builds/worker/workspace/build/src/js/src/jit/arm64/MacroAssembler-arm64.cpp:298
[task 2019-09-02T10:01:38.788Z] Exit code: -11
[task 2019-09-02T10:01:38.788Z] FAIL - wasm/bce.js
[task 2019-09-02T10:01:38.788Z] TEST-UNEXPECTED-FAIL | js/src/jit-test/tests/wasm/bce.js | Assertion failure: offset < wasm::OffsetGuardLimit, at /builds/worker/workspace/build/src/js/src/jit/arm64/MacroAssembler-arm64.cpp:298 (code -11, args "--test-wasm-await-tier2") [1.1 s]
[task 2019-09-02T10:01:38.789Z] INFO exit-status : -11
[task 2019-09-02T10:01:38.789Z] INFO timed-out : False
[task 2019-09-02T10:01:38.789Z] INFO stderr 2> Assertion failure: offset < wasm::OffsetGuardLimit, at /builds/worker/workspace/build/src/js/src/jit/arm64/MacroAssembler-arm64.cpp:298
[task 2019-09-02T10:01:38.792Z] TEST-PASS | js/src/jit-test/tests/wasm/big-resize.js | Success (code 0, args "--wasm-compiler=baseline") [0.2 s]

2: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=264534328&repo=autoland&lineNumber=11495

[task 2019-09-02T10:40:13.523Z] 10:40:13 INFO - TEST-PASS | tests/jit-test/jit-test/tests/wasm/backtrace.js | Success (code 0, args "--test-wasm-await-tier2") [0.1 s]
[task 2019-09-02T10:40:13.756Z] 10:40:13 INFO - TEST-PASS | tests/jit-test/jit-test/tests/wasm/backtrace.js | Success (code 0, args "--disable-wasm-huge-memory") [0.2 s]
[task 2019-09-02T10:40:13.895Z] 10:40:13 INFO - Assertion failure: offset < wasm::OffsetGuardLimit, at /builds/worker/workspace/build/src/js/src/jit/x64/CodeGenerator-x64.cpp:403
[task 2019-09-02T10:40:13.895Z] 10:40:13 INFO - Exit code: -11
[task 2019-09-02T10:40:13.895Z] 10:40:13 INFO - FAIL - wasm/baseline-abs-addr-opt.js
[task 2019-09-02T10:40:13.895Z] 10:40:13 WARNING - TEST-UNEXPECTED-FAIL | tests/jit-test/jit-test/tests/wasm/baseline-abs-addr-opt.js | Assertion failure: offset < wasm::OffsetGuardLimit, at /builds/worker/workspace/build/src/js/src/jit/x64/CodeGenerator-x64.cpp:403 (code -11, args "") [0.2 s]
[task 2019-09-02T10:40:13.895Z] 10:40:13 INFO - INFO exit-status : -11
[task 2019-09-02T10:40:13.895Z] 10:40:13 INFO - INFO timed-out : False
[task 2019-09-02T10:40:13.895Z] 10:40:13 INFO - INFO stderr 2> Assertion failure: offset < wasm::OffsetGuardLimit, at /builds/worker/workspace/build/src/js/src/jit/x64/CodeGenerator-x64.cpp:403
[task 2019-09-02T10:40:14.395Z] 10:40:14 INFO - Assertion failure: offset < wasm::OffsetGuardLimit, at /builds/worker/workspace/build/src/js/src/jit/x64/CodeGenerator-x64.cpp:403
[task 2019-09-02T10:40:14.395Z] 10:40:14 INFO - Exit code: -11
[task 2019-09-02T10:40:14.395Z] 10:40:14 INFO - FAIL - wasm/baseline-abs-addr-opt.js
[task 2019-09-02T10:40:14.395Z] 10:40:14 WARNING - TEST-UNEXPECTED-FAIL | tests/jit-test/jit-test/tests/wasm/baseline-abs-addr-opt.js | Assertion failure: offset < wasm::OffsetGuardLimit, at /builds/worker/workspace/build/src/js/src/jit/x64/CodeGenerator-x64.cpp:403 (code -11, args "--ion-eager --ion-offthread-compile=off --more-compartments") [0.5 s]
[task 2019-09-02T10:40:14.395Z] 10:40:14 INFO - INFO exit-status : -11
[task 2019-09-02T10:40:14.395Z] 10:40:14 INFO - INFO timed-out : False
[task 2019-09-02T10:40:14.396Z] 10:40:14 INFO - INFO stderr 2> Assertion failure: offset < wasm::OffsetGuardLimit, at /builds/worker/workspace/build/src/js/src/jit/x64/CodeGenerator-x64.cpp:403
[task 2019-09-02T10:40:15.013Z] 10:40:15 INFO - Assertion failure: offset < wasm::OffsetGuardLimit, at /builds/worker/workspace/build/src/js/src/jit/x64/CodeGenerator-x64.cpp:403
[task 2019-09-02T10:40:15.013Z] 10:40:15 INFO - Exit code: -11
[task 2019-09-02T10:40:15.013Z] 10:40:15 INFO - FAIL - wasm/baseline-abs-addr-opt.js
[task 2019-09-02T10:40:15.014Z] 10:40:15 WARNING - TEST-UNEXPECTED-FAIL | tests/jit-test/jit-test/tests/wasm/baseline-abs-addr-opt.js | Assertion failure: offset < wasm::OffsetGuardLimit, at /builds/worker/workspace/build/src/js/src/jit/x64/CodeGenerator-x64.cpp:403 (code -11, args "--ion-eager --ion-offthread-compile=off --ion-check-range-analysis --ion-extra-checks --no-sse3 --no-threads") [0.6 s]
[task 2019-09-02T10:40:15.014Z] 10:40:15 INFO - INFO exit-status : -11
[task 2019-09-02T10:40:15.014Z] 10:40:15 INFO - INFO timed-out : False
[task 2019-09-02T10:40:15.014Z] 10:40:15 INFO - INFO stderr 2> Assertion failure: offset < wasm::OffsetGuardLimit, at /builds/worker/workspace/build/src/js/src/jit/x64/CodeGenerator-x64.cpp:403
[task 2019-09-02T10:40:15.245Z] 10:40:15 INFO - Assertion failure: offset < wasm::OffsetGuardLimit, at /builds/worker/workspace/build/src/js/src/jit/x64/CodeGenerator-x64.cpp:403
[task 2019-09-02T10:40:15.245Z] 10:40:15 INFO - Exit code: -11
[task 2019-09-02T10:40:15.245Z] 10:40:15 INFO - FAIL - wasm/baseline-abs-addr-opt.js
[task 2019-09-02T10:40:15.245Z] 10:40:15 WARNING - TEST-UNEXPECTED-FAIL | tests/jit-test/jit-test/tests/wasm/baseline-abs-addr-opt.js | Assertion failure: offset < wasm::OffsetGuardLimit, at /builds/worker/workspace/build/src/js/src/jit/x64/CodeGenerator-x64.cpp:403 (code -11, args "--baseline-eager") [0.2 s]
[task 2019-09-02T10:40:15.245Z] 10:40:15 INFO - INFO exit-status : -11
[task 2019-09-02T10:40:15.245Z] 10:40:15 INFO - INFO timed-out : False
[task 2019-09-02T10:40:15.246Z] 10:40:15 INFO - INFO stderr 2> Assertion failure: offset < wasm::OffsetGuardLimit, at /builds/worker/workspace/build/src/js/src/jit/x64/CodeGenerator-x64.cpp:403
[task 2019-09-02T10:40:15.445Z] 10:40:15 INFO - Assertion failure: offset < wasm::OffsetGuardLimit, at /builds/worker/workspace/build/src/js/src/jit/x64/CodeGenerator-x64.cpp:403
[task 2019-09-02T10:40:15.445Z] 10:40:15 INFO - Exit code: -11

Comment 6

[Ryan Hunt [:rhunt]]

Assertions in CodeGenerator and MacroAssembler need to be updated as well. [1]

[1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=999cde917bb43da2494be823630947c2e4a18672

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b1f7e414bcf0
Correct return type for `GetOffsetGuardLimit`. r=lth

Comment 8

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/b1f7e414bcf0

Comment 9

[Ryan Hunt [:rhunt]]

Comment on attachment 9089683 [details]
Bug 1578031 - Correct return type for GetOffsetGuardLimit. r?lth

Beta/Release Uplift Approval Request

• User impact if declined: Wasm memory access offsets of greater than 1 will be folded into the base pointer, instead of using a machine instruction immediate. Previously we could support offsets of up to 2GiB or 64KiB. This should only affect performance, but is significant.
• Is this code covered by automated tests?: No
• 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): We've tested the original offset guard limits for a while. Using an incorrect offset guard is much more risky.
• String changes made/needed:

Comment 10

[Florin Strugariu [:Bebe]]

== Change summary for alert #22942 (as of Tue, 03 Sep 2019 12:56:41 GMT) ==

Improvements:

10% raptor-wasm-misc-firefox linux64-shippable opt 44,064.13 -> 39,739.46
10% raptor-wasm-misc-ion-firefox linux64-shippable opt 43,917.87 -> 39,609.96
10% raptor-wasm-misc-ion-firefox linux64-shippable-qr opt 43,930.73 -> 39,677.87
9% raptor-wasm-misc-firefox linux64-shippable-qr opt 43,835.86 -> 39,999.05
2% raptor-wasm-misc-baseline-firefox linux64-shippable opt 71,504.26 -> 69,756.17
2% raptor-wasm-misc-baseline-firefox linux64-shippable-qr opt 71,604.80 -> 69,951.33

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=22942

Comment 11

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 9089683 [details]
Bug 1578031 - Correct return type for GetOffsetGuardLimit. r?lth

Perf improvement, OK to uplift for beta 4.

Comment 12

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/releases/mozilla-beta/rev/6cdaee1535a1