It would be useful to include some folks who work on the sanitizers. MSan in particular is only really useful when linking against a sanitized standard library.
On Sun, Aug 17, 2014 at 12:57 AM, Justin Bogner <[email protected]> wrote: > I'm pretty sure the extra false positives you're seeing here are the > result of using an instrumented program with an uninstrumented shared > library. > > I wasn't able to get any interesting results from your example, but I > followed your instructions to run the libc++ tests with msan. That is, I > built llvm and clang with libcxx and libcxxabi checked out, and added > compile_flags=['-fsanitize=memory'] in libc++'s lit.cfg. This reproduces > the false positives in string that you mentioned. > > On the other hand, if I build libc++.so with -fsanitize=memory (via > -DLLVM_USE_SANITIZER=Memory) and then run the tests against that with > the memory sanitizer, the new errors in string go away. There are quite > a few tests that seem to have other problems this way, but that's true > with or without the template instantiations. > > So it looks to me like the "there is no good way to run libc++ tests > with sanitizers" claim is pretty accurate ;) > > Eric Fiselier <[email protected]> writes: > > Oh jeez, I forgot a very important aspect to the reproducer. Last time I > > looked into this it seemed that __attribute__ ((__always_inline__)) also > > played a part in this. In the last example "get_x()" should have this > > attribute. > > > > On Sat, Aug 16, 2014 at 3:04 AM, Eric Fiselier <[email protected]> wrote: > > > > I could have been a lot clearer. It seems to be a bug in MSAN. I've > run > > into it before. > > The code that causes it is something like: > > > > // TestClass.hpp > > template <class T> > > class TestClass > > { > > int x{0}; > > int get_x() { return x; } > > }; > > > > extern template class TestClass<int>; > > > > // TestClass.cpp > > template class TestClass<int>; > > > > // main.cpp > > #include "TestClass.hpp" > > int main() > > { > > TestClass<int> t; > > // MSAN fires on use of uninitialized value here. > > int x = t.get_x();te > > } > > > > That's by no means a reproducer, but it represents similar code to > what > > causes the test failures in "/test/string". MSAN seems to fire *any* > time > > a value is returned from a method of a class that has been > instantiated > > externally. I think the shared object boundary might be important. I > > haven't had time to investigate. > > > > Unfortunately there is currently no good way to run the libc++ tests > with > > sanitizers. The easiest way is to just insert "-fsanitize=memory" > into the > > `compile_flags` list in `test/lit.cfg'. Then just "cd test/string ; > lit > > -sv . " > > > > I hope that made it a little clearer. > > > > /Eric > > > > P.S. These failures arn't localized to "test/string". They are > anywhere > > templates are instantiated externally. String is just > representative. > > > > On Sat, Aug 16, 2014 at 2:42 AM, Justin Bogner < > [email protected]> > > wrote: > > > > Eric Fiselier <[email protected]> writes: > > > Hi Justin, > > > > > > I'm not quite sure we are should do that right now. > > > If your compiling with MSAN and you call a function in an > extern > > > template MSAN will fire. > > > > I don't think I understand what you mean here. Which function? > Why > > does > > msan fire? > > > > > For example testing test/string with MSAN enabled causes 124 > > failures. > > > Normally there are just 2. > > > > If there are invalid memory accesses we should fix them. I don't > see > > how > > explicit template instantiations would cause memory errors > though. > > > > Are there bots that show this issue? How can I reproduce it? > > > > > We could probably guard the _LIBCPP_EXTERN_TEMPLATE definition > to > > > check for MSAN but for now I don't think we are ready to > re-enable > > > external template definitions. > > > > > > /Eric > > > > > > On Fri, Aug 15, 2014 at 11:58 AM, Justin Bogner < > > [email protected]> wrote: > > > > > > Author: bogner > > > Date: Fri Aug 15 12:58:56 2014 > > > New Revision: 215740 > > > > > > URL: > http://llvm.org/viewvc/llvm-project?rev=215740&view=rev > > > Log: > > > Revert "Turn off extern templates for most uses." > > > > > > Turning off explicit template instantiation leads to a > pretty > > > significant build time and code size cost. We're better off > > dealing > > > with ABI incompatibility issues that come up in a less > heavy > > handed > > > way. > > > > > > This reverts commit r189610. > > > > > > Modified: > > > libcxx/trunk/include/__config > > > libcxx/trunk/src/algorithm.cpp > > > libcxx/trunk/src/ios.cpp > > > libcxx/trunk/src/locale.cpp > > > libcxx/trunk/src/string.cpp > > > libcxx/trunk/src/valarray.cpp > > > > > > Modified: libcxx/trunk/include/__config > > > URL: > http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/ > > __config?rev > > > =215740&r1=215739&r2=215740&view=diff > > > > ================================================================ > > ========== > > > ==== > > > --- libcxx/trunk/include/__config (original) > > > +++ libcxx/trunk/include/__config Fri Aug 15 12:58:56 2014 > > > @@ -608,7 +608,7 @@ template <unsigned> struct > __static_asse > > > #endif > > > > > > #ifndef _LIBCPP_EXTERN_TEMPLATE > > > -#define _LIBCPP_EXTERN_TEMPLATE(...) > > > +#define _LIBCPP_EXTERN_TEMPLATE(...) extern template > > __VA_ARGS__; > > > #endif > > > > > > #ifndef _LIBCPP_EXTERN_TEMPLATE2 > > > > > > Modified: libcxx/trunk/src/algorithm.cpp > > > URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/ > > algorithm.cpp? > > > rev=215740&r1=215739&r2=215740&view=diff > > > > ================================================================ > > ========== > > > ==== > > > --- libcxx/trunk/src/algorithm.cpp (original) > > > +++ libcxx/trunk/src/algorithm.cpp Fri Aug 15 12:58:56 2014 > > > @@ -7,7 +7,6 @@ > > > // > > > //=== > > > > > > ---------------------------------------------------------------------- > > ===/ > > > / > > > > > > -#define _LIBCPP_EXTERN_TEMPLATE(...) extern template > > __VA_ARGS__; > > > #include "algorithm" > > > #include "random" > > > #include "mutex" > > > > > > Modified: libcxx/trunk/src/ios.cpp > > > URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/ > > ios.cpp?rev= > > > 215740&r1=215739&r2=215740&view=diff > > > > ================================================================ > > ========== > > > ==== > > > --- libcxx/trunk/src/ios.cpp (original) > > > +++ libcxx/trunk/src/ios.cpp Fri Aug 15 12:58:56 2014 > > > @@ -7,8 +7,6 @@ > > > // > > > //=== > > > > > > ---------------------------------------------------------------------- > > ===/ > > > / > > > > > > -#define _LIBCPP_EXTERN_TEMPLATE(...) extern template > > __VA_ARGS__; > > > - > > > #include "ios" > > > #include "streambuf" > > > #include "istream" > > > > > > Modified: libcxx/trunk/src/locale.cpp > > > URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/ > > locale.cpp?rev= > > > 215740&r1=215739&r2=215740&view=diff > > > > ================================================================ > > ========== > > > ==== > > > --- libcxx/trunk/src/locale.cpp (original) > > > +++ libcxx/trunk/src/locale.cpp Fri Aug 15 12:58:56 2014 > > > @@ -7,8 +7,6 @@ > > > // > > > //=== > > > > > > ---------------------------------------------------------------------- > > ===/ > > > / > > > > > > -#define _LIBCPP_EXTERN_TEMPLATE(...) extern template > > __VA_ARGS__; > > > - > > > // On Solaris, we need to define something to make the C99 > > parts of > > > localeconv > > > // visible. > > > #ifdef __sun__ > > > > > > Modified: libcxx/trunk/src/string.cpp > > > URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/ > > string.cpp?rev= > > > 215740&r1=215739&r2=215740&view=diff > > > > ================================================================ > > ========== > > > ==== > > > --- libcxx/trunk/src/string.cpp (original) > > > +++ libcxx/trunk/src/string.cpp Fri Aug 15 12:58:56 2014 > > > @@ -7,8 +7,6 @@ > > > // > > > //=== > > > > > > ---------------------------------------------------------------------- > > ===/ > > > / > > > > > > -#define _LIBCPP_EXTERN_TEMPLATE(...) extern template > > __VA_ARGS__; > > > - > > > #include "string" > > > #include "cstdlib" > > > #include "cwchar" > > > > > > Modified: libcxx/trunk/src/valarray.cpp > > > URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/ > > valarray.cpp?rev > > > =215740&r1=215739&r2=215740&view=diff > > > > ================================================================ > > ========== > > > ==== > > > --- libcxx/trunk/src/valarray.cpp (original) > > > +++ libcxx/trunk/src/valarray.cpp Fri Aug 15 12:58:56 2014 > > > @@ -7,8 +7,6 @@ > > > // > > > //=== > > > > > > ---------------------------------------------------------------------- > > ===/ > > > / > > > > > > -#define _LIBCPP_EXTERN_TEMPLATE(...) extern template > > __VA_ARGS__; > > > - > > > #include "valarray" > > > > > > _LIBCPP_BEGIN_NAMESPACE_STD > > > > > > _______________________________________________ > > > cfe-commits mailing list > > > [email protected] > > > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
