Closed
Bug 1385667
Opened 7 years ago
Closed 7 years ago
Firefox fails to build with recent GLIBC: error: field 'context' has incomplete type 'google_breakpad::ucontext'
Categories
(Toolkit :: Crash Reporting, defect)
Toolkit
Crash Reporting
Tracking
()
RESOLVED
FIXED
mozilla57
People
(Reporter: zebob.m, Assigned: emilio)
References
Details
Attachments
(3 files)
59 bytes,
text/x-review-board-request
|
ted
:
review+
|
Details |
11.96 KB,
patch
|
Details | Diff | Splinter Review | |
13.89 KB,
patch
|
ted
:
review+
ritu
:
approval-mozilla-esr52+
|
Details | Diff | Splinter Review |
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•7 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•7 years ago
|
Component: General → Crash Reporting
Product: Firefox → Toolkit
Comment 2•7 years ago
|
||
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)
Assignee | ||
Comment 3•7 years ago
|
||
I hit this today...
Assignee | ||
Comment 4•7 years ago
|
||
(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 6•7 years ago
|
||
Reporter | ||
Comment 7•7 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.
Assignee | ||
Comment 8•7 years ago
|
||
(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.
Assignee | ||
Comment 9•7 years ago
|
||
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•7 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•7 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•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/d3970338a22b
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Comment 13•7 years ago
|
||
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 | ||
Comment 14•7 years ago
|
||
(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•7 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?
status-firefox-esr52:
--- → affected
Hi Ted, is this something we should uplift to ESR52.6? If yes, could you please r+ the patch. Thanks!
Flags: needinfo?(ted)
Comment 17•6 years ago
|
||
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+
tracking-firefox-esr52:
--- → 58+
Comment 19•6 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-esr52/rev/57d4fae4dffe
Comment 20•6 years ago
|
||
FWIW, this was fixed in upstream breakpad. https://github.com/google/breakpad/commit/bddcc58860f522a0d4cbaa7e9d04058caee0db9d
You need to log in
before you can comment on or make changes to this bug.
Description
•