Closed Bug 1654846 Opened 4 years ago Closed 3 years ago

make new dump_syms mandatory for automation jobs

Categories

(Firefox Build System :: General, task, P3)

task

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: froydnj, Unassigned)

References

(Blocks 1 open bug)

Details

The actual task described by this bug's title is pretty straightforward: you add a bit of code similar to what we have for WINCHECKSEC:

https://searchfox.org/mozilla-central/rev/227f22acef5c4865503bde9f835452bf38332c8e/toolkit/moz.configure#1976-1985

That runs into two trivial problems:

  1. AArch64 Windows isn't supported (well) by new dump_syms due to its new-fangled unwind information. We don't support the unwind information on the old dump_syms, though, so maybe this is not a problem.
  2. Bug 1621475 says that new dump_syms doesn't support mingw...or maybe just puts files in the wrong place, or something.

Both of these problems are easily overcome by platform checks in configure.

The remaining problem is that plain builds that we run don't set DUMP_SYMS, and thus would fail with such a change. Options that I can see for handling this:

  1. Set DUMP_SYMS for such builds, since dump_syms is installed during bootstrap and we'd pick it up anyway.
  2. Detect that we're doing plain builds (via PERFHERDER_EXTRA_OPTIONS, maybe?) and don't require DUMP_SYMS in such cases.
  3. Don't dump symbols for plain builds (which we should probably be doing anyway...) and key the requirement of DUMP_SYMS off of whether symbol dumping is enabled or not.
  4. Something else I haven't thought of.

Opinions?

If symbols aren't required or used for plain builds I'd go with 3.

Severity: -- → S3
Priority: -- → P3

It seems to me that this was fixed in bug 1686646 and plain builds also use the new dump_syms since it's picked up automatically. Also we've disabled building the old dump_syms altogether. Mike can you confirm this?

Flags: needinfo?(mh+mozilla)
Status: NEW → RESOLVED
Closed: 3 years ago
Flags: needinfo?(mh+mozilla)
Resolution: --- → FIXED

Thanks!

You need to log in before you can comment on or make changes to this bug.