1440034 - Crash annotation fd is incorrect in GeckoView

Status: RESOLVED FIXED

(Toolkit :: Crash Reporting, enhancement)

Description

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

I noticed this in GeckoChildProcessHost::LaunchAndroidService:

  int32_t crashFd = (it != fds_to_remap.end()) ? it->first : -1;
  int32_t crashAnnotationFd = (it != fds_to_remap.end()) ? it->first : -1;

There seems to be a missing increment there, so it will use the same file descriptor for both of those.

(Also, we probably rethink how fds_to_remap works in general — hard-coding assumptions about what order the elements are in like this seems less than ideal — but that's a separate bug.)

Comment 1

[Alex Gaynor [:Alex_Gaynor]]

Attachment: Bug 1440034 - fixed reporting of annotations at the time of crashes on GeckoView;

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

Comment 2

[Randall Barker [:rbarker]]

Comment on attachment 8952824 [details]
Bug 1440034 - fixed reporting of annotations at the time of crashes on GeckoView;

https://reviewboard.mozilla.org/r/222054/#review227966

Comment 3

[Nicholas Nethercote [inactive]]

The patch makes things better, but it's still not 100% right, AFAICT. Judging from GeckoChildProcessHost::PerformAsyncLaunchInternal(), it's possible for the crashFd to not be inserted (e.g. if CrashReporter::IsDummy() is true), but crashAnnotationFd is unconditionally inserted.

It should probably be changed so that the two crash fds are always either both missing or both present?

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3237c9532102
fixed reporting of annotations at the time of crashes on GeckoView; r=rbarker

Comment 5

[Natalia Csoregi [:nataliaCs]]

Backed out for breaking android builds.

backout: https://hg.mozilla.org/integration/autoland/rev/9ff8aa7fdf52ec4d59c82bc8c670a679ece12169

push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=3237c9532102e87bd57505e5aa7b48589837280d

failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=163611000&repo=autoland&lineNumber=10261

[task 2018-02-22T03:30:40.949Z] 03:30:40     INFO -  In file included from /builds/worker/workspace/build/src/obj-firefox/ipc/glue/Unified_cpp_ipc_glue0.cpp:92:
[task 2018-02-22T03:30:40.949Z] 03:30:40     INFO -  /builds/worker/workspace/build/src/ipc/glue/GeckoChildProcessHost.cpp:1196:23: error: expected ';' at end of declaration
[task 2018-02-22T03:30:40.949Z] 03:30:40     INFO -    int32_t crashFd = -1
[task 2018-02-22T03:30:40.949Z] 03:30:40     INFO -                        ^
[task 2018-02-22T03:30:40.949Z] 03:30:40     INFO -                        ;
[task 2018-02-22T03:30:40.949Z] 03:30:40     INFO -  1 error generated.
[task 2018-02-22T03:30:40.949Z] 03:30:40     INFO -  /builds/worker/workspace/build/src/config/rules.mk:1047: recipe for target 'Unified_cpp_ipc_glue0.o' failed
[task 2018-02-22T03:30:40.949Z] 03:30:40     INFO -  make[4]: *** [Unified_cpp_ipc_glue0.o] Error 1
[task 2018-02-22T03:30:40.949Z] 03:30:40     INFO -  make[4]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/ipc/glue'
[task 2018-02-22T03:30:40.949Z] 03:30:40     INFO -  make[4]: *** Waiting for unfinished jobs....
[task 2018-02-22T03:30:40.950Z] 03:30:40     INFO -  make[4]: Entering directory '/builds/worker/workspace/build/src/obj-firefox/ipc/ipdl'
[task 2018-02-22T03:30:40.950Z] 03:30:40     INFO -  make[4]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/ipc/ipdl'

Comment 6

[Alex Gaynor [:Alex_Gaynor]]

Comment on attachment 8952824 [details]
Bug 1440034 - fixed reporting of annotations at the time of crashes on GeckoView;

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

Comment 7

[Alex Gaynor [:Alex_Gaynor]]

Ok, let's try this again. Try run to verify it's clean: https://treeherder.mozilla.org/#/jobs?repo=try&revision=cd081b2039a0b71bb537cffbd66e46bcf34abc44&group_state=expanded

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2d731c03025f
fixed reporting of annotations at the time of crashes on GeckoView; r=rbarker

Comment 9

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/2d731c03025f