PF4Public / gentoo-overlay

Personal Gentoo overlay
78 stars 18 forks source link

www-client/ungoogled-chromium needs clarification on building with cfi #40

Closed 0xC0ncord closed 4 years ago

0xC0ncord commented 4 years ago

Description This is a follow-up to #32. I have successfully built ungoogled-chromium with the cfi USE flag. The warning message in the ebuild should clarify instructions to the user if they wish to use it.

On my system, I was only able to build ungoogled-chromium if my clang+llvm toolchain was built with libc++ instead of libstdc++. Additionally, I had to build ungoogled-chromium using libc++ as well.

Previously, in #32, my toolchain was not properly bootstrapped for some reason, and an entire rebuild of the toolchain fixed this. In my testing, if I was not using libc++, the torque build generator used by chromium would fail with an invalid opcode at some (reproducible) point during the build if cfi was enabled. Only after rebuilding the toolchain with libc++ did this stop occurring.

Lastly, in my build, I was forced to disable system-jsoncpp because of linker errors related to this problem that I have not yet resolved, and disabling system-jsoncpp works around it. Additionally, dev-libs/re2 (and potentially other dependencies) needed to be rebuilt after switching toolchains, otherwise, disabling the system-* USE flags may suffice.

The warning in the ebuild should be updated to include this information if the user wishes to enable the cfi USE flag.

www-client/ungoogled-chromium-84.0.4147.89_p1::pf4public was built with the following:
USE="cfi clang convert-dict cups libcxx optimize-thinlto optimize-webui proprietary-codecs pulseaudio (selinux) suid system-ffmpeg system-harfbuzz system-icu system-libevent system-libvpx system-openh264 system-openjpeg tcmalloc thinlto vaapi -closure-compile -custom-cflags -enable-driver -hangouts -headless -kerberos -ozone -system-jsoncpp -vdpau -wayland -widevine" ABI_X86="(64)" L10N="-am -ar -bg -bn -ca -cs -da -de -el -en-GB -es -es-419 -et -fa -fi -fil -fr -gu -he -hi -hr -hu -id -it -ja -kn -ko -lt -lv -ml -mr -ms -nb -nl -pl -pt-BR -pt-PT -ro -ru -sk -sl -sr -sv -sw -ta -te -th -tr -uk -vi -zh-CN -zh-TW" PYTHON_TARGETS="python2_7 python3_8 -python3_6 -python3_7 -python3_9"
CFLAGS="-march=skylake -pipe -stdlib=libc++ -Wno-unknown-warning-option -Wno-builtin-macro-redefined"
CXXFLAGS="-march=skylake -pipe -stdlib=libc++ -flax-vector-conversions=all -Wno-unknown-warning-option -Wno-builtin-macro-redefined"
LDFLAGS="-Wl,--as-needed -Wl,--sort-common -fuse-ld=lld -Wl,-S -stdlib=libc++ -Wl,-plugin-opt,-import-instr-limit=30 -Wl,--thinlto-jobs=9"

/etc/portage/make.conf

CC="clang"
CXX="clang++"
AR="llvm-ar"
NM="llvm-nm"
RANLIB="llvm-ranlib"
COMMON_FLAGS="-march=skylake -O2 -pipe -fomit-frame-pointer"
LTO_FLAGS="-flto=thin"
CFLAGS="${COMMON_FLAGS} ${LTO_FLAGS}"
CXXFLAGS="${COMMON_FLAGS} ${LTO_FLAGS} -stdlib=libc++"
FCFLAGS="${COMMON_FLAGS}"
FFLAGS="${COMMON_FLAGS}"
LDFLAGS="-Wl,-O3 -Wl,--as-needed -Wl,--sort-common -fuse-ld=lld -Wl,-S -rtlib=compiler-rt -unwindlib=libunwind -stdlib=libc++"
MAKEOPTS="-j9"
CPU_FLAGS_X86="aes avx avx2 f16c fma3 mmx mmxext pclmul popcnt sse sse2 sse3 sse4_1 sse4_2 ssse3"
PF4Public commented 4 years ago

Awesome! Thank you for testing this out! Looks like the key issue was to build with libc++. Would it suffice just to add a requirement of libcxx USE flag for cfi?

0xC0ncord commented 4 years ago

Would it suffice just to add a requirement of libcxx USE flag for cfi?

I will test to see if this is all that is required. In my toolchain, I am also using compiler-rt and libunwind, as these seem like standard practice for clang+llvm toolchains. On my other system where I will try this, I will start from a clean slate and only enable libcxx and we will see what happens.

