1406081 - JS engine needs signal handlers for GCOV

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement)

Description

[Christian Holler (:decoder)]

Apparently the JS shell builds we produce lack the signal handlers we implemented in bug 1363469. We need move the code for these signal handlers such that it is available in the shell and also expose --enable-coverage (currently defined in toolkit) for these builds.

This blocks coverage reports for the JS shell fuzzing.

Comment 1

[Christian Holler (:decoder)]

Attachment: Bug 1406081 - Make --enable-coverage flag available in JS shell.

Review commit: https://reviewboard.mozilla.org/r/186832/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/186832/

Comment 2

[Christian Holler (:decoder)]

Attachment: Bug 1406081 - Add GCOV signal handlers to JS shell.

Review commit: https://reviewboard.mozilla.org/r/186834/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/186834/

Comment 3

[Christian Holler (:decoder)]

I looked at sharing the existing code in tools/code-coverage/ with the JS shell but decided that sharing this code probably adds more code in total than just duplicating the relevant parts adds. The shell doesn't need any kind of inter-process locks nor does it need shutdown hooks. Separating out the signal handler installs to mozglue also comes with a fair amount of additional code because the callbacks would be different (browser callbacks needs methods with mutexes).

So I decided to just share the --enable-coverage flag and copy relevant signal handler code with minor adjustments to js/src/shell/js.cpp which should only be built with the standalone shell as far as I know.

Comment 4

[Nathan Froyd [:froydnj]]

Comment on attachment 8915644 [details]
Bug 1406081 - Make --enable-coverage flag available in JS shell.

https://reviewboard.mozilla.org/r/186832/#review191924

::: build/moz.configure/toolchain.configure:1322
(Diff revision 1)
> +option('--enable-coverage', env='MOZ_CODE_COVERAGE',
> +       help='Enable code coverage')

This needs to be `js_option` to wind up being exposed to the JS shell in browser builds.  Otherwise, I'm 90% sure that you wind up in the confusing situation that `--enable-coverage` works for the shell in shell-only builds, but not in browser builds.

Comment 5

[Jan de Mooij [:jandem]]

Comment on attachment 8915645 [details]
Bug 1406081 - Add GCOV signal handlers to JS shell.

https://reviewboard.mozilla.org/r/186834/#review192222

Makes sense.

::: js/src/shell/js.cpp:163
(Diff revision 1)
> +#if defined(__GNUC__) && !defined(__clang__)
> +extern "C" void __gcov_dump();
> +extern "C" void __gcov_reset();
> +
> +void counters_dump(int) {
> +  __gcov_dump();

Nit: 4 space indent instead of 2 here and in the rest of the patch.

::: js/src/shell/js.cpp:171
(Diff revision 1)
> +void counters_reset(int) {
> +  __gcov_reset();
> +}
> +#else
> +void counters_dump(int) {
> +  /* Do nothing */

Do we want to support this configuration? Should we MOZ_CRASH or (at compile time) #error?

::: js/src/shell/js.cpp:179
(Diff revision 1)
> +void counters_reset(int) {
> +  /* Do nothing */
> +}
> +#endif
> +
> +static void InstallCoverageSignalHandlers()

Nit: \n after |static void|

Comment 6

[Christian Holler (:decoder)]

Comment on attachment 8915644 [details]
Bug 1406081 - Make --enable-coverage flag available in JS shell.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/186832/diff/1-2/

Comment 7

[Christian Holler (:decoder)]

Comment on attachment 8915645 [details]
Bug 1406081 - Add GCOV signal handlers to JS shell.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/186834/diff/1-2/

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5ef339233c71
Make --enable-coverage flag available in JS shell. r=froydnj
https://hg.mozilla.org/integration/autoland/rev/8b711541ec46
Add GCOV signal handlers to JS shell. r=jandem

Comment 9

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/5ef339233c71
https://hg.mozilla.org/mozilla-central/rev/8b711541ec46