[PATCH] D33216: Generate ubsan shared libraries.

[Vedant Kumar via Phabricator via llvm-commits]

vsk added inline comments.


================
Comment at: lib/ubsan/CMakeLists.txt:170
+      PARENT_TARGET ubsan)
+
     if (UNIX)
----------------
aoli wrote:
> vsk wrote:
> > 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?
> 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 
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.


================
Comment at: lib/ubsan/CMakeLists.txt:174
+      ARCHS ${UBSAN_SUPPORTED_ARCH}
+      OBJECT_LIBS ${UBSAN_COMMON_RUNTIME_OBJECT_LIBS}
+      RTUbsan_cxx
----------------
Here, it seems like you'd just want 'RTInterception', and not all of '${UBSAN_COMMON_RUNTIME_OBJECT_LIBS}'.


https://reviews.llvm.org/D33216