Firefox fails to build with recent GLIBC: error: field 'context' has incomplete type 'google_breakpad::ucontext'

RESOLVED FIXED in Firefox -esr52

Status

()

defect
RESOLVED FIXED
2 years ago
a year ago

People

(Reporter: zebob.m, Assigned: emilio)

Tracking

Trunk
mozilla57
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr5258+ fixed, firefox57 fixed)

Details

Attachments

(3 attachments)

Reporter

Description

2 years ago
When building firefox with a recent GLIBC, the follwoing error occurs:


>make[5]: Entering directory '/builddir/build/BUILD/gecko-dev-65bbd0525acd0a981c3811b11e23cb057a06373d/objdir/toolkit/crashreporter'
>mkdir -p '.deps/'
>/usr/bin/g++ -std=gnu++11 -o Unified_cpp_crashreporter0.o -c -I/builddir/build/BUILD/gecko-dev-65bbd0525acd0a981c3811b11e23cb057a06373d/objdir/dist/stl_wrappers -I/builddir/build/BUILD/gecko-dev-65bbd0525acd0a981c3811b11e23cb057a06373d/objdir/dist/system_wrappers -include /builddir/build/BUILD/gecko-dev-65bbd0525acd0a981c3811b11e23cb057a06373d/config/gcc_hidden.h -DNDEBUG=1 -DTRIMMED=1 -DOS_POSIX=1 -DOS_LINUX=1 -DUNICODE -D_UNICODE -DNO_STABS_SUPPORT -DSTATIC_EXPORTABLE_JS_API -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -I/builddir/build/BUILD/gecko-dev-65bbd0525acd0a981c3811b11e23cb057a06373d/toolkit/crashreporter -I/builddir/build/BUILD/gecko-dev-65bbd0525acd0a981c3811b11e23cb057a06373d/objdir/toolkit/crashreporter -I/builddir/build/BUILD/gecko-dev-65bbd0525acd0a981c3811b11e23cb057a06373d/objdir/ipc/ipdl/_ipdlheaders -I/builddir/build/BUILD/gecko-dev-65bbd0525acd0a981c3811b11e23cb057a06373d/ipc/chromium/src -I/builddir/build/BUILD/gecko-dev-65bbd0525acd0a981c3811b11e23cb057a06373d/ipc/glue -I/builddir/build/BUILD/gecko-dev-65bbd0525acd0a981c3811b11e23cb057a06373d/toolkit/crashreporter/google-breakpad/src -I/builddir/build/BUILD/gecko-dev-65bbd0525acd0a981c3811b11e23cb057a06373d/toolkit/crashreporter/breakpad-client -I/builddir/build/BUILD/gecko-dev-65bbd0525acd0a981c3811b11e23cb057a06373d/toolkit/crashreporter/google-breakpad/src -I/builddir/build/BUILD/gecko-dev-65bbd0525acd0a981c3811b11e23cb057a06373d/objdir/dist/include  -I/usr/include/nspr4 -I/usr/include/nss3       -fPIC  -DMOZILLA_CLIENT -include /builddir/build/BUILD/gecko-dev-65bbd0525acd0a981c3811b11e23cb057a06373d/objdir/mozilla-config.h -MD -MP -MF .deps/Unified_cpp_crashreporter0.o.pp  -Wall -Wc++11-compat -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wsign-compare -Wtype-limits -Wunreachable-code -Wwrite-strings -Wno-invalid-offsetof -Wc++14-compat -Wc++1z-compat -Wimplicit-fallthrough -Wno-error=maybe-uninitialized -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=free-nonheap-object -Wformat -Wformat-overflow=2 -O2 -g -pipe -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -m64 -mtune=generic -Wformat-security -Wformat -Werror=format-security -fno-delete-null-pointer-checks -fPIC -Wl,-z,relro -Wl,-z,now -fno-exceptions -fno-strict-aliasing -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fno-math-errno -pthread -pipe  -g -g -O2 -fno-omit-frame-pointer   -Wno-unused-local-typedefs -Wno-shadow -Wno-deprecated-declarations -Wno-bool-compare -Wno-unused-but-set-variable -Wno-shadow  /builddir/build/BUILD/gecko-dev-65bbd0525acd0a981c3811b11e23cb057a06373d/objdir/toolkit/crashreporter/Unified_cpp_crashreporter0.cpp
>In file included from /builddir/build/BUILD/gecko-dev-65bbd0525acd0a981c3811b11e23cb057a06373d/toolkit/crashreporter/nsExceptionHandler.cpp:68:0,
>                 from /builddir/build/BUILD/gecko-dev-65bbd0525acd0a981c3811b11e23cb057a06373d/objdir/toolkit/crashreporter/Unified_cpp_crashreporter0.cpp:11:
>/builddir/build/BUILD/gecko-dev-65bbd0525acd0a981c3811b11e23cb057a06373d/toolkit/crashreporter/breakpad-client/linux/handler/exception_handler.h:194:21: error: field 'context' has incomplete type 'google_breakpad::ucontext'
>     struct ucontext context;
>                     ^~~~~~~
>/builddir/build/BUILD/gecko-dev-65bbd0525acd0a981c3811b11e23cb057a06373d/toolkit/crashreporter/breakpad-client/linux/handler/exception_handler.h:194:12: note: forward declaration of 'struct google_breakpad::ucontext'
>     struct ucontext context;
>            ^~~~~~~~
>make[5]: Leaving directory '/builddir/build/BUILD/gecko-dev-65bbd0525acd0a981c3811b11e23cb057a06373d/objdir/toolkit/crashreporter'


