[libcxx] Never use <cassert> within libc++

[EricWF]

Summary

It is my opinion that libc++ should never use <cassert>, including in the dylib. This patch remove all uses of assert from within libc++ and replaces most of them with _LIBCPP_ASSERT instead.

Additionally this patch turn LIBCXX_ENABLE_ASSERTIONS off by default, because the standard library should not be aborting user programs unless explicitly asked to.

[mclow.lists]

this LGTM. assert is not something we should have in the dylib.

include/__config

827

Does this belong here?

[EricWF]

include/__config

827

Yeah, so -DLIBCXX_ENABLE_ASSERTIONS=ON now define -D_LIBCPP_DEBUG=0 during the build. This suppresses std::strings extern template declarations and changes the export lists of libc++.dylib. This change ensures that enabling debug mode during the build doesn't do that.

[rmaprath]

@EricWF: My downstream builds have -DLLVM_ENABLE_ASSERTIONS=ON by default and they are showing some failures after this commit. Log attached:

log.txt102 KBDownload

It should be pretty easy to reproduce these (I'm on Ubuntu 16.04) if you enable assertions. Since the default has changed, the public builders won't be seeing these.

Are these failures expected?

Thanks.

/ Asiri

[rmaprath]

Sorry, I meant -DLIBCXX_ENABLE_ASSERTIONS=ON.

[EricWF]

It seems like weird usages of _LIBCPP_ASSERT in <string_view> are causing this problem.
Specifically the usages in <string_view> attempt to use _LIBCPP_ASSERT in C++11 constexpr functions and this currently does not work.

For now I've reverted this change in r292923.

CMakeLists.txt

 endif()
 
 #===============================================================================
 # Setup CMake Options
 #===============================================================================
 include(CMakeDependentOption)
 
 # Basic options ---------------------------------------------------------------
-option(LIBCXX_ENABLE_ASSERTIONS "Enable assertions independent of build mode." ON)
+option(LIBCXX_ENABLE_ASSERTIONS "Enable assertions independent of build mode." OFF)
 option(LIBCXX_ENABLE_SHARED "Build libc++ as a shared library." ON)
 option(LIBCXX_ENABLE_STATIC "Build libc++ as a static library." ON)
 option(LIBCXX_ENABLE_EXPERIMENTAL_LIBRARY "Build libc++experimental.a" ON)
 option(LIBCXX_ENABLE_FILESYSTEM
         "Build filesystem as part of libc++experimental.a" ${LIBCXX_ENABLE_EXPERIMENTAL_LIBRARY})
 option(LIBCXX_INCLUDE_TESTS "Build the libc++ tests." ${LLVM_INCLUDE_TESTS})
 
 # Benchmark options -----------------------------------------------------------
else()
     # Relax this restriction from HandleLLVMOptions
     string(REPLACE "-Wl,-z,defs" "" CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS}")
   endif()
 endif()
 
 # Assertion flags =============================================================
 define_if(LIBCXX_ENABLE_ASSERTIONS -UNDEBUG)
 define_if_not(LIBCXX_ENABLE_ASSERTIONS -DNDEBUG)
+define_if(LIBCXX_ENABLE_ASSERTIONS -D_LIBCPP_DEBUG=0)
 define_if(LIBCXX_DEBUG_BUILD -D_DEBUG)
 if (LIBCXX_ENABLE_ASSERTIONS AND NOT LIBCXX_DEBUG_BUILD)
   # MSVC doesn't like _DEBUG on release builds. See PR 4379.
   define_if_not(LIBCXX_TARGETING_MSVC -D_DEBUG)
 endif()
 
 # Modules flags ===============================================================
 # FIXME The libc++ sources are fundamentally non-modular. They need special

include/__config

 #ifdef _LIBCPP_DEBUG
 #   if _LIBCPP_DEBUG == 0
 #       define _LIBCPP_DEBUG_LEVEL 1
 #   elif _LIBCPP_DEBUG == 1
 #       define _LIBCPP_DEBUG_LEVEL 2
 #   else
 #       error Supported values for _LIBCPP_DEBUG are 0 and 1
 #   endif
+# if !defined(_LIBCPP_BUILDING_LIBRARY)
 #   define _LIBCPP_EXTERN_TEMPLATE(...)
-#endif
+# endif
+#endif
 
 #ifndef _LIBCPP_EXTERN_TEMPLATE
 #define _LIBCPP_EXTERN_TEMPLATE(...) extern template __VA_ARGS__;
 #endif
 
 #ifndef _LIBCPP_EXTERN_TEMPLATE2
 #define _LIBCPP_EXTERN_TEMPLATE2(...) extern template __VA_ARGS__;
 #endif

include/__threading_support

 #if defined(_LIBCPP_HAS_THREAD_API_EXTERNAL)
 # include <__external_threading>
 #elif !defined(_LIBCPP_HAS_NO_THREADS)
 
 #if defined(_LIBCPP_HAS_THREAD_API_PTHREAD)
 # include <pthread.h>
 # include <sched.h>
 #elif defined(_LIBCPP_HAS_THREAD_API_WIN32)
-#include <assert.h>
 #include <Windows.h>
 #include <process.h>
 #include <fibersapi.h>
 
 #include <chrono>
 #endif
 
 #if defined(_LIBCPP_HAS_THREAD_LIBRARY_EXTERNAL) || \

src/condition_variable.cpp

 //===-------------------- condition_variable.cpp --------------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is dual licensed under the MIT and the University of Illinois Open
 // Source Licenses. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "__config"
 
 #ifndef _LIBCPP_HAS_NO_THREADS
 
 #include "condition_variable"
 #include "thread"
 #include "system_error"
-#include "cassert"
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
 condition_variable::~condition_variable()
 {
     __libcpp_condvar_destroy(&__cv_);
 }
 

src/experimental/filesystem/path.cpp

 //===--------------------- filesystem/path.cpp ----------------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is dual licensed under the MIT and the University of Illinois Open
 // Source Licenses. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
-#undef NDEBUG
 #include "experimental/filesystem"
 #include "string_view"
 #include "utility"
-#include "cassert"
 
 namespace { namespace parser
 {
 using namespace std;
 using namespace std::experimental::filesystem;
 
 using string_view_t = path::__string_view;
 using string_view_pair = pair<string_view_t, string_view_t>;
void increment() noexcept {
     case PS_AtEnd:
       _LIBCPP_UNREACHABLE();
     }
   }
 
   void decrement() noexcept {
     const PosPtr REnd = &Path.front() - 1;
     const PosPtr RStart = getCurrentTokenStartPos() - 1;
-    assert(RStart != REnd);
 
     switch (State) {
     case PS_AtEnd: {
       // Try to consume a trailing separator or root directory first.
       if (PosPtr SepEnd = consumeSeparator(RStart, REnd)) {
         if (SepEnd == REnd)
           return makeState((RStart == REnd + 2) ? PS_InRootName : PS_InRootDir,
                            Path.data(), RStart + 1);

 
 string_view_t path::__root_path_raw() const
 {
     auto PP = PathParser::CreateBegin(__pn_);
     if (PP.State == PathParser::PS_InRootName) {
       auto NextCh = PP.peek();
       if (NextCh && *NextCh == '/') {
         ++PP;
-        assert(PP.State == PathParser::PS_InRootDir);
         return createView(__pn_.data(), &PP.RawEntry.back());
       }
       return PP.RawEntry;
     }
     if (PP.State == PathParser::PS_InRootDir)
       return *PP;
     return {};
 }

src/mutex.cpp

 //===------------------------- mutex.cpp ----------------------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is dual licensed under the MIT and the University of Illinois Open
 // Source Licenses. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #define _LIBCPP_BUILDING_MUTEX
 #include "mutex"
 #include "limits"
 #include "system_error"
-#include "cassert"
 #include "include/atomic_support.h"
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 #ifndef _LIBCPP_HAS_NO_THREADS
 
 const defer_lock_t  defer_lock = {};
 const try_to_lock_t try_to_lock = {};
 const adopt_lock_t  adopt_lock = {};
mutex::try_lock() _NOEXCEPT
     return __libcpp_mutex_trylock(&__m_);
 }
 
 void
 mutex::unlock() _NOEXCEPT
 {
     int ec = __libcpp_mutex_unlock(&__m_);
     (void)ec;
-    assert(ec == 0);
+    _LIBCPP_ASSERT(ec == 0, "call to mutex::unlock failed");
 }
 
 // recursive_mutex
 
 recursive_mutex::recursive_mutex()
 {
     int ec = __libcpp_recursive_mutex_init(&__m_);
     if (ec)
         __throw_system_error(ec, "recursive_mutex constructor failed");
 }
 
 recursive_mutex::~recursive_mutex()
 {
     int e = __libcpp_recursive_mutex_destroy(&__m_);
     (void)e;
-    assert(e == 0);
+    _LIBCPP_ASSERT(e == 0, "call to ~recursive_mutex() failed");
 }
 
 void
 recursive_mutex::lock()
 {
     int ec = __libcpp_recursive_mutex_lock(&__m_);
     if (ec)
         __throw_system_error(ec, "recursive_mutex lock failed");
 }
 
 void
 recursive_mutex::unlock() _NOEXCEPT
 {
     int e = __libcpp_recursive_mutex_unlock(&__m_);
     (void)e;
-    assert(e == 0);
+    _LIBCPP_ASSERT(e == 0, "call to recursive_mutex::unlock() failed");
 }
 
 bool
 recursive_mutex::try_lock() _NOEXCEPT
 {
     return __libcpp_recursive_mutex_trylock(&__m_);
 }
 

src/system_error.cpp

 #define _LIBCPP_BUILDING_SYSTEM_ERROR
 #include "system_error"
 
 #include "include/config_elast.h"
 #include "cerrno"
 #include "cstring"
 #include "cstdio"
 #include "cstdlib"
-#include "cassert"
 #include "string"
 #include "string.h"
+#include "__debug"
 
 #if defined(__ANDROID__)
 #include <android/api-level.h>
 #endif
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
 // class error_category
if ((ret = ::strerror_r(ev, buffer, strerror_buff_size)) != 0) {
         // If `ret == -1` then the error is specified using `errno`, otherwise
         // `ret` represents the error.
         const int new_errno = ret == -1 ? errno : ret;
         errno = old_errno;
         if (new_errno == EINVAL) {
             std::snprintf(buffer, strerror_buff_size, "Unknown error %d", ev);
             return string(buffer);
         } else {
-            assert(new_errno == ERANGE);
+            _LIBCPP_ASSERT(new_errno == ERANGE, "unexpected error from ::strerr_r");
             // FIXME maybe? 'strerror_buff_size' is likely to exceed the
             // maximum error size so ERANGE shouldn't be returned.
             std::abort();
         }
     }
     return string(buffer);
 }
 #endif