CFI: public libc++ symbol visibility leads to unrelated casts faults |
||
Issue descriptionChrome Version: tip OS: linux-x86_64 What steps will reproduce the problem? (1) Remove libc++ symbols from the CFI blacklist, e.g. apply https://reviews.llvm.org/D35855 to third_party/llvm-build/Release+Asserts/lib/clang/6.0.0/cfi_blacklist.txt (2) Build gl_unittests or browser_tests with is_cfi=true use_cfi_diag=true and run them* *To enable use_cfi_diag=true to get pretty printed error messages the builds needs to be performed with a version of clang that includes https://reviews.llvm.org/D36013 which hasn't been rolled in yet. You will get a failure like the following: [ RUN ] GLSurfaceEGLTest.SurfaceFormatTest ../../buildtools/third_party/libc++/trunk/include/ios:718:12: runtime error: control flow integrity check for type 'std::__1::basic_streambuf<char>' failed during cast to unrelated type (vtable address 0x000000254cb0) 0x000000254cb0: note: invalid vtable in module /usr/local/google/home/vtsyrklevich/Development/chromium/src/out/fixed-no-trap-libc++/gl_unittests 00 00 00 00 40 e3 4c 00 00 00 00 00 70 e3 4c 00 00 00 00 00 e0 3f 4e 00 00 00 00 00 f0 3f 4e 00 This occurs because libc++ static libraries are built with externally visible symbols. gl_unittests loads libGLESv2.so which also statically links libc++, but because of the visibility for libc++ symbols is set to default, the dynamic linker resolves libGLESv2.so's symbol for the vtable of 'std::__1::basic_streambuf<char>' with the one from gl_unittests body later causing the CFI unrelated cast exception. Changing the build options to limit libc++ symbol visibility when statically linking resolves this issue.
,
Aug 3 2017
This isn't a blocker for the clang roll, is it? (Also, welcome!)
,
Aug 3 2017
It is because this is uncovered by the compiler-rt change (D35855) linked above. I have changes queued up to re-enable those options in chrome's CFI blacklist as well as a CL that fixes the build options for this bug but I've been blocked on an account reset from accounts@chromium to upload either of those CLs.
,
Aug 4 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/buildtools/+/abaf2ba54948698469e0d6aa71142b641a4f8a33 commit abaf2ba54948698469e0d6aa71142b641a4f8a33 Author: Vlad Tsyrklevich <vtsyrklevich@google.com> Date: Fri Aug 04 18:37:58 2017 Don't leak libc++ symbols across modules Currently public libc++ symbol visibility is only disabled for the static build of libc++ itself; however, dynamic libraries that link in libc++ and pull in its header files also need to avoid leaking symbols or else dynamic libraries will resolve internal-only libc++ symbols from the executable. Move these definitions from //buildtools//third_party/libc++/BUILD.gn to //build/config/posix/BUILD.gn This change needs to land alongside or after https://chromium-review.googlesource.com/c/601529 R=glider@chromium.org, pcc@chromium.org, thomasanderson@chromium.org Bug: 751360 Change-Id: Ief91c2a344879b63e05e9be9bc7675321d80e2b4 [modify] https://crrev.com/abaf2ba54948698469e0d6aa71142b641a4f8a33/third_party/libc++/BUILD.gn
,
Aug 5 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2440912f23c01e661b47341bb322f50f67cb153b commit 2440912f23c01e661b47341bb322f50f67cb153b Author: Vlad Tsyrklevich <vtsyrklevich@google.com> Date: Sat Aug 05 02:12:13 2017 Don't leak libc++ symbols across modules Currently public libc++ symbol visibility is only disabled for the static build of libc++ itself; however, dynamic libraries that link in libc++ and pull in it's header files also need to avoid leaking symbols or else dynamic libraries will resolve internal-only libc++ symbols from the executable. Move these definitions from //buildtools//third_party/libc++/BUILD.gn to //build/config/posix/BUILD.gn This change needs to land alongside or before https://chromium-review.googlesource.com/c/601422 R=pcc@chromium.org, thakis@chromium.org, thomasanderson@chromium.org Bug: 751360 Change-Id: I78ff6902e014150a56da9d908869199e1bce226a Reviewed-on: https://chromium-review.googlesource.com/601529 Commit-Queue: Peter Collingbourne <pcc@chromium.org> Reviewed-by: Nico Weber <thakis@chromium.org> Reviewed-by: Thomas Anderson <thomasanderson@chromium.org> Cr-Commit-Position: refs/heads/master@{#492215} [modify] https://crrev.com/2440912f23c01e661b47341bb322f50f67cb153b/DEPS [modify] https://crrev.com/2440912f23c01e661b47341bb322f50f67cb153b/build/config/posix/BUILD.gn
,
Aug 5 2017
|
||
►
Sign in to add a comment |
||
Comment 1 by vtsyrklevich@chromium.org
, Aug 2 2017