Closed Bug 1041886 Opened 5 years ago Closed 5 years ago
Separate Linux sandbox code into its own shared library
Windows Firefox builds the sandbox code as its own shared library so that it can use versions of the chromium/base code that would otherwise conflict with those from IPC. (See also bug 925471.) We're going to need this on Linux if we want to import any more of the Chromium sandboxing code there (and we do, in fact, want that). It will also allow getting rid of the unsightly hacks at the bottom of security/sandbox/linux/Sandbox.cpp (or, at least, allow sharing the equivalent of those hacks with other OSes — see security/sandbox/chromium/base/shim/base.)
Step 0: break out the logging macro into a header file, because it will be needed in more than one module in the next patch. Also rename it, because having something with as generic a name as LOG_ERROR in an include file seems not right.
Attachment #8474930 - Flags: review?(gdestuynder)
…and this is the patch that actually moves code around and creates the separate library for the sandbox. I'm not sure if this needs build peer review, but it probably can't hurt to ask for it, especially because this is the first time I've tried to do something like this to the build config.
Comment on attachment 8474931 [details] [diff] [review] bug1041886-p1-sandbox-lib-hg1.diff Review of attachment 8474931 [details] [diff] [review]: ----------------------------------------------------------------- ::: security/sandbox/linux/glue/moz.build @@ +5,5 @@ > +# file, You can obtain one at http://mozilla.org/MPL/2.0/. > + > +FAIL_ON_WARNINGS = True > + > +SOURCES += ['SandboxCrash.cpp'] SOURCES += [ 'SandboxCrash.cpp', ] same for others
Attachment #8474931 - Flags: review?(mh+mozilla) → review+
Attachment #8474930 - Flags: review?(gdestuynder) → review+
Attachment #8474931 - Flags: review?(gdestuynder) → review+
Changeset 2f9d0821e08c breaks the build on Linux: 0:17.04 ../../../build/unix/gold/ld: error: /home/kinetik/mozilla/inbound/obj-x86_64-unknown-linux-gnu/security/sandbox/linux/Sandbox.o: requires dynamic R_X86_64_PC32 reloc against '_ZN7mozilla6unusedE' which may overflow at runtime; recompile with -fPIC 0:17.04 ../../../build/unix/gold/ld: error: read-only segment has dynamic relocations 0:17.04 /home/kinetik/mozilla/inbound/security/sandbox/linux/Sandbox.cpp:403: error: undefined reference to 'mozilla::unused' 0:17.04 collect2: error: ld returned 1 exit status
I also hit this link error on my Linux box.
Sounds like a visibility issue.
I can't reproduce, fwiw. What's your version of gold? (gold --version)
Are the failing builds -O0, maybe? That might explain why I have no actual references to mozilla::unused, and neither my builds nor try/buildbot had problems. For some reason I thought mozilla::unused was MFBT, but it's actually xpcom/glue. In any case, this *should* be a one-line fix.
(In reply to Mike Hommey [:glandium] (out from Sep 6 to Sep 22) from comment #8) > I can't reproduce, fwiw. What's your version of gold? (gold --version) ~% gold --version zsh: command not found: gold  ~% ld.gold --version GNU gold (version 2.23.2) 1.11 (In reply to Jed Davis [:jld] from comment #9) > Are the failing builds -O0, maybe? --disable-optimize, so yes.
https://hg.mozilla.org/integration/mozilla-inbound/rev/a92aab848977 — fix for no-opt breakage, tested locally.
Fixes the problem for me. Thanks!
(In reply to Jed Davis from comment #11) > https://hg.mozilla.org/integration/mozilla-inbound/rev/a92aab848977 — fix > for no-opt breakage, tested locally. Have you filed a followup to get this fixed properly?
Aha, bug 1059038 is mentioned in a separate line in your checkin comment, which is why I didn't notice it the first time around. Sorry about that.
https://hg.mozilla.org/mozilla-central/rev/b3dcb5b33f78 https://hg.mozilla.org/mozilla-central/rev/2f9d0821e08c https://hg.mozilla.org/mozilla-central/rev/a92aab848977
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
This also breaks -flto build on Linux: Executing: /var/tmp/gcc_test/usr/local/bin/c++ -Wall -Wpointer-arith -Woverloaded-virtual -Werror=return-type -Werror=int-to-pointer-cast -Werror=type-limits -Wempty-body -Wsign-compare -Wno-invalid-offsetof -Wcast-align -flto=4 -ffunction-sections -fdata-sections -fno-exceptions -fno-strict-aliasing -fno-rtti -fno-exceptions -fno-math-errno -std=gnu++0x -pthread -pipe -DNDEBUG -DTRIMMED -O3 -fomit-frame-pointer -fPIC -shared -Wl,-z,defs -Wl,-h,libmozsandbox.so -o libmozsandbox.so /var/tmp/moz-build-dir/security/sandbox/linux/tmpZIKfIW.list -lpthread -Wl,--hash-style=gnu,--as-needed,--gc-sections,--icf=all,--icf-iterations=3 -Wl,-z,noexecstack -Wl,-z,text -Wl,--build-id -Wl,-rpath-link,/var/tmp/moz-build-dir/dist/bin -Wl,-rpath-link,/usr/lib -ldl -lrt /var/tmp/moz-build-dir/security/sandbox/linux/tmpZIKfIW.list: INPUT("unused.o") INPUT("logging.o") INPUT("basicblock.o") INPUT("codegen.o") INPUT("die.o") INPUT("syscall.o") INPUT("Sandbox.o") INPUT("SandboxAssembler.o") INPUT("SandboxFilter.o") /usr/bin/ld: error: /tmp/ccAwcilU.ltrans5.ltrans.o: requires dynamic R_X86_64_PC32 reloc against 'SyscallAsm' which may overflow at runtime; recompile with -fPIC /usr/bin/ld: error: read-only segment has dynamic relocations /tmp/ccAwcilU.ltrans5.ltrans.o:ccAwcilU.ltrans5.o:function sandbox::Die::SandboxDie(char const*, char const*, int) [clone .constprop.19]: error: undefined reference to 'SyscallAsm' /tmp/ccAwcilU.ltrans5.ltrans.o:ccAwcilU.ltrans5.o:function sandbox::Die::SandboxDie(char const*, char const*, int) [clone .constprop.19]: error: undefined reference to 'SyscallAsm' collect2: error: ld returned 1 exit status /var/tmp/mozilla-central/config/rules.mk:829: recipe for target 'libmozsandbox.so' failed An easy fix would be to disable -flto for syscall.cc: diff --git a/security/sandbox/linux/moz.build b/security/sandbox/linux/moz.build index 2cbbccf3bf48..c005d545defc 100644 --- a/security/sandbox/linux/moz.build +++ b/security/sandbox/linux/moz.build @@ -25,6 +25,9 @@ SOURCES += [ 'SandboxFilter.cpp', ] +if '-flto' in CONFIG['OS_CXXFLAGS']: + SOURCES['../chromium/sandbox/linux/seccomp-bpf/syscall.cc'].flags += ['-fno-lto'] + DEFINES['NS_NO_XPCOM'] = True DISABLE_STL_WRAPPING = True
See: https://gcc.gnu.org/wiki/LinkTimeOptimizationFAQ#Symbol_usage_from_assembly_language for further info.
You need to log in before you can comment on or make changes to this bug.