[libcxx] r215740 - Revert "Turn off extern templates for most uses."

Eric Fiselier eric at efcs.ca
Sat Aug 16 02:04:47 PDT 2014


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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140816/614a96f0/attachment.html>


More information about the cfe-commits mailing list