1312181 - SM(mozjs-crate) hits compiler (gcc?) bug when I land bug 1243617 patch with some others.

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Tooru Fujisawa [:arai]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7f889e843913&selectedJob=29696161
> In file included from /home/worker/workspace/build/src/js/src/target/debug/build/mozjs_sys-686333db719511ca/out/dist/include/mozilla/ArrayUtils.h:14:0,
>                  from /home/worker/workspace/build/src/js/src/target/debug/build/mozjs_sys-686333db719511ca/out/dist/include/mozilla/RangedPtr.h:15,
>                  from /home/worker/workspace/build/src/js/src/target/debug/build/mozjs_sys-686333db719511ca/out/dist/include/mozilla/Range.h:10,
>                  from /home/worker/workspace/build/src/js/src/target/debug/build/mozjs_sys-686333db719511ca/out/dist/include/js/CharacterEncoding.h:10,
>                  from /home/worker/workspace/build/src/js/src/jscntxt.h:14,
>                  from /home/worker/workspace/build/src/js/src/frontend/BytecodeEmitter.h:12,
>                  from /home/worker/workspace/build/src/js/src/frontend/BytecodeEmitter.cpp:11,
>                  from /home/worker/workspace/build/src/js/src/target/debug/build/mozjs_sys-686333db719511ca/out/js/src/Unified_cpp_js_src7.cpp:2:
> /home/worker/workspace/build/src/js/src/frontend/Parser.h: In instantiation of 'T& js::frontend::BytecodeEmitter::NestableControl::as() [with T = LoopControl]':
> /home/worker/workspace/build/src/js/src/frontend/BytecodeEmitter.cpp:1965:71:   required from here
> /home/worker/workspace/build/src/js/src/frontend/Parser.h:525:22: error: cannot call member function 'bool js::frontend::ParseContext::Statement::is() const [with T = LoopControl]' without object
>      MOZ_ASSERT(is<T>());
>                       ^
> /home/worker/workspace/build/src/js/src/target/debug/build/mozjs_sys-686333db719511ca/out/dist/include/mozilla/Assertions.h:362:69: note: in definition of macro 'MOZ_VALIDATE_ASSERT_CONDITION_TYPE'
>       static_assert(mozilla::detail::AssertionConditionType<decltype(x)>::isValid, \
>                                                                      ^
> /home/worker/workspace/build/src/js/src/target/debug/build/mozjs_sys-686333db719511ca/out/dist/include/mozilla/Assertions.h:389:39: note: in expansion of macro 'MOZ_ASSERT_HELPER1'
>  #define MOZ_RELEASE_ASSERT_GLUE(a, b) a b
>                                        ^
> /home/worker/workspace/build/src/js/src/target/debug/build/mozjs_sys-686333db719511ca/out/dist/include/mozilla/Assertions.h:391:3: note: in expansion of macro 'MOZ_RELEASE_ASSERT_GLUE'
>    MOZ_RELEASE_ASSERT_GLUE( \
>    ^
> /home/worker/workspace/build/src/js/src/target/debug/build/mozjs_sys-686333db719511ca/out/dist/include/mozilla/Assertions.h:396:27: note: in expansion of macro 'MOZ_RELEASE_ASSERT'
>  #  define MOZ_ASSERT(...) MOZ_RELEASE_ASSERT(__VA_ARGS__)
>                            ^
> /home/worker/workspace/build/src/js/src/frontend/Parser.h:525:5: note: in expansion of macro 'MOZ_ASSERT'
>      MOZ_ASSERT(is<T>());
>      ^

It's throwing an error for js::frontend::ParseContext::Statement::is() when instantiating js::frontend::BytecodeEmitter::NestableControl::as, they're totally unrelated.

The issue happens when I land the following patches at once:
  * bug 1308775
  * bug 1311319
  * bug 1243617
  * bug 1283710
the issue doesn't happen when I land bug 1243617 patch alone.
also the issue doesn't happen when I land other 3 patches.

The issue disappears by either following:
  * remove AssertionConditionType part from MOZ_ASSERT
  * put |is<T>()| out of MOZ_ASSERT

it sounds like a compiler bug related to template, decltype, etc.
for now I'd like to put |is<T>()| out of MOZ_ASSERT as a workaround.

Comment 1

[Tooru Fujisawa [:arai]]

Attachment: Move is<T>() out of MOZ_ASSERT in BytecodeEmitter::NestableControl::as() as a workaround for compiler bug.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=4418b78f1349
this passes the try run.

Comment 2

[Tooru Fujisawa [:arai]]

Comment on attachment 8803603 [details] [diff] [review]
Move is<T>() out of MOZ_ASSERT in BytecodeEmitter::NestableControl::as() as a workaround for compiler bug.

this should be the same issue as bug 1066193 and bug 1063233, thanks to ehoogeveen :)
will prepare another patch that adds |this->|

Comment 3

[Tooru Fujisawa [:arai]]

Attachment: Add this-> to is<T>() in MOZ_ASSERT in BytecodeEmitter::NestableControl::as() as a workaround for compiler bug.

yay, it works :)
https://treeherder.mozilla.org/#/jobs?repo=try&revision=6e4fb4709408&selectedJob=29702786

Comment 4

[Emanuel Hoogeveen [:ehoogeveen]]

Comment on attachment 8803612 [details] [diff] [review]
Add this-> to is<T>() in MOZ_ASSERT in BytecodeEmitter::NestableControl::as() as a workaround for compiler bug.

Review of attachment 8803612 [details] [diff] [review]:
-----------------------------------------------------------------

I'm technically not a peer, but given the simplicity and precedent I think it's okay if I stamp this.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a5b7320dc158
Add this-> to is<T>() in MOZ_ASSERT in BytecodeEmitter::NestableControl::as() as a workaround for compiler bug. r=ehoogeveen

Comment 6

[Ryan VanderMeulen [:RyanVM]]

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