[compiler-rt] r240617 - [CMake] Fix PR23539: Don't reference C++ ABI symbols prior to Mac OS 10.9.
Anna Zaks
zaks.anna at gmail.com
Thu Jun 25 13:15:13 PDT 2015
On Thu, Jun 25, 2015 at 11:51 AM, Alexey Samsonov <vonosmas at gmail.com>
wrote:
> We'd better ask users :)
>
> Anna, Hans,
>
> Currently when we build sanitizer runtimes on modern Mac OS X hosts, and
> no explicit -mmacosx-version-min is specified, we implicitly add
> -mmacosx-version-min=10.7,
> as it's the minimum supported version that we can target. Do you think it
> makes sense to use the host version of OS X instead? I personally don't
> think so, as we might
> easily use modern hosts for, say, Chromium buildbots, but use the
> freshly-built Clang to target older version of Mac OS.
>
The min deployment target should not depend on the system on which you
build clang.
- You might build clang + compiler-rt on a newer OS and deploy back to
older users/testers. I'd expect this to be very common.
- You want to get the same behavior from clang/UBSan regardless of the
machine on which it has been built.
My understanding is that the downside is that certain checks will be
disabled by default. The solution there is to set the default min
deployment target to 10.9 by default. People who still want to deploy back
to 10.7 could set SANITIZER_MIN_OSX_VERSION to 10.7 when they build clang.
This would work for me as there are very few users who are on 10.8 and
earlier (<10 -15% ?); and I'd rather not penalize everyone else.
>
> On Thu, Jun 25, 2015 at 11:47 AM, Frédéric Riss <friss at apple.com> wrote:
>
>>
>> On Jun 25, 2015, at 11:38 AM, Alexey Samsonov <vonosmas at gmail.com> wrote:
>>
>> Ah, right. But that's the expected change - if no version is specified,
>> we assume that we want to support 10.7, and therefore can't depend on
>> availability of libc++abi symbols
>> (otherwise we'll get problems at link time later).
>>
>>
>> True. Wouldn’t it make more sense to default to the current machine’s
>> version when nothing is specified?
>>
>> I'm testing a quick fix now.
>>
>> On Thu, Jun 25, 2015 at 11:29 AM, Frédéric Riss <friss at apple.com> wrote:
>>
>>>
>>> On Jun 25, 2015, at 11:24 AM, Alexey Samsonov <vonosmas at gmail.com>
>>> wrote:
>>>
>>> Hi Frederic,
>>>
>>> Ah, just noticed it. Sorry for the breakage :-/.
>>> I don't think your analysis is correct, though - we used to
>>> unconditionally set MIN_OSX_VERSION to 10.7 before, so the default behavior
>>> was not changed.
>>>
>>>
>>> what I mean is that you added
>>>
>>> if VERSION < 10.9
>>>
>>> below
>>>
>>> if no version specified
>>> VERSION = 10.7
>>>
>>> thus your test will always trigger when no version is specified (what I
>>> called the default behavior). If I revert this patch, the CFI tests pass
>>> just fine on my machine.
>>>
>>> Fred
>>>
>>> The problem happens with newly added CFI diagnostic mode, that uses
>>> UBSan runtime, and depends on the presence of c++abi-specific code there,
>>> while we disabled
>>> linking in this code on older versions of Darwin.
>>>
>>> I will disable cfi tests on Darwin to fix the bot, and then discuss with
>>> Peter the way to resolve this.
>>>
>>>
>>> On Thu, Jun 25, 2015 at 11:20 AM, Frédéric Riss <friss at apple.com> wrote:
>>>
>>>>
>>>> > On Jun 25, 2015, at 9:22 AM, Frédéric Riss <friss at apple.com> wrote:
>>>> >
>>>> > Hi Alexey
>>>> >
>>>> > This commit broke the Darwin bot at:
>>>> > http://lab.llvm.org:8080/green/job/clang-stage1-cmake-RA/
>>>> >
>>>> > The failures are link errors like:
>>>> > undef: ___ubsan_handle_cfi_bad_type
>>>> > Undefined symbols for architecture x86_64:
>>>> > "___ubsan_handle_cfi_bad_type", referenced from:
>>>> > _main in vdtor-0e96d6.o
>>>> > ld: symbol(s) not found for architecture x86_64
>>>> > clang-3.7: error: linker command failed with exit code 1 (use -v to
>>>> see invocation)
>>>> >
>>>> > I guess that somehow the check you added triggers on the bot,
>>>> although it shouldn’t as the machines there are 10.10
>>>> >
>>>> > Did you notice this?
>>>>
>>>> What happens is that you added a check against
>>>> SANITIZER_MIN_OSX_VERSION, but when none is specified (which is the
>>>> default) the CMakelists decides that the default value is 10.7. So you
>>>> actually changed the default behavior here. As this breaks a public bot,
>>>> I’ll revert it if I don’t here from you soon.
>>>>
>>>> (There is a more fundamental issue here that the link error is really
>>>> the wrong diagnostic. The fronted should prevent you from getting into that
>>>> situation by telling you that the configuration your asking for won’t work.)
>>>>
>>>> Fred
>>>>
>>>> > Fred
>>>> >
>>>> >> On Jun 24, 2015, at 5:57 PM, Alexey Samsonov <vonosmas at gmail.com>
>>>> wrote:
>>>> >>
>>>> >> Author: samsonov
>>>> >> Date: Wed Jun 24 19:57:42 2015
>>>> >> New Revision: 240617
>>>> >>
>>>> >> URL: http://llvm.org/viewvc/llvm-project?rev=240617&view=rev
>>>> >> Log:
>>>> >> [CMake] Fix PR23539: Don't reference C++ ABI symbols prior to Mac OS
>>>> 10.9.
>>>> >>
>>>> >> Summary:
>>>> >> This patch implements step 1 from
>>>> >> https://llvm.org/bugs/show_bug.cgi?id=23539#c10
>>>> >>
>>>> >> I'd appreciate if you could test it on Mac OS and verify that parts
>>>> of UBSan
>>>> >> runtime that reference C++ ABI symbols are properly excluded, and
>>>> fix ASan/UBSan
>>>> >> builds.
>>>> >>
>>>> >> Test Plan: regression test suite
>>>> >>
>>>> >> Reviewers: thakis, hans
>>>> >>
>>>> >> Subscribers: llvm-commits, zaks.anna, kubabrecka
>>>> >>
>>>> >> Differential Revision: http://reviews.llvm.org/D10621
>>>> >>
>>>> >> Modified:
>>>> >> compiler-rt/trunk/CMakeLists.txt
>>>> >> compiler-rt/trunk/lib/ubsan/CMakeLists.txt
>>>> >> compiler-rt/trunk/test/lit.common.cfg
>>>> >> compiler-rt/trunk/test/lit.common.configured.in
>>>> >>
>>>> compiler-rt/trunk/test/ubsan/TestCases/TypeCheck/vptr-virtual-base.cpp
>>>> >> compiler-rt/trunk/test/ubsan/TestCases/TypeCheck/vptr.cpp
>>>> >>
>>>> >> Modified: compiler-rt/trunk/CMakeLists.txt
>>>> >> URL:
>>>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/CMakeLists.txt?rev=240617&r1=240616&r2=240617&view=diff
>>>> >>
>>>> ==============================================================================
>>>> >> --- compiler-rt/trunk/CMakeLists.txt (original)
>>>> >> +++ compiler-rt/trunk/CMakeLists.txt Wed Jun 24 19:57:42 2015
>>>> >> @@ -327,6 +327,15 @@ if(APPLE)
>>>> >> endif()
>>>> >> endif()
>>>> >>
>>>> >> +if(APPLE AND SANITIZER_MIN_OSX_VERSION VERSION_LESS "10.9")
>>>> >> + # Mac OS X prior to 10.9 had problems with exporting symbols from
>>>> >> + # libc++/libc++abi.
>>>> >> + set(SANITIZER_CAN_USE_CXXABI FALSE)
>>>> >> +else()
>>>> >> + set(SANITIZER_CAN_USE_CXXABI TRUE)
>>>> >> +endif()
>>>> >> +pythonize_bool(SANITIZER_CAN_USE_CXXABI)
>>>> >> +
>>>> >> add_subdirectory(include)
>>>> >>
>>>> >> set(COMPILER_RT_LIBCXX_PATH ${LLVM_MAIN_SRC_DIR}/projects/libcxx)
>>>> >>
>>>> >> Modified: compiler-rt/trunk/lib/ubsan/CMakeLists.txt
>>>> >> URL:
>>>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/ubsan/CMakeLists.txt?rev=240617&r1=240616&r2=240617&view=diff
>>>> >>
>>>> ==============================================================================
>>>> >> --- compiler-rt/trunk/lib/ubsan/CMakeLists.txt (original)
>>>> >> +++ compiler-rt/trunk/lib/ubsan/CMakeLists.txt Wed Jun 24 19:57:42
>>>> 2015
>>>> >> @@ -28,11 +28,16 @@ set(UBSAN_CXXFLAGS ${SANITIZER_COMMON_CF
>>>> >> add_custom_target(ubsan)
>>>> >>
>>>> >> if(APPLE)
>>>> >> + set(UBSAN_COMMON_SOURCES ${UBSAN_SOURCES})
>>>> >> + if(SANITIZER_CAN_USE_CXXABI)
>>>> >> + list(APPEND UBSAN_COMMON_SOURCES ${UBSAN_CXX_SOURCES})
>>>> >> + endif()
>>>> >> +
>>>> >> # Common parts of UBSan runtime.
>>>> >> add_compiler_rt_object_libraries(RTUbsan
>>>> >> OS ${SANITIZER_COMMON_SUPPORTED_OS}
>>>> >> ARCHS ${UBSAN_COMMON_SUPPORTED_ARCH}
>>>> >> - SOURCES ${UBSAN_SOURCES} ${UBSAN_CXX_SOURCES}
>>>> >> + SOURCES ${UBSAN_COMMON_SOURCES}
>>>> >> CFLAGS ${UBSAN_CXXFLAGS})
>>>> >>
>>>> >> if(COMPILER_RT_HAS_UBSAN)
>>>> >>
>>>> >> Modified: compiler-rt/trunk/test/lit.common.cfg
>>>> >> URL:
>>>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/lit.common.cfg?rev=240617&r1=240616&r2=240617&view=diff
>>>> >>
>>>> ==============================================================================
>>>> >> --- compiler-rt/trunk/test/lit.common.cfg (original)
>>>> >> +++ compiler-rt/trunk/test/lit.common.cfg Wed Jun 24 19:57:42 2015
>>>> >> @@ -90,6 +90,10 @@ compiler_rt_debug = getattr(config, 'com
>>>> >> if not compiler_rt_debug:
>>>> >> config.available_features.add('compiler-rt-optimized')
>>>> >>
>>>> >> +sanitizer_can_use_cxxabi = getattr(config,
>>>> 'sanitizer_can_use_cxxabi', True)
>>>> >> +if sanitizer_can_use_cxxabi:
>>>> >> + config.available_features.add('cxxabi')
>>>> >> +
>>>> >> lit.util.usePlatformSdkOnDarwin(config, lit_config)
>>>> >>
>>>> >> def is_darwin_lto_supported():
>>>> >>
>>>> >> Modified: compiler-rt/trunk/test/lit.common.configured.in
>>>> >> URL:
>>>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/lit.common.configured.in?rev=240617&r1=240616&r2=240617&view=diff
>>>> >>
>>>> ==============================================================================
>>>> >> --- compiler-rt/trunk/test/lit.common.configured.in (original)
>>>> >> +++ compiler-rt/trunk/test/lit.common.configured.in Wed Jun 24
>>>> 19:57:42 2015
>>>> >> @@ -27,6 +27,7 @@ set_default("python_executable", "@PYTHO
>>>> >> set_default("compiler_rt_debug", @COMPILER_RT_DEBUG_PYBOOL@)
>>>> >> set_default("compiler_rt_libdir", "@COMPILER_RT_LIBRARY_OUTPUT_DIR@
>>>> ")
>>>> >> set_default("emulator", "@COMPILER_RT_EMULATOR@")
>>>> >> +set_default("sanitizer_can_use_cxxabi",
>>>> @SANITIZER_CAN_USE_CXXABI_PYBOOL@)
>>>> >>
>>>> >> # LLVM tools dir can be passed in lit parameters, so try to
>>>> >> # apply substitution.
>>>> >>
>>>> >> Modified:
>>>> compiler-rt/trunk/test/ubsan/TestCases/TypeCheck/vptr-virtual-base.cpp
>>>> >> URL:
>>>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/ubsan/TestCases/TypeCheck/vptr-virtual-base.cpp?rev=240617&r1=240616&r2=240617&view=diff
>>>> >>
>>>> ==============================================================================
>>>> >> ---
>>>> compiler-rt/trunk/test/ubsan/TestCases/TypeCheck/vptr-virtual-base.cpp
>>>> (original)
>>>> >> +++
>>>> compiler-rt/trunk/test/ubsan/TestCases/TypeCheck/vptr-virtual-base.cpp Wed
>>>> Jun 24 19:57:42 2015
>>>> >> @@ -1,6 +1,8 @@
>>>> >> // RUN: %clangxx -frtti -fsanitize=vptr -fno-sanitize-recover=vptr
>>>> -g %s -O3 -o %t
>>>> >> // RUN: not %run %t 2>&1 | FileCheck %s
>>>> >>
>>>> >> +// REQUIRES: cxxabi
>>>> >> +
>>>> >> struct S { virtual int f() { return 0; } };
>>>> >> struct T : virtual S {};
>>>> >>
>>>> >>
>>>> >> Modified: compiler-rt/trunk/test/ubsan/TestCases/TypeCheck/vptr.cpp
>>>> >> URL:
>>>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/ubsan/TestCases/TypeCheck/vptr.cpp?rev=240617&r1=240616&r2=240617&view=diff
>>>> >>
>>>> ==============================================================================
>>>> >> --- compiler-rt/trunk/test/ubsan/TestCases/TypeCheck/vptr.cpp
>>>> (original)
>>>> >> +++ compiler-rt/trunk/test/ubsan/TestCases/TypeCheck/vptr.cpp Wed
>>>> Jun 24 19:57:42 2015
>>>> >> @@ -24,7 +24,7 @@
>>>> >> // RUN: echo "vptr_check:S" > %t.loc-supp
>>>> >> // RUN: UBSAN_OPTIONS="suppressions='%t.loc-supp'" not %run %t x-
>>>> 2>&1 | FileCheck %s --check-prefix=CHECK-LOC-SUPPRESS
>>>> >>
>>>> >> -// REQUIRES: stable-runtime
>>>> >> +// REQUIRES: stable-runtime, cxxabi
>>>> >> #include <new>
>>>> >> #include <assert.h>
>>>> >> #include <stdio.h>
>>>> >>
>>>> >>
>>>> >> _______________________________________________
>>>> >> llvm-commits mailing list
>>>> >> llvm-commits at cs.uiuc.edu
>>>> >> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>> >
>>>>
>>>>
>>>
>>>
>>> --
>>> Alexey Samsonov
>>> vonosmas at gmail.com
>>>
>>>
>>>
>>
>>
>> --
>> Alexey Samsonov
>> vonosmas at gmail.com
>>
>>
>>
>
>
> --
> Alexey Samsonov
> vonosmas at gmail.com
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150625/3d07cd9b/attachment.html>
More information about the llvm-commits
mailing list