Hi All, I've attached a reproducer. just run `make` to build and execute it.
/Eric On Sun, Aug 17, 2014 at 2:38 PM, Eric Fiselier <[email protected]> wrote: > Hi Justin, > > > 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. > > That sounds like a likely cause. I still want to check out the > "always_inline" attribute. > > > > > 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. > > Unfortunately I suspect building with -DLLVM_USE_SANITIZER only works > when building in-tree. > Something else we need to change to make libc++ work better with > sanitizers. > I'll try and get a patch out tonight that fixes most of these issues. > I'll also have to look into the other errors. > > Chandler mentioned that MSAN isn't very useful without an instrumented > standard library. This puts us in a pickle. > Most of the time libc++ is shipped to users instrumented. I really don't > want the end-user to see all these FPs. > Without the extern template declarations they don't appear. For that > reason I think we should keep them out for now. > > We could guard the definition of _LIBCPP_EXTERN_TEMPLATE with > __has_feature(memory_sanitizer) (or w/e the correct name is). > But this will only work on clang and not GCC or MSVC. > > Marshall is back tomorrow. I think he will have something to add to this > as well. > > To the MSAN people: Can you offer any information on these false positives > we are seeing? > > > /Eric > > > > > > On Sun, Aug 17, 2014 at 2:39 AM, Chandler Carruth <[email protected]> > wrote: > >> 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 >>> >> >> >
test_case.tar.gz
Description: GNU Zip compressed data
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
