1510574 - Linux32 crash symbols missing at least in mochitests

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Narcis Beleuzu [:NarcisB]]

Seems to have started from 
https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Crunnable&selectedJob=214301297&revision=eb60816546795a5fad643951818dbd4ea9b711ea

Comment 2

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

Can you take a look why the symbols are missing here, Gabriele? Thank you.

Comment 4

[Gabriele Svelto [:gsvelto]]

I grabbed a minidump from the failing ones and the issue there was fairly silly. Breakpad's minidump-reading logic fails if the number of modules in the modules list exceeds a certain number:

https://searchfox.org/mozilla-central/rev/0859e6b10fb901875c80de8f8fc33cbb77b2505e/toolkit/crashreporter/google-breakpad/src/processor/minidump.cc#2725

This can be overridden but minidump_stackwalk sticks with the default which is 2048:

https://searchfox.org/mozilla-central/rev/0859e6b10fb901875c80de8f8fc33cbb77b2505e/toolkit/crashreporter/google-breakpad/src/processor/minidump.cc#3861

The minidump I looked at had 5800+ modules (mostly because of deleted mappings). I bumped up the limit locally and the minidump was processed just fine, symbolicated stack and all. It took ~7 seconds on my machine which is a fair bit longer than usual but nothing dramatic.

Ted, do you think we can raise this limit? minidump_dump already sets it to MAX_UINT32 so maybe we can go a bit higher than 2048 without suffering adverse effects.

Comment 5

[Gabriele Svelto [:gsvelto]]

Attachment: Bug 1510574 - Increase the maximum number of modules minidump_stackwalk is allowed to read

Comment 6

[Bob Clary [:bc] (inactive)]

you'll need using google_breakpad::MinidumpModuleList; as well. btw, this fixes a long standing issue for me though I also included the change to minidump.cc as well though not sure it is required.

Comment 7

[Bob Clary [:bc] (inactive)]

May have spoken too soon. Partially fixed it appears.

Comment 8

[Intermittent Failures Robot]

4 failures in 2808 pushes (0.001 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-inbound: 3
* mozilla-central: 1

Platform breakdown:
* linux64: 2
* linux64-qr: 2

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1510574&startday=2018-11-26&endday=2018-12-02&tree=all

Comment 9

[(not currently active) Ted Mielczarek]

I commented in Phabricator, but we don't currently build the in-tree code for use in automation. glandium updated the (very old) binaries we had been using in bug 1512706, but punted on a more complete solution because it got complicated. It looked like he might fix bug 1391408 though.

Comment 10

[Mike Hommey [:glandium]]

I will, but I also won't go further to hook it up for test jobs to download it from the toolchain tasks.

Comment 11

[Gabriele Svelto [:gsvelto]]

I was wondering, would it make sense to build minidump_stackwalk as an host binary like we do with dump_syms instead of taking it from outside of mozilla-central?

Comment 12

[Mike Hommey [:glandium]]

That's essentially what I have WIP patches for (except it's not /exactly/ building them as host binaries)

Comment 13

[Boris Zbarsky [:bzbarsky]]

Would this affect Linux64 too?  I've been running into this on my Linux64 try pushes for at least a week, and it's a huge pain... If this bug doesn't cover Linux64, I'm happy to file a separate one to track that.

Comment 14

[Gabriele Svelto [:gsvelto]]