The error occurs because ucontext declared in <sys/ucontext.h> has been recently renamed in recent version of the GLIBC to ucontext_t: see commit https://sourceware.org/git/?p=glibc.git;a=commit;f=sysdeps/i386/sys/ucontext.h;h=251287734e89a52da3db682a8241eb6bccc050c9
Reporter

Comment 1

2 years ago
@ted.mielczarek You're the one who forked breakpad client into Firefox, could you have a look at this?
Flags: needinfo?(ted)

Updated

2 years ago
Component: General → Crash Reporting
Product: Firefox → Toolkit
It looks like we could probably just change `struct ucontext` to `ucontext_t` there:
https://dxr.mozilla.org/mozilla-central/rev/5322c03f4c8587fe526172d3f87160031faa6d75/toolkit/crashreporter/breakpad-client/linux/handler/exception_handler.h#194

Since the typedef to `ucontext_t` has existed this whole time and still exists.
Flags: needinfo?(ted)
I hit this today...
(The triple dots was because I'm writing a patch, didn't intend to be mean btw :P)
Assignee: nobody → emilio+bugs
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Comment hidden (mozreview-request)
Reporter

Comment 7

2 years ago
The patch will work with new version of GLIBC, but what if one tries to build with an older version of GLIBC? It will fail too because ucontext_t is not defined. I patched my build with a lot of #if __GLIBC__ == 2 && __GLIBC_MINOR__ < 26 because I need to build Firefox on platforms with different versions of GLIBC.

