<div dir="ltr">Hi All,<div><br></div><div>I've attached a reproducer. just run `make` to build and execute it.</div><div><br></div><div>/Eric</div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Sun, Aug 17, 2014 at 2:38 PM, Eric Fiselier <span dir="ltr"><<a href="mailto:eric@efcs.ca" target="_blank">eric@efcs.ca</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Hi Justin,<div class=""><div><br></div><div>> <span style="font-family:arial,sans-serif;font-size:13px">I'm pretty sure the extra false positives you're seeing here are the</span></div>
<span style="font-family:arial,sans-serif;font-size:13px">result of using an instrumented program with an uninstrumented shared</span><br style="font-family:arial,sans-serif;font-size:13px">
<span style="font-family:arial,sans-serif;font-size:13px">library.</span><div><span style="font-family:arial,sans-serif;font-size:13px"><br></span></div></div><div><font face="arial, sans-serif">That sounds like a likely cause. I still want to check out the "always_inline" attribute.</font></div>
<div class="">
<div><font face="arial, sans-serif"><br></font></div><div><font face="arial, sans-serif"><br></font></div><div><font face="arial, sans-serif"><br></font></div><div><font face="arial, sans-serif">> </font><span style="font-family:arial,sans-serif;font-size:13px">On the other hand, if I build libc++.so with -fsanitize=memory (via</span></div>
<span style="font-family:arial,sans-serif;font-size:13px">-DLLVM_USE_SANITIZER=Memory) and then run the tests against that with</span><br style="font-family:arial,sans-serif;font-size:13px"><span style="font-family:arial,sans-serif;font-size:13px">the memory sanitizer, the new errors in string go away. There are quite</span><br style="font-family:arial,sans-serif;font-size:13px">
<span style="font-family:arial,sans-serif;font-size:13px">a few tests that seem to have other problems this way, but that's true</span><br style="font-family:arial,sans-serif;font-size:13px"><span style="font-family:arial,sans-serif;font-size:13px">with or without the template instantiations.</span><div>
<span style="font-family:arial,sans-serif;font-size:13px"><br></span></div></div><div><font face="arial, sans-serif">Unfortunately</font><span style="font-family:arial,sans-serif;font-size:13px"> I suspect building with -DLLVM_USE_SANITIZER only works when building in-tree.</span></div>
<div><span style="font-family:arial,sans-serif;font-size:13px">Something else we need to change to make libc++ work better with sanitizers.</span></div><div><span style="font-family:arial,sans-serif;font-size:13px">I'll try and get a patch out tonight that fixes most of these issues.</span></div>
<div><span style="font-family:arial,sans-serif;font-size:13px">I'll also have to look into the other errors.</span></div><div><span style="font-family:arial,sans-serif;font-size:13px"><br></span></div><div><span style="font-family:arial,sans-serif;font-size:13px">Chandler mentioned that MSAN isn't very useful without an instrumented standard library. This puts us in a pickle.</span></div>
<div><font face="arial, sans-serif">Most of the time libc++ is shipped to users instrumented. I really don't want the end-user to see all these FPs.</font></div><div><font face="arial, sans-serif">Without the extern template declarations they don't appear. For that reason I think we should keep them out for now.</font></div>
<div><font face="arial, sans-serif"><br></font></div><div><font face="arial, sans-serif">We could guard the definition of _LIBCPP_EXTERN_TEMPLATE with __has_feature(memory_sanitizer) (or w/e the correct name is).</font></div>
<div><font face="arial, sans-serif">But this will only work on clang and not GCC or MSVC. </font></div><div><font face="arial, sans-serif"><br></font></div><div><font face="arial, sans-serif">Marshall is back tomorrow. I think he will have something to add to this as well.</font></div>
<div><font face="arial, sans-serif"><br></font></div><div><font face="arial, sans-serif">To the MSAN people: Can you offer any information on these false positives we are seeing?</font></div><span class="HOEnZb"><font color="#888888"><div>
<font face="arial, sans-serif"><br>
</font></div><div><font face="arial, sans-serif"><br></font></div><div><font face="arial, sans-serif">/Eric</font></div><div><font face="arial, sans-serif"><br></font></div><div><font face="arial, sans-serif"><br></font></div>
<div><span style="font-family:arial,sans-serif;font-size:13px"><br></span></div></font></span></div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br><br><div class="gmail_quote">On Sun, Aug 17, 2014 at 2:39 AM, Chandler Carruth <span dir="ltr"><<a href="mailto:chandlerc@google.com" target="_blank">chandlerc@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">It would be useful to include some folks who work on the sanitizers.<div><br></div><div>MSan in particular is only really useful when linking against a sanitized standard library.</div>
</div><div><div><div class="gmail_extra">
<br><br><div class="gmail_quote">On Sun, Aug 17, 2014 at 12:57 AM, Justin Bogner <span dir="ltr"><<a href="mailto:mail@justinbogner.com" target="_blank">mail@justinbogner.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
I'm pretty sure the extra false positives you're seeing here are the<br>
result of using an instrumented program with an uninstrumented shared<br>
library.<br>
<br>
I wasn't able to get any interesting results from your example, but I<br>
followed your instructions to run the libc++ tests with msan. That is, I<br>
built llvm and clang with libcxx and libcxxabi checked out, and added<br>
compile_flags=['-fsanitize=memory'] in libc++'s lit.cfg. This reproduces<br>
the false positives in string that you mentioned.<br>
<br>
On the other hand, if I build libc++.so with -fsanitize=memory (via<br>
-DLLVM_USE_SANITIZER=Memory) and then run the tests against that with<br>
the memory sanitizer, the new errors in string go away. There are quite<br>
a few tests that seem to have other problems this way, but that's true<br>
with or without the template instantiations.<br>
<br>
So it looks to me like the "there is no good way to run libc++ tests<br>
with sanitizers" claim is pretty accurate ;)<br>
<div><div><br>
Eric Fiselier <<a href="mailto:eric@efcs.ca" target="_blank">eric@efcs.ca</a>> writes:<br>
> Oh jeez, I forgot a very important aspect to the reproducer. Last time I<br>
> looked into this it seemed that __attribute__ ((__always_inline__)) also<br>
> played a part in this. In the last example "get_x()" should have this<br>
> attribute.<br>
><br>
> On Sat, Aug 16, 2014 at 3:04 AM, Eric Fiselier <<a href="mailto:eric@efcs.ca" target="_blank">eric@efcs.ca</a>> wrote:<br>
><br>
> I could have been a lot clearer. It seems to be a bug in MSAN. I've run<br>
> into it before. <br>
> The code that causes it is something like:<br>
><br>
> // TestClass.hpp<br>
> template <class T><br>
> class TestClass<br>
> {<br>
> int x{0};<br>
> int get_x() { return x; }<br>
> };<br>
><br>
> extern template class TestClass<int>;<br>
><br>
> // TestClass.cpp<br>
> template class TestClass<int>;<br>
><br>
> // main.cpp <br>
> #include "TestClass.hpp"<br>
> int main()<br>
> {<br>
> TestClass<int> t;<br>
> // MSAN fires on use of uninitialized value here.<br>
> int x = t.get_x();te <br>
> }<br>
><br>
> That's by no means a reproducer, but it represents similar code to what<br>
> causes the test failures in "/test/string". MSAN seems to fire *any* time<br>
> a value is returned from a method of a class that has been instantiated<br>
> externally. I think the shared object boundary might be important. I<br>
> haven't had time to investigate. <br>
><br>
> Unfortunately there is currently no good way to run the libc++ tests with<br>
> sanitizers. The easiest way is to just insert "-fsanitize=memory" into the<br>
> `compile_flags` list in `test/lit.cfg'. Then just "cd test/string ; lit<br>
> -sv . "<br>
><br>
> I hope that made it a little clearer.<br>
><br>
> /Eric<br>
><br>
> P.S. These failures arn't localized to "test/string". They are anywhere<br>
> templates are instantiated externally. String is just representative. <br>
><br>
> On Sat, Aug 16, 2014 at 2:42 AM, Justin Bogner <<a href="mailto:mail@justinbogner.com" target="_blank">mail@justinbogner.com</a>><br>
> wrote:<br>
><br>
> Eric Fiselier <<a href="mailto:eric@efcs.ca" target="_blank">eric@efcs.ca</a>> writes:<br>
> > Hi Justin,<br>
> ><br>
> > I'm not quite sure we are should do that right now. <br>
> > If your compiling with MSAN and you call a function in an extern<br>
> > template MSAN will fire. <br>
><br>
> I don't think I understand what you mean here. Which function? Why<br>
> does<br>
> msan fire?<br>
><br>
> > For example testing test/string with MSAN enabled causes 124<br>
> failures.<br>
> > Normally there are just 2.<br>
><br>
> If there are invalid memory accesses we should fix them. I don't see<br>
> how<br>
> explicit template instantiations would cause memory errors though.<br>
><br>
> Are there bots that show this issue? How can I reproduce it?<br>
><br>
> > We could probably guard the _LIBCPP_EXTERN_TEMPLATE definition to<br>
> > check for MSAN but for now I don't think we are ready to re-enable<br>
> > external template definitions.<br>
> ><br>
> > /Eric<br>
> ><br>
> > On Fri, Aug 15, 2014 at 11:58 AM, Justin Bogner <<br>
> <a href="mailto:mail@justinbogner.com" target="_blank">mail@justinbogner.com</a>> wrote:<br>
> ><br>
> > Author: bogner<br>
> > Date: Fri Aug 15 12:58:56 2014<br>
> > New Revision: 215740<br>
> ><br>
> > URL: <a href="http://llvm.org/viewvc/llvm-project?rev=215740&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=215740&view=rev</a><br>
> > Log:<br>
> > Revert "Turn off extern templates for most uses."<br>
> ><br>
> > Turning off explicit template instantiation leads to a pretty<br>
> > significant build time and code size cost. We're better off<br>
> dealing<br>
> > with ABI incompatibility issues that come up in a less heavy<br>
> handed<br>
> > way.<br>
> ><br>
> > This reverts commit r189610.<br>
> ><br>
> > Modified:<br>
> > libcxx/trunk/include/__config<br>
> > libcxx/trunk/src/algorithm.cpp<br>
> > libcxx/trunk/src/ios.cpp<br>
> > libcxx/trunk/src/locale.cpp<br>
> > libcxx/trunk/src/string.cpp<br>
> > libcxx/trunk/src/valarray.cpp<br>
> ><br>
> > Modified: libcxx/trunk/include/__config<br>
> > URL: <a href="http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/" target="_blank">http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/</a><br>
> __config?rev<br>
> > =215740&r1=215739&r2=215740&view=diff<br>
> > ================================================================<br>
> ==========<br>
> > ====<br>
> > --- libcxx/trunk/include/__config (original)<br>
> > +++ libcxx/trunk/include/__config Fri Aug 15 12:58:56 2014<br>
> > @@ -608,7 +608,7 @@ template <unsigned> struct __static_asse<br>
> > #endif<br>
> ><br>
> > #ifndef _LIBCPP_EXTERN_TEMPLATE<br>
> > -#define _LIBCPP_EXTERN_TEMPLATE(...)<br>
> > +#define _LIBCPP_EXTERN_TEMPLATE(...) extern template<br>
> __VA_ARGS__;<br>
> > #endif<br>
> ><br>
> > #ifndef _LIBCPP_EXTERN_TEMPLATE2<br>
> ><br>
> > Modified: libcxx/trunk/src/algorithm.cpp<br>
> > URL: <a href="http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/" target="_blank">http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/</a><br>
> algorithm.cpp?<br>
> > rev=215740&r1=215739&r2=215740&view=diff<br>
> > ================================================================<br>
> ==========<br>
> > ====<br>
> > --- libcxx/trunk/src/algorithm.cpp (original)<br>
> > +++ libcxx/trunk/src/algorithm.cpp Fri Aug 15 12:58:56 2014<br>
> > @@ -7,7 +7,6 @@<br>
> > //<br>
> > //===<br>
> > <br>
> ----------------------------------------------------------------------<br>
> ===/<br>
> > /<br>
> ><br>
> > -#define _LIBCPP_EXTERN_TEMPLATE(...) extern template<br>
> __VA_ARGS__;<br>
> > #include "algorithm"<br>
> > #include "random"<br>
> > #include "mutex"<br>
> ><br>
> > Modified: libcxx/trunk/src/ios.cpp<br>
> > URL: <a href="http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/" target="_blank">http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/</a><br>
> ios.cpp?rev=<br>
> > 215740&r1=215739&r2=215740&view=diff<br>
> > ================================================================<br>
> ==========<br>
> > ====<br>
> > --- libcxx/trunk/src/ios.cpp (original)<br>
> > +++ libcxx/trunk/src/ios.cpp Fri Aug 15 12:58:56 2014<br>
> > @@ -7,8 +7,6 @@<br>
> > //<br>
> > //===<br>
> > <br>
> ----------------------------------------------------------------------<br>
> ===/<br>
> > /<br>
> ><br>
> > -#define _LIBCPP_EXTERN_TEMPLATE(...) extern template<br>
> __VA_ARGS__;<br>
> > -<br>
> > #include "ios"<br>
> > #include "streambuf"<br>
> > #include "istream"<br>
> ><br>
> > Modified: libcxx/trunk/src/locale.cpp<br>
> > URL: <a href="http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/" target="_blank">http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/</a><br>
> locale.cpp?rev=<br>
> > 215740&r1=215739&r2=215740&view=diff<br>
> > ================================================================<br>
> ==========<br>
> > ====<br>
> > --- libcxx/trunk/src/locale.cpp (original)<br>
> > +++ libcxx/trunk/src/locale.cpp Fri Aug 15 12:58:56 2014<br>
> > @@ -7,8 +7,6 @@<br>
> > //<br>
> > //===<br>
> > <br>
> ----------------------------------------------------------------------<br>
> ===/<br>
> > /<br>
> ><br>
> > -#define _LIBCPP_EXTERN_TEMPLATE(...) extern template<br>
> __VA_ARGS__;<br>
> > -<br>
> > // On Solaris, we need to define something to make the C99<br>
> parts of<br>
> > localeconv<br>
> > // visible.<br>
> > #ifdef __sun__<br>
> ><br>
> > Modified: libcxx/trunk/src/string.cpp<br>
> > URL: <a href="http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/" target="_blank">http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/</a><br>
> string.cpp?rev=<br>
> > 215740&r1=215739&r2=215740&view=diff<br>
> > ================================================================<br>
> ==========<br>
> > ====<br>
> > --- libcxx/trunk/src/string.cpp (original)<br>
> > +++ libcxx/trunk/src/string.cpp Fri Aug 15 12:58:56 2014<br>
> > @@ -7,8 +7,6 @@<br>
> > //<br>
> > //===<br>
> > <br>
> ----------------------------------------------------------------------<br>
> ===/<br>
> > /<br>
> ><br>
> > -#define _LIBCPP_EXTERN_TEMPLATE(...) extern template<br>
> __VA_ARGS__;<br>
> > -<br>
> > #include "string"<br>
> > #include "cstdlib"<br>
> > #include "cwchar"<br>
> ><br>
> > Modified: libcxx/trunk/src/valarray.cpp<br>
> > URL: <a href="http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/" target="_blank">http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/</a><br>
> valarray.cpp?rev<br>
> > =215740&r1=215739&r2=215740&view=diff<br>
> > ================================================================<br>
> ==========<br>
> > ====<br>
> > --- libcxx/trunk/src/valarray.cpp (original)<br>
> > +++ libcxx/trunk/src/valarray.cpp Fri Aug 15 12:58:56 2014<br>
> > @@ -7,8 +7,6 @@<br>
> > //<br>
> > //===<br>
> > <br>
> ----------------------------------------------------------------------<br>
> ===/<br>
> > /<br>
> ><br>
> > -#define _LIBCPP_EXTERN_TEMPLATE(...) extern template<br>
> __VA_ARGS__;<br>
> > -<br>
> > #include "valarray"<br>
> ><br>
> > _LIBCPP_BEGIN_NAMESPACE_STD<br>
> ><br>
> > _______________________________________________<br>
> > cfe-commits mailing list<br>
> > <a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
> > <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</div></div></blockquote></div><br></div>
</div></div></blockquote></div><br></div>
</div></div></blockquote></div><br></div>