0xC0ncord commented 4 years ago

Quick update: I am still trying other ways to build ungoogled-chromium on this other system. I want to see if I am able to build it even without -rtlib=compiler-rt and/or -unwindlib=libunwind. I am cataloguing every hurdle I run into as I do this.

PF4Public commented 4 years ago

I am cataloguing every hurdle I run into as I do this.

Are there a lot of them?

0xC0ncord commented 4 years ago

Are there a lot of them?

Not generally speaking. What's taking a while is needing to rebuild the toolchain every time I change any flag that pertains to it, then rebuilding ungoogled-chromium and taking note of the errors.

0xC0ncord commented 4 years ago

While experimenting with libcxx support (or lack thereof) I ran across this: https://bugs.chromium.org/p/chromium/issues/detail?id=910644

This looks to be the exact same error that was encountered in #32. Somehow, it looks like this still occurs in my case (on both systems I'm testing on) unless libcxx is being used, and I have no idea why. I'm still doing more testing with libcxx and I should have a complete list of results by the end of today. I at least wanted to share the above bug before I lost it.

PF4Public commented 4 years ago

While experimenting with libcxx support (or lack thereof) I ran across this: https://bugs.chromium.org/p/chromium/issues/detail?id=910644

That is a good find! But it is indeed marked as resolved more than a year ago… Weird.

0xC0ncord commented 4 years ago

Okay, so this is what I have found in my testing. All of the following guidance was done with the cfi USE flag enabled. Any USE changes to the clang+llvm toolchain caused me to bootstrap the toolchain after building it initally with gcc.

If the llvm+clang toolchain is not built with the default-libcxx USE flag, then v8/torque will fail to compile with something like the following:

FAILED: torque 
clang++ -pie -Wl,--build-id=sha1 -fPIC -Wl,-z,noexecstack -Wl,-z,relro -Wl,-z,now -Wl,-z,defs -Wl,--as-needed -fuse-ld=lld -Wl,--icf=all -Wl,--color-diagno
stics -flto=thin -Wl,--thinlto-jobs=8 -Wl,--thinlto-cache-dir=thinlto-cache -Wl,--thinlto-cache-policy,cache_size=10\%:cache_size_bytes=10g:cache_size_file
s=100000 -Wl,--lto-O2 -fwhole-program-vtables -m64 -rdynamic -fsanitize=cfi-vcall -fsanitize=cfi-derived-cast -fsanitize=cfi-unrelated-cast -fsanitize=cfi-
icall -pie -Wl,--disable-new-dtags -Wl,-O2 -Wl,--gc-sections -Wl,--as-needed -Wl,--sort-common -fuse-ld=lld -Wl,-S -Wl,-plugin-opt,-import-instr-limit=30 -
Wl,--thinlto-jobs=13 -o "./torque" -Wl,--start-group @"./torque.rsp"  -Wl,--end-group  -latomic -ldl -lpthread -lrt
ld.lld: error: undefined symbol: _Unwind_Resume

If default-libcxx is enabled, this error does not occur. Note that if the toolchain is built with default-libcxx, then libcxx and libcxxabi are also required, as well as -unwindlib=libunwind in LDFLAGS, or else the toolchain will run into the same errors (on my system it did).

It seems that in order to properly build with cfi, all of the relevant libcxx USE flags need to be enabled in order for these errors to be avoided and for the original invalid opcode error to be worked around, as well as to build all C++ dependencies with -stdlib=libc++ in order for proper linkage.

But, I am not satisfied with this answer because on my system, using -stdlib=libc++ breaks several important packages (namely cmake and qtwebkit). cmake wants to link to jsoncpp and for some reason it cannot do so if jsoncpp is built with libc++, and qtwebkit will not link with re2 if it is built with libc++ for the same reason. The linker errors here are the same as those outlined in my original post or here. The jsoncpp linker error can be avoided by not using the system-jsoncpp USE flag for ungoogled-chromium, but this cannot be used to avoid the linker error with re2 as no such flag is provided. Even so, I don't think that adding a system-re2 USE flag is the proper solution for this.

I should point out that this issue shouldn't be specific to ungoogled-chromium but more than likely chromium as well, only chromium does not provide the cfi USE flag directly in the ebuild.

alexminder commented 4 years ago

Does ArchLinux package can help?

https://aur.archlinux.org/cgit/aur.git/tree/PKGBUILD?h=ungoogled-chromium-ozone