I attached the patch I use to show what I mean.
(In reply to Robert-André Mauchin from comment #7)
> The patch will work with new version of GLIBC, but what if one tries to
> build with an older version of GLIBC? It will fail too because ucontext_t is
> not defined. I patched my build with a lot of #if __GLIBC__ == 2 &&
> __GLIBC_MINOR__ < 26 because I need to build Firefox on platforms with
> different versions of GLIBC.
> 
> I attached the patch I use to show what I mean.

As ted mentioned in comment 2, GLIBC has changed from:

  typedef struct ucontext { ... } ucontext_t;

to:

  typedef struct ucontext_t { ... } ucontext_t;

Thus, my patch, that switches everything to just ucontext_t, should be enough to build with both old and new versions of glibc.
To prove this builds with older glibc versions, an automation build with this patch:

 * https://treeherder.mozilla.org/#/jobs?repo=try&revision=95eaad1a25c036729b04e15f8b9a10b1d707415a

Comment 10

2 years ago
mozreview-review
Comment on attachment 8899165 [details]
Bug 1385667: Use ucontext_t consistently in breakpad-client.

https://reviewboard.mozilla.org/r/170444/#review175796
Attachment #8899165 - Flags: review?(ted) → review+

Comment 11

2 years ago
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/d3970338a22b
Use ucontext_t consistently in breakpad-client. r=ted

Comment 12

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/d3970338a22b
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
I still have problems building with glibc 2.26 even after this patch:

> 3:44.64 In file included from /build/firefox/src/mozilla-unified/obj-i686-pc-linux-gnu/toolkit/crashreporter/breakpad-client/linux/Unified_cpp_linux0.cpp:29:0:
> 3:44.64 /build/firefox/src/mozilla-unified/toolkit/crashreporter/breakpad-client/linux/dump_writer_common/ucontext_reader.cc:51:63: error: ‘ucontext’ does not name a type; did you mean ‘ucontext_t’?
> 3:44.64  void UContextReader::FillCPUContext(RawContextCPU *out, const ucontext *uc,
> 3:44.64                                                                ^~~~~~~~
> 3:44.64                                                                ucontext_t
> 3:44.64 /build/firefox/src/mozilla-unified/toolkit/crashreporter/breakpad-client/linux/dump_writer_common/ucontext_reader.cc:51:6: error: prototype for ‘void google_breakpad::UContextReader::FillCPUContext(google_breakpad::RawContextCPU*, const int*, const _libc_fpstate*)’ does not match any in class ‘google_breakpad::UContextReader’
> 3:44.64  void UContextReader::FillCPUContext(RawContextCPU *out, const ucontext *uc,
> 3:44.64       ^~~~~~~~~~~~~~
> 3:44.64 In file included from /build/firefox/src/mozilla-unified/toolkit/crashreporter/breakpad-client/linux/dump_writer_common/ucontext_reader.cc:30:0,
> 3:44.64                  from /build/firefox/src/mozilla-unified/obj-i686-pc-linux-gnu/toolkit/crashreporter/breakpad-client/linux/Unified_cpp_linux0.cpp:29:
> 3:44.64 /build/firefox/src/mozilla-unified/toolkit/crashreporter/breakpad-client/linux/dump_writer_common/ucontext_reader.h:52:15: error: candidate is: static void google_breakpad::UContextReader::FillCPUContext(google_breakpad::RawContextCPU*, const ucontext_t*, const _libc_fpstate*)
> 3:44.64    static void FillCPUContext(RawContextCPU *out, const ucontext_t *uc,
> 3:44.64                ^~~~~~~~~~~~~~

Seems some instances of "const ucontext *uc" were missed?
Assignee

Updated

2 years ago
Depends on: 1394149
(In reply to Jan Alexander Steffens from comment #13)
> Seems some instances of "const ucontext *uc" were missed?

Yup, those are arch-dependent, and I missed them, d'oh. Filed bug 1394149 and posted a patch.

Comment 15

2 years ago
[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration: Fix building with later GLIBC on ESR
User impact if declined: Local builders cannot build with later GLIBC
Fix Landed on Version: 57
Risk to taking this patch (and alternatives if risky): None
String or UUID changes made by this patch: None

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.
Attachment #8936147 - Flags: review?(ted)
Attachment #8936147 - Flags: approval-mozilla-esr52?
Hi Ted, is this something we should uplift to ESR52.6? If yes, could you please r+ the patch. Thanks!
Flags: needinfo?(ted)
Comment on attachment 8936147 [details] [diff] [review]
breakpad_linux.diff

This is a pretty inoffensive patch. It doesn't actually change anything for our official builds, but if it unbreaks compiling ESR with a newer glibc then it's probably worth landing it just to remove that headache.
Flags: needinfo?(ted)
Attachment #8936147 - Flags: review?(ted) → review+
Comment on attachment 8936147 [details] [diff] [review]
breakpad_linux.diff

Thanks Ted. Let's take it then. ESR52+
Attachment #8936147 - Flags: approval-mozilla-esr52? → approval-mozilla-esr52+
You need to log in before you can comment on or make changes to this bug.