1952657 - abseil-cpp tries to include non-existent "execinfo.h" header on musl+loongarch64/armv7

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Patrycja Rosa [:ptrcnull] (she/her)]

the mechanism for wrapping system includes is just iterating over a massive array of headers, not checking whether they exist or not, but abseil-cpp uses a __has_include(<execinfo.h>) to check whether they can import it safely; that breaks when the header is unconditionally wrapped by the build system:

22:07.93 In file included from Unified_cpp_stacktrace_gn0.cpp:2:
22:07.94 In file included from /builds/alpine/aports/community/firefox/src/firefox-136.0/third_party/abseil-cpp/absl/debugging/stacktrace.cc:46:
22:07.94 In file included from /builds/alpine/aports/community/firefox/src/firefox-136.0/third_party/abseil-cpp/absl/debugging/internal/stacktrace_generic-inl.inc:23:
22:07.94 /builds/alpine/aports/community/firefox/src/firefox-136.0/obj/dist/system_wrappers/execinfo.h:3:15: fatal error: 'execinfo.h' file not found
22:07.94     3 | #include_next <execinfo.h>
22:07.94       |               ^~~~~~~~~~~~
22:07.94 1 error generated.
22:07.96 gmake[4]: *** [/builds/alpine/aports/community/firefox/src/firefox-136.0/config/rules.mk:676: Unified_cpp_stacktrace_gn0.o] Error 1

Comment 1

[[:sergesanspaille]]

Attachment: WIP: Bug 1952657 - Check if system header exists before forwarding it in system-wrappers

Comment 2

[[:sergesanspaille]]

Patrycja, would https://phabricator.services.mozilla.com/D240842 do the trick?

Comment 3

[Patrycja Rosa [:ptrcnull] (she/her)]

that would not work here - stacktrace_config.h chooses which implementation to use based on the existence of the header, and it chooses one that uses symbols from said header; if the header ends up being empty, that will just result in missing symbols and undefined functions.

Comment 4

[Patrycja Rosa [:ptrcnull] (she/her)]

Attachment: Bug 1952657 - Only wrap header execinfo.h when it actually exists. r=dbaker,mjf,glandium

Comment 5

[Ryan VanderMeulen (PTO, back July 27)]

From the D248421 review:

Honestly, I'd take this as an occasion to reflect on whether we actually want abseil to have its own stack unwinder. Or even, if we make any practical use of that code at all.

I poked at this as part of restructuring abseil-cpp into a standalone Library("abseil") target in Firefox (Bug 2038068). Glandium's comment is on point. Firefox does not use absl's stack unwinder, it has its own stack unwinding infrastructure. absl/debugging/stacktrace.cc and the related debugging targets (symbolize, examine_stack, failure_signal_handler, etc.) are pulled in transitively by the GN //:absl target but none of their symbols are referenced by any Firefox code.

Seems safe to drop.

Comment 6

[Ryan VanderMeulen (PTO, back July 27)]

Taking a closer look and attempting a Try push with the absl/debugging directory completely excluded from the build, there were linker failures because absl/log calls absl::GetStackTrace and absl::Symbolize.
https://treeherder.mozilla.org/logviewer?job_id=565344641&repo=try&task=ds7X9qCeSmi1-zCBH34VHQ.0&lineNumber=120563

So it seems that trying to omit their stack unwinder from our builds isn't really a feasible option after all and the attached patch is the correct functional approach to take. While we could maybe theoretically patch abseil-cpp to never use the execinfo.h header, that seems more invasive and troublesome than it's worth?

Comment 7

[Mike Hommey [:glandium]]

How about stubbing out absl::GetStackTrace and absl::Symbolize? I don't think we have much use for that log output (but I could be wrong).
That said, that doesn't need to block fixing this.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/b24a8076022f
https://hg.mozilla.org/integration/autoland/rev/9192890a06d6
Only wrap header execinfo.h when it actually exists. r=glandium,firefox-build-system-reviewers

Comment 9

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/9192890a06d6