(In reply to Boris Zbarsky [:bzbarsky, bz on IRC] from comment #13)
> Would this affect Linux64 too?  I've been running into this on my Linux64
> try pushes for at least a week, and it's a huge pain...

Yes, it would affect all platforms.

Comment 15

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

(In reply to Gabriele Svelto [:gsvelto] from comment #4)
> The minidump I looked at had 5800+ modules (mostly because of deleted mappings).

Is there a way to convince breakpad to ignore non-executable mappings?  If it weren't picking up shared memory and data files, that number would probably be a lot smaller.

(Shared memory segments on Linux currently show up as deleted files, because they are, but I've also seen breakpad pick up memfd mappings created by system libraries so bug 1440203 probably won't help here.)

Comment 16

[(not currently active) Ted Mielczarek]

The Linux dump writing code does all sorts of fiddling with the list of mappings, so we could absolutely filter out stuff that's not useful:
https://dxr.mozilla.org/mozilla-central/rev/c2593a3058afdfeaac5c990e18794ee8257afe99/toolkit/crashreporter/breakpad-client/linux/minidump_writer/linux_dumper.cc#575

We do stuff /proc/self/maps into the minidump verbatim (it's hard to get at without running minidump_dump, but it's there), so I don't think we'd lose much by doing that.

Comment 17

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

(In reply to Ted Mielczarek [:ted] [:ted.mielczarek] from comment #16)

The Linux dump writing code does all sorts of fiddling with the list of
mappings, so we could absolutely filter out stuff that's not useful:
https://dxr.mozilla.org/mozilla-central/rev/
c2593a3058afdfeaac5c990e18794ee8257afe99/toolkit/crashreporter/breakpad-
client/linux/minidump_writer/linux_dumper.cc#575

If I'm following that correctly, it merges adjacent mappings from the same file, and tracks whether any of those mappings was executable… but does Breakpad ever need to do anything with non-executable segments?

In any case, for code that isn't actually a library or executable, MappingInfo::exec should always be false, and vice versa.

Comment 18

[(not currently active) Ted Mielczarek]

No, Breakpad has no need for non-executable segments.

Comment 19

[Mike Hommey [:glandium]]

(In reply to Ted Mielczarek [:ted] [:ted.mielczarek] from comment #18)

No, Breakpad has no need for non-executable segments.

... And yet, they are included in crash reports as modules.

Comment 20

[Gabriele Svelto [:gsvelto]]

(In reply to Mike Hommey [:glandium] from comment #19)

... And yet, they are included in crash reports as modules.

I'm on PTO this week but I'll resume work on this ASAP. The starting point as we had discussed is to fork breakpad for good and do all the adjustments we need to fix our short-term pain-points. I would also like to fix this issue as quickly as possible. I've got patches to build minidump_stackwalker as a host tool, Mike can you attach your changes so that I can integrate them? What I still need to do is adjust our automation script to pick up the host tool instead of using tooltool but that shouldn't be too hard.

Comment 21

[Mike Hommey [:glandium]]

I've got patches to build minidump_stackwalker as a host tool

That's not enough. Because the host is not necessarily where it's going to run. For one, minidump_stackwalk doesn't build on Windows, so windows builds need one built with mingw. And mac tests need a native mac one, but the host for mac builds is linux. Which is why I went with separate jobs to build minidump_stackwalk. Which should all be landed by the time you're back from PTO.

In any case, minidump_stackwalk is not involved in putting modules in minidumps.

Comment 22

[Gabriele Svelto [:gsvelto]]

(In reply to Mike Hommey [:glandium] from comment #21)

That's not enough. Because the host is not necessarily where it's going to run. For one, minidump_stackwalk doesn't build on Windows, so windows builds need one built with mingw. And mac tests need a native mac one, but the host for mac builds is linux. Which is why I went with separate jobs to build minidump_stackwalk. Which should all be landed by the time you're back from PTO.

That's nasty.

In any case, minidump_stackwalk is not involved in putting modules in minidumps.

You're right, I was still stuck on the idea of increasing the maximum number of modules allowed by the tool but it's much easier to trim down the module list when generating the dump. I'll hack the client code ASAP to do that.

Comment 23

[Gabriele Svelto [:gsvelto]]

OK, I've got a patch that removes the shared memory segments from the module list which seems to be working correctly. Getting rid of all the non-executable modules doesn't work because it messes up Breakpad's stack scanning logic. That logic should never be looking at pointers within an IPC shared segment so not having those modules shouldn't be a problem.

Comment 24

[Gabriele Svelto [:gsvelto]]

Attachment: Bug 1510574 - Remove shared memory segments from generated minidumps to cut down on the number of modules

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/30fd54bb8089
Remove shared memory segments from generated minidumps to cut down on the number of modules r=ted

Comment 26

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/30fd54bb8089

Comment 27

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

For reference, I've made some notes about future changes that will need adjustments to the path filtering (bug 1426526 comment #2, bug 1440203 comment #6).