[libcxx] r215740 - Revert "Turn off extern templates for most uses."
Justin Bogner
mail at justinbogner.com
Sun Aug 17 00:57:05 PDT 2014
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 <eric at efcs.ca> 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 <eric at efcs.ca> 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 <mail at justinbogner.com>
> wrote:
>
> Eric Fiselier <eric at efcs.ca> 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 <
> mail at justinbogner.com> 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
> > cfe-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list