When building the JS shell with GCC 8.0.1 on Fedora 28, the following error occurs: /home/sstangl/dev/gecko-dev/js/src/frontend/Parser.cpp: In member function ‘void js::frontend::GeneralParser<ParseHandler, CharT>::checkDestructuringAssignmentName(js::frontend::GeneralParser<ParseHandler, CharT>::Node, js::frontend::TokenPos, js::frontend::GeneralParser<ParseHandler, CharT>::PossibleError*)’: /home/sstangl/dev/gecko-dev/js/src/dbg32/dist/include/mozilla/Assertions.h:417:71: internal compiler error: in cp_tree_equal, at cp/tree.c:3889 static_assert(mozilla::detail::AssertionConditionType<decltype(x)>::isValid, \ ^ /home/sstangl/dev/gecko-dev/js/src/dbg32/dist/include/mozilla/Assertions.h:432:5: note: in expansion of macro ‘MOZ_VALIDATE_ASSERT_CONDITION_TYPE’ MOZ_VALIDATE_ASSERT_CONDITION_TYPE(expr); \ ... etc, etc. The error appears to be in a tree comparison function when expanding through a bunch of macros. An easy workaround is to pre-calculate the value being asserted outside of the macro, so that GCC's tree operations don't have to deal with whatever precise conditions are giving rise to the error. With this patch applied, the shell compiles.

Comment on attachment 8972464 [details] [diff] [review] 0001-Break-up-a-one-liner-to-prevent-an-internal-GCC-8.0..patch Review of attachment 8972464 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/frontend/Parser.cpp @@ +9163,5 @@ > PossibleError* possibleError) > { > +#ifdef DEBUG > + // GCC 8.0.1 crashes if this is a one-liner. > + bool is_name = handler.isName(name); isName for camelCaps naming consistency. This should get backported to all branches, IMO, so those of us building on Fedora 28 and later -- or with newer gcc -- can keep compiling older releases.

Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/0b238edd9258 Break up a one-liner to prevent an internal GCC 8.0.1 error. r=Waldo

Have we reported this upstream?

I ran into this compiling ESR60 with the Freedesktop 18.07 Flatpak SDK. I agree that it should be backported. Strangely, it only occurred for me when I changed some unrelated code (with the patch from bug 1478679.) Was it ever reported upstream to GCC?

In fact it happened to me in a different assert. Here's a patch to put the same workaround in that assert as well.

Here's a version of the patch suitable for backporting to ESR60, since the code changed slightly in the meantime.

Pushed by dluca@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/019ed2fbfc31 Repeat GCC bug workaround in another place. r=sstangl

Comment on attachment 8995551 [details] [diff] [review] Repeat GCC bug workaround in another place (ESR60 version) [Approval Request Comment] If this is not a sec:{high,crit} bug, please state case for ESR consideration: Bug 1478679, also under consideration for uplift, triggers a compiler bug in GCC 8.1, which these patches work around. User impact if declined: Bug 1478679 can't be uplifted, so the memory leak in the developer tools will remain. Fix Landed on Version: 63 Risk to taking this patch (and alternatives if risky): Low risk, the fix is not invasive. String or UUID changes made by this patch: None See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.

Philip, the patch with the flag "checkin?", is that for trunk (mozilla-central, mozilla-inbound, autoland) or is it e.g. for beta? It's unusual for a patch to land on central and ESR60 but not beta.

The "checkin?" patch was for trunk. I didn't realize I should provide a patch for beta as well. Looking at https://dxr.mozilla.org/mozilla-beta/source/js/src/jit/RegisterSets.h#754, it looks like the "ESR60" patch should apply to beta.

Comment on attachment 8995550 [details] [diff] [review] Repeat GCC bug workaround in another place See comment 11, 13 and 14 [Approval Request Comment] If this is not a sec:{high,crit} bug, please state case for ESR consideration: Bug 1478679, also under consideration for uplift, triggers a compiler bug in GCC 8.1, which these patches work around. User impact if declined: Bug 1478679 can't be uplifted, so the memory leak in the developer tools will remain. Fix Landed on Version: 63 Risk to taking this patch (and alternatives if risky): Low risk, the fix is not invasive. String or UUID changes made by this patch: None

Comment on attachment 8995550 [details] [diff] [review] Repeat GCC bug workaround in another place Workaround for a gcc compiler issue, let's uplift for beta 15.

Marking fixed in 63 from comment 12.

Can you follow up to make sure someone reports this upstream? Thanks.

Comment on attachment 8995551 [details] [diff] [review] Repeat GCC bug workaround in another place (ESR60 version) Supporting patch for Spidermonkey embedders. Approved for ESR 60.2.

https://hg.mozilla.org/releases/mozilla-esr60/rev/3434c7949d47 https://hg.mozilla.org/releases/mozilla-esr60/rev/6bdca48795ec

I can't reproduce this anymore with GCC 8.2.0, so I assume it's been fixed and there's no longer a need to report it.