Build dump_syms locally if possible on Windows

RESOLVED FIXED in Firefox 48

Status

()

defect
RESOLVED FIXED
9 years ago
2 years ago

People

(Reporter: ted, Assigned: poiru)

Tracking

Trunk
mozilla48
x86
Windows 7
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox48 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

bug 575519 is a band-aid for an annoying situation. We have a checked-in binary of dump_syms.exe on Windows, because it requires VC++ Pro to build it (but it will run with VC++ Express). However, it depends on the specific version of the DIA DLLs installed with the compiler, so the VC8 binary that's checked in won't work if you don't have VC8 installed. The band-aid is just to add a VC9 copy, but then we're still broken if someone has only VC10 installed.

We should just add a configure test to see if we can build it, and if so, build a local copy instead of relying on the checked-in binary. That way, if a user has VC++ Pro, they don't have to worry about what binaries we have in-tree.
See Also: → 575519

Comment 1

7 years ago
Posted patch win host dump_syms.exe (obsolete) — Splinter Review
Assignee

Comment 2

3 years ago
This will compile with any version of VS2015 (even the free Community version has the DIA SDK and ATL).

Note that mozilla-build does not add the DIA SDK to INCLUDE/LIB at the moment. I won't land that until we release a fixed mozilla-build.
Assignee: nobody → birunthan
Attachment #657524 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8703424 - Flags: review?(ted)
Assignee

Updated

3 years ago
Blocks: 1198374, VC14
Comment on attachment 8703424 [details] [diff] [review]
Build dump_syms on Windows when using VS2015

Review of attachment 8703424 [details] [diff] [review]:
-----------------------------------------------------------------

Cool, this has always bothered me! This might conflict slightly with my patches in bug 1069556, but I don't think it'll be too bad.

::: Makefile.in
@@ +193,5 @@
>  MAKE_SYM_STORE_ARGS := -c --vcs-info
>  ifdef PDBSTR_PATH
>  MAKE_SYM_STORE_ARGS += -i
>  endif
> +ifeq ($(shell test $(_MSC_VER) -ge 1900; echo $$?),0)

Put this test in configure somewhere and AC_SUBST a special var for it. (Alternately, just set DUMP_SYMS_BIN completely in configure and AC_SUBST it).
Attachment #8703424 - Flags: review?(ted)
Assignee

Updated

3 years ago
Attachment #8703424 - Attachment is obsolete: true
Comment on attachment 8726830 [details] [diff] [review]
Build dump_syms on Windows when using VS2015

Review of attachment 8726830 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, thanks! Presumably we can remove all the conditionals if we drop support for MSVC 2013 in the future?
Attachment #8726830 - Flags: review?(ted) → review+
Comment on attachment 8726830 [details] [diff] [review]
Build dump_syms on Windows when using VS2015

Review of attachment 8726830 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/crashreporter/google-breakpad/src/tools/windows/dump_syms/moz.build
@@ +6,5 @@
> +
> +HostProgram('dump_syms')
> +
> +HOST_SOURCES += [
> +    '../../../common/windows/guid_string.cc',

'../../../common/windows/omap.cc' needs to be added.
Comment on attachment 8726830 [details] [diff] [review]
Build dump_syms on Windows when using VS2015

Review of attachment 8726830 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/crashreporter/google-breakpad/src/tools/windows/dump_syms/moz.build
@@ +6,5 @@
> +
> +HostProgram('dump_syms')
> +
> +HOST_SOURCES += [
> +    '../../../common/windows/guid_string.cc',

We also need dia_util.cc from the same directory as everything else.

I can confirm dump_syms builds with dia_util.cc and omap.cc added.
Since I had made the changes to add the 2 missing source files, I just figured I'd land those.

Comment 13

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/43092fb2943c
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Depends on: 1254967
Duplicate of this bug: 374649
See Also: → 1373354
You need to log in before you can comment on or make changes to this bug.