'is_official_build=true' # implies is_cfi=true on x86_64

  # Fixes for building with libstdc++ instead of libc++
  patch -Np1 -i ../patches/chromium-84-AXObject-stl-iterator.patch
  patch -Np1 -i ../patches/chromium-84-ListContainerHelper-include-cstring.patch
  patch -Np1 -i ../patches/chromium-84-crashpad-include-cstring.patch
  patch -Np1 -i ../patches/chromium-84-gcc-10-include-stddef.patch
  patch -Np1 -i ../patches/chromium-84-gcc-10-webrtc-include-stddef.patch
  patch -Np1 -i ../patches/chromium-84-gcc-DCHECK_EQ-unique_ptr.patch
  patch -Np1 -i ../patches/chromium-84-std-vector-const.patch
0xC0ncord commented 4 years ago

@alexminder Interesting. I will try these out tonight, but I may need to do a pretty large rebuild as most of my system is already using libc++ where possible.

PF4Public commented 4 years ago

BTW, does cfi have any significance or use or benefit for the trouble it presents for compiling?

0xC0ncord commented 4 years ago

BTW, does cfi have any significance or use or benefit for the trouble it presents for compiling?

CFI is a proven effective security feature that has been in use on official builds of Google Chrome and Chromium for a while now.

From the Clang documention:

Clang includes an implementation of a number of control flow integrity (CFI) schemes, which are designed to abort the program upon detecting certain forms of undefined behavior that can potentially allow attackers to subvert the program’s control flow.

And from the Chromium documentation:

CFI for virtual calls is enabled for the official Chrome on Linux x86-64 (M54 and newer). CFI for indirect (C-style) calls is enabled for the official Chrome on Linux x86-64 (M68 and newer). Chrome is bad-cast clean, and we have a bot on chromium.memory that keeps it that way We're working on additional compiler improvements to allow deploying CFI on more platforms.

PF4Public commented 4 years ago

Recently I came to conclusion, that libcxx flag should be once again removed from ebuild.

Last time, when I've removed it, I thought, that this should be accomplished by a user locally through env configuration, but since mainline chromium ebuild embraced libcxx functionality, I've copied it verbatim into my ebuild. Turns out, their handling of libcxx is very flawed. So once again, I've decided to remove this flag with next release. This could probably ease the cfi troubles.

0xC0ncord commented 4 years ago

Recently I came to conclusion, that libcxx flag should be once again removed from ebuild.

Understood. I will be looking forward to test. I have another system not yet converted to libcxx so I will try building using stdlibc++ (default) as soon as the ebuild is available. The patches referenced by @alexminder should help if there are any issues.

PF4Public commented 4 years ago

Recently I came to conclusion, that libcxx flag should be once again removed from ebuild.

Just a note: That does not mean you cannot build ungoogled-chromium with libc++. You will need to manually edit portage env and apply CPPFLAGS and other settings as needed per package on your system.

PF4Public commented 4 years ago

125 is out for testing. I've removed libcxx flag. Hope nothing broke.

0xC0ncord commented 4 years ago

Hope nothing broke.

Happy to report that so far on my main system (where I'm using libcxx) nothing broke. I am currently prepping the toolchain on another system to see if I can build it with other USE flag combinations and get cfi working.

0xC0ncord commented 4 years ago

For the past 2 weeks I have been rebuilding my system with libc++ across the board where possible, as I have discovered that using multiple stdc++ libraries causes problems. But, I am still running into issues, though these are all unrelated to ungoogled-chromium and cfi. I have had zero issues building ungoogled-chromium with cfi as long as its dependencies are built with -stdlib=libc++ using clang. Perhaps we can amend the notice in the ebuild if cfi is enabled to point here, too. Your thoughts, @PF4Public ?

PF4Public commented 4 years ago

Perhaps we can amend the notice in the ebuild if cfi is enabled to point here, too. Your thoughts, @PF4Public ?

Absolutely! I'm just patiently waiting for your verdict on the issue.

Do I get it right, that if everything is compiled with -stdlib=libc++, cfi does not break the build?

0xC0ncord commented 4 years ago

Do I get it right, that if everything is compiled with -stdlib=libc++, cfi does not break the build?

Yes. My verdict is that if -stdlib=libc++ is not used for all dependencies, then the build will fail due to a CFI violation in the build process for v8/torque (still not sure why). Note that I haven't tried building dependencies this way with gcc/g++, so I'm not sure if the same problem would occur.

PF4Public commented 4 years ago

I've just reworded the warning message. Feel free to close this bug if you consider that it is better explained now.

0xC0ncord commented 4 years ago

I've just reworded the warning message. Feel free to close this bug if you consider that it is better explained now.

LGTM! Thank you for your patience.