Generate ubsan shared libraries.

[aoli]

Summary

Those libraries are required by aosp (https://android.googlesource.com/platform/prebuilts/clang/host/linux-x86/+/master/Android.mk). Currenly the shared libraries are generated by aosp Makefile system. We are looking forward to using cmake to generate them.

[srhines]

lib/ubsan/CMakeLists.txt

167

Shouldn't this be UBSAN_CXXFLAGS?

[aoli]

Use UBSAN_CXXFLAGS for ubsan shared libraries.

[vsk]

lib/ubsan/CMakeLists.txt

170

I'm not familiar with the differences between RTUbsan_cxx and its C variant, but for consistency with what's already here, should we have 'SHARED' versions of clang_rt.ubsan_standalone and clang_rt.ubsan_standalone_cxx?

Also: do you know whether the ubsan standalone runtimes are tested in a check-ubsan test run?

[aoli]

Generate clang_rt.ubsan_standalone and clang_rt.ubsan_standalone_cxx runtimes.

[aoli]

lib/ubsan/CMakeLists.txt

170

I've updated the CMakeList.txt which generates both clang_rt.ubsan_standalone and clang_rt.ubsan_standalone_cxx now. It seems that the ubsan standalone runtimes are tested in the test run. https://github.com/llvm-mirror/compiler-rt/blob/master/test/ubsan/CMakeLists.txt#L26

[vsk]

lib/ubsan/CMakeLists.txt

170

Hm, yes, but I've just now noticed that this doesn't appear to do the right thing, at least on my system.

For example, I can run a standalone ubsan test like this:
./bin/llvm-lit projects/compiler-rt/test/ubsan/Standalone-x86_64/TestCases/Misc/bool.cpp -a

Inspecting the link command, I see:
"/Volumes/Builds/llvm.org-ubsan-arith-DA/lib/clang/5.0.0/lib/darwin/libclang_rt.ubsan_osx_dynamic.dylib"

So it's not using the standalone lib at all? I filed a bug: https://bugs.llvm.org/show_bug.cgi?id=33059. I don't think this should hold up your patch, but missing test coverage is always spooky.

174

Here, it seems like you'd just want 'RTInterception', and not all of '${UBSAN_COMMON_RUNTIME_OBJECT_LIBS}'.

[aoli]

lib/ubsan/CMakeLists.txt

174

Oh, RTInterception was added by accident. I think I'm not using it either. Shall I keep share libraries' OBJECT_LIBS just the same as those for static libraries?

[vsk]

lib/ubsan/CMakeLists.txt

174

I think it's a good idea to keep the OBJECT_LIBS as close to possible as those for static libraries. Could you double-check that RTInterception isn't needed in just this specific case? I do see some Windows-specific code in lib/ubsan that sets up interceptors.

[aoli]

lib/ubsan/CMakeLists.txt

174

I just checked again. To compile clang_rt.ubsan_standalone_cxx it also requires RTUbsan, RTSanitizerCommon and RTSanitizerCommonLibc but not RTInterception. I'd prefer to have only one clang_rt.ubsan_standalone which includes RTUbsan, RTUbsan_cxx ,RTSanitizerCommon` and RTSanitizerCommonLibc now(which is the same as my first change). Because clang_rt.ubsan_standalone_cxx will include all libraries in clang_rt.ubsan_standalone.

To find the reference to RTInterception, I first compiled the shared library without RTInterception on my machine and it doesn't generate any error. Then I grepped several APIs shown in interception_win.h and interception_mac.h and it didn't return any result so I suppose the RTInterception is not being used in ubsan. The only thing I found was sanitizer_win_weak_interception.h which was actually included in RTSanitizerCommon. I'm new to LLVM and not quiet familiar with the source code and I'm not sure if I'm doing right.

[vsk]

lib/ubsan/CMakeLists.txt

174

I just checked again. To compile clang_rt.ubsan_standalone_cxx it also requires RTUbsan, RTSanitizerCommon and RTSanitizerCommonLibc but not RTInterception.

This makes sense, looking at RTUbsan_cxx, it's just RTUbsan plus some source files which implement C++ specific checks.

I'd prefer to have only one clang_rt.ubsan_standalone which includes RTUbsan, RTUbsan_cxx ,RTSanitizerCommon` and RTSanitizerCommonLibc now (which is the same as my first change).

I don't think clang_rt.ubsan_standalone is meant to include any C++ specific checks. I think this could matter in practice because -fsanitize=vptr needs to find some typeinfo objects from libcxxabi, or at least something that looks like it.

To find the reference to RTInterception, I first compiled the shared library without RTInterception on my machine and it doesn't generate any error. Then I grepped several APIs shown in interception_win.h and interception_mac.h and it didn't return any result so I suppose the RTInterception is not being used in ubsan. The only thing I found was sanitizer_win_weak_interception.h which was actually included in RTSanitizerCommon.

Perfect, thanks for checking. The version of your patch we've got right now LGTM, after we remove the RTInterception dep.

I'm new to LLVM and not quiet familiar with the source code and I'm not sure if I'm doing right.

Welcome! You're doing it right.

[aoli]

Remove unused object libraries.

[aoli]

Have ubsan_standalone and ubsan_standalone_cxx separately.

[aoli]

Fix indent issue.

[aoli]

Fix indent issue.

[vsk]

LGTM. If you haven't already got commit access, let me know, and I'd be happy to commit this for you.

[filcab]

lib/ubsan/CMakeLists.txt

42

Are all of these needed?

43

I wouldn't expect the non-c++ version of the lib to bring libstdc++ with it.

170

That looks like the standalone lib (versus the ASan/TSan/etc lib which include ubsan).
You're testing on macOS, though, which already has its own build of a standalone ubsan library, separate from this part of the makefile, which is for !APPLE (check lines 47 and 83).

[aoli]

lib/ubsan/CMakeLists.txt

43

Sorry I did not see your comments. I'll bring a new commit to fix this.

lib/ubsan/CMakeLists.txt

 set(UBSAN_STANDALONE_CFLAGS ${SANITIZER_COMMON_CFLAGS})
 append_rtti_flag(OFF UBSAN_STANDALONE_CFLAGS)
 append_list_if(SANITIZER_CAN_USE_CXXABI -DUBSAN_CAN_USE_CXXABI UBSAN_STANDALONE_CFLAGS)
 
 set(UBSAN_CXXFLAGS ${SANITIZER_COMMON_CFLAGS})
 append_rtti_flag(ON UBSAN_CXXFLAGS)
 append_list_if(SANITIZER_CAN_USE_CXXABI -DUBSAN_CAN_USE_CXXABI UBSAN_CXXFLAGS)
 
+append_list_if(COMPILER_RT_HAS_LIBDL dl UBSAN_DYNAMIC_LIBS)
+append_list_if(COMPILER_RT_HAS_LIBLOG log UBSAN_DYNAMIC_LIBS)
+append_list_if(COMPILER_RT_HAS_LIBC c UBSAN_DYNAMIC_LIBS)
+append_list_if(COMPILER_RT_HAS_LIBRT rt UBSAN_DYNAMIC_LIBS)
+append_list_if(COMPILER_RT_HAS_LIBM m UBSAN_DYNAMIC_LIBS)
+append_list_if(COMPILER_RT_HAS_LIBPTHREAD pthread UBSAN_DYNAMIC_LIBS)
+append_list_if(COMPILER_RT_HAS_LIBSTDCXX stdc++ UBSAN_DYNAMIC_LIBS)
+ 
 add_compiler_rt_component(ubsan)
 
 if(APPLE)
   set(UBSAN_COMMON_SOURCES ${UBSAN_SOURCES})
   if(SANITIZER_CAN_USE_CXXABI)
     list(APPEND UBSAN_COMMON_SOURCES ${UBSAN_CXXABI_SOURCES})
   endif()
 
add_compiler_rt_runtime(clang_rt.ubsan
                   RTUbsan_standalone
                   RTSanitizerCommon
                   RTSanitizerCommonLibc
       LINK_FLAGS ${WEAK_SYMBOL_LINK_FLAGS}
       PARENT_TARGET ubsan)
   endif()
 
 else()
+  set(UBSAN_COMMON_RUNTIME_OBJECT_LIBS
+    RTInterception
+    RTSanitizerCommon
+    RTSanitizerCommonLibc
+    )
+
   # Common parts of UBSan runtime.
   add_compiler_rt_object_libraries(RTUbsan
     ARCHS ${UBSAN_COMMON_SUPPORTED_ARCH}
     SOURCES ${UBSAN_SOURCES} CFLAGS ${UBSAN_CFLAGS})
 
   if(SANITIZER_CAN_USE_CXXABI)
     # C++-specific parts of UBSan runtime. Requires a C++ ABI library.
     set(UBSAN_CXX_SOURCES ${UBSAN_CXXABI_SOURCES})
if(COMPILER_RT_HAS_UBSAN)
     
     add_compiler_rt_runtime(clang_rt.ubsan_standalone_cxx
       STATIC
       ARCHS ${UBSAN_SUPPORTED_ARCH}
       OBJECT_LIBS RTUbsan_cxx
       CFLAGS ${UBSAN_CXXFLAGS}
       PARENT_TARGET ubsan)
 
+    add_compiler_rt_runtime(clang_rt.ubsan_standalone
+      SHARED
+      ARCHS ${UBSAN_SUPPORTED_ARCH}
+      OBJECT_LIBS ${UBSAN_COMMON_RUNTIME_OBJECT_LIBS}
+      RTUbsan
+      RTUbsan_cxx
+      CFLAGS ${UBSAN_CFLAGS}
+      LINK_LIBS ${UBSAN_DYNAMIC_LIBS}
+      PARENT_TARGET ubsan)
+
     if (UNIX)
       set(ARCHS_FOR_SYMBOLS ${UBSAN_SUPPORTED_ARCH})
       list(REMOVE_ITEM ARCHS_FOR_SYMBOLS i386 i686)
       add_sanitizer_rt_symbols(clang_rt.ubsan_standalone
         ARCHS ${ARCHS_FOR_SYMBOLS}
         PARENT_TARGET ubsan
         EXTRA ubsan.syms.extra)
       add_sanitizer_rt_symbols(clang_rt.ubsan_standalone_cxx
         ARCHS ${ARCHS_FOR_SYMBOLS}
         PARENT_TARGET ubsan
         EXTRA ubsan.syms.extra)
     endif()
   endif()
 endif()