[libcxx-commits] [libcxx] r355367 - Fix -fsanitize=vptr badness in <__debug>

Eric Fiselier via libcxx-commits libcxx-commits at lists.llvm.org
Mon Mar 4 18:10:31 PST 2019


Author: ericwf
Date: Mon Mar  4 18:10:31 2019
New Revision: 355367

URL: http://llvm.org/viewvc/llvm-project?rev=355367&view=rev
Log:
Fix -fsanitize=vptr badness in <__debug>

Summary:

This patch fixes a lifetime bug when inserting a new container into the debug database. It is
diagnosed by UBSAN when debug mode is enabled. This patch corrects how nodes are constructed
during insertion.

The fix requires unconditionally breaking the debug mode ABI. Users should not expect ABI
stability from debug mode.

Reviewers: ldionne, serge-sans-paille, EricWF

Reviewed By: EricWF

Subscribers: mclow.lists, christof, libcxx-commits

Tags: #libc

Differential Revision: https://reviews.llvm.org/D58011

Modified:
    libcxx/trunk/include/__debug
    libcxx/trunk/lib/abi/CHANGELOG.TXT
    libcxx/trunk/lib/abi/x86_64-apple-darwin.v1.abilist
    libcxx/trunk/lib/abi/x86_64-apple-darwin.v2.abilist
    libcxx/trunk/lib/abi/x86_64-unknown-linux-gnu.v1.abilist
    libcxx/trunk/src/debug.cpp
    libcxx/trunk/test/libcxx/debug/containers/db_sequence_container_iterators.pass.cpp
    libcxx/trunk/utils/libcxx/test/config.py

Modified: libcxx/trunk/include/__debug
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/__debug?rev=355367&r1=355366&r2=355367&view=diff
==============================================================================
--- libcxx/trunk/include/__debug (original)
+++ libcxx/trunk/include/__debug Mon Mar  4 18:10:31 2019
@@ -250,16 +250,22 @@ public:
     __db_c_const_iterator __c_end() const;
     __db_i_const_iterator __i_end() const;
 
+    typedef __c_node*(_InsertConstruct)(void*, void*, __c_node*);
+
+    template <class _Cont>
+    _LIBCPP_INLINE_VISIBILITY static __c_node* __create_C_node(void *__mem, void *__c, __c_node *__next) {
+        return ::new(__mem) _C_node<_Cont>(__c, __next);
+    }
+
     template <class _Cont>
     _LIBCPP_INLINE_VISIBILITY
     void __insert_c(_Cont* __c)
     {
-        __c_node* __n = __insert_c(static_cast<void*>(__c));
-        ::new(__n) _C_node<_Cont>(__n->__c_, __n->__next_);
+        __insert_c(static_cast<void*>(__c), &__create_C_node<_Cont>);
     }
 
     void __insert_i(void* __i);
-    __c_node* __insert_c(void* __c);
+    void __insert_c(void* __c, _InsertConstruct* __fn);
     void __erase_c(void* __c);
 
     void __insert_ic(void* __i, const void* __c);

Modified: libcxx/trunk/lib/abi/CHANGELOG.TXT
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/lib/abi/CHANGELOG.TXT?rev=355367&r1=355366&r2=355367&view=diff
==============================================================================
--- libcxx/trunk/lib/abi/CHANGELOG.TXT (original)
+++ libcxx/trunk/lib/abi/CHANGELOG.TXT Mon Mar  4 18:10:31 2019
@@ -13,6 +13,32 @@ Afterwards the ABI list should be update
 New entries should be added directly below the "Version" header.
 
 -----------
+Version 9.0
+-----------
+
+* rTBD - Fix -fsanitize=vptr badness in <__debug>
+
+  This patch fixes a lifetime bug when inserting a new container into the debug database. It is
+  diagnosed by UBSAN when debug mode is enabled. This patch corrects how nodes are constructed
+  during insertion.
+
+  The fix requires unconditionally breaking the debug mode ABI. Users should not expect ABI
+  stability from debug mode.
+
+
+  x86_64-unknown-linux-gnu
+  ------------------------
+  Symbol added: _ZNSt3__111__libcpp_db10__insert_cEPvPFPNS_8__c_nodeES1_S1_S3_E
+  Symbol removed: _ZNSt3__111__libcpp_db10__insert_cEPv
+
+
+  x86_64-apple-apple-darwin
+  -------------------------
+  Symbol added: __ZNSt3__111__libcpp_db10__insert_cEPvPFPNS_8__c_nodeES1_S1_S3_E
+  Symbol removed: __ZNSt3__111__libcpp_db10__insert_cEPv
+
+
+-----------
 Version 8.0
 -----------
 

Modified: libcxx/trunk/lib/abi/x86_64-apple-darwin.v1.abilist
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/lib/abi/x86_64-apple-darwin.v1.abilist?rev=355367&r1=355366&r2=355367&view=diff
==============================================================================
--- libcxx/trunk/lib/abi/x86_64-apple-darwin.v1.abilist (original)
+++ libcxx/trunk/lib/abi/x86_64-apple-darwin.v1.abilist Mon Mar  4 18:10:31 2019
@@ -599,7 +599,7 @@
 {'is_defined': True, 'name': '__ZNSt3__110to_wstringEx', 'type': 'FUNC'}
 {'is_defined': True, 'name': '__ZNSt3__110to_wstringEy', 'type': 'FUNC'}
 {'is_defined': True, 'name': '__ZNSt3__111__call_onceERVmPvPFvS2_E', 'type': 'FUNC'}
-{'is_defined': True, 'name': '__ZNSt3__111__libcpp_db10__insert_cEPv', 'type': 'FUNC'}
+{'is_defined': True, 'name': '__ZNSt3__111__libcpp_db10__insert_cEPvPFPNS_8__c_nodeES1_S1_S3_E', 'type': 'FUNC'}
 {'is_defined': True, 'name': '__ZNSt3__111__libcpp_db10__insert_iEPv', 'type': 'FUNC'}
 {'is_defined': True, 'name': '__ZNSt3__111__libcpp_db11__insert_icEPvPKv', 'type': 'FUNC'}
 {'is_defined': True, 'name': '__ZNSt3__111__libcpp_db15__iterator_copyEPvPKv', 'type': 'FUNC'}

Modified: libcxx/trunk/lib/abi/x86_64-apple-darwin.v2.abilist
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/lib/abi/x86_64-apple-darwin.v2.abilist?rev=355367&r1=355366&r2=355367&view=diff
==============================================================================
--- libcxx/trunk/lib/abi/x86_64-apple-darwin.v2.abilist (original)
+++ libcxx/trunk/lib/abi/x86_64-apple-darwin.v2.abilist Mon Mar  4 18:10:31 2019
@@ -602,7 +602,7 @@
 {'is_defined': True, 'name': '__ZNSt3__210to_wstringEx', 'type': 'FUNC'}
 {'is_defined': True, 'name': '__ZNSt3__210to_wstringEy', 'type': 'FUNC'}
 {'is_defined': True, 'name': '__ZNSt3__211__call_onceERVmPvPFvS2_E', 'type': 'FUNC'}
-{'is_defined': True, 'name': '__ZNSt3__211__libcpp_db10__insert_cEPv', 'type': 'FUNC'}
+{'is_defined': True, 'name': '__ZNSt3__211__libcpp_db10__insert_cEPvPFPNS_8__c_nodeES1_S1_S3_E', 'type': 'FUNC'}
 {'is_defined': True, 'name': '__ZNSt3__211__libcpp_db10__insert_iEPv', 'type': 'FUNC'}
 {'is_defined': True, 'name': '__ZNSt3__211__libcpp_db11__insert_icEPvPKv', 'type': 'FUNC'}
 {'is_defined': True, 'name': '__ZNSt3__211__libcpp_db15__iterator_copyEPvPKv', 'type': 'FUNC'}

Modified: libcxx/trunk/lib/abi/x86_64-unknown-linux-gnu.v1.abilist
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/lib/abi/x86_64-unknown-linux-gnu.v1.abilist?rev=355367&r1=355366&r2=355367&view=diff
==============================================================================
--- libcxx/trunk/lib/abi/x86_64-unknown-linux-gnu.v1.abilist (original)
+++ libcxx/trunk/lib/abi/x86_64-unknown-linux-gnu.v1.abilist Mon Mar  4 18:10:31 2019
@@ -512,7 +512,7 @@
 {'is_defined': True, 'name': '_ZNSt3__110to_wstringEx', 'type': 'FUNC'}
 {'is_defined': True, 'name': '_ZNSt3__110to_wstringEy', 'type': 'FUNC'}
 {'is_defined': True, 'name': '_ZNSt3__111__call_onceERVmPvPFvS2_E', 'type': 'FUNC'}
-{'is_defined': True, 'name': '_ZNSt3__111__libcpp_db10__insert_cEPv', 'type': 'FUNC'}
+{'is_defined': True, 'name': '_ZNSt3__111__libcpp_db10__insert_cEPvPFPNS_8__c_nodeES1_S1_S3_E', 'type': 'FUNC'}
 {'is_defined': True, 'name': '_ZNSt3__111__libcpp_db10__insert_iEPv', 'type': 'FUNC'}
 {'is_defined': True, 'name': '_ZNSt3__111__libcpp_db11__insert_icEPvPKv', 'type': 'FUNC'}
 {'is_defined': True, 'name': '_ZNSt3__111__libcpp_db15__iterator_copyEPvPKv', 'type': 'FUNC'}

Modified: libcxx/trunk/src/debug.cpp
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/debug.cpp?rev=355367&r1=355366&r2=355367&view=diff
==============================================================================
--- libcxx/trunk/src/debug.cpp (original)
+++ libcxx/trunk/src/debug.cpp Mon Mar  4 18:10:31 2019
@@ -203,8 +203,8 @@ __libcpp_db::__insert_ic(void* __i, cons
     i->__c_ = c;
 }
 
-__c_node*
-__libcpp_db::__insert_c(void* __c)
+void
+__libcpp_db::__insert_c(void* __c, __libcpp_db::_InsertConstruct *__fn)
 {
 #ifndef _LIBCPP_HAS_NO_THREADS
     WLock _(mut());
@@ -234,15 +234,12 @@ __libcpp_db::__insert_c(void* __c)
     }
     size_t hc = hash<void*>()(__c) % static_cast<size_t>(__cend_ - __cbeg_);
     __c_node* p = __cbeg_[hc];
-    __c_node* r = __cbeg_[hc] =
-      static_cast<__c_node*>(malloc(sizeof(__c_node)));
-    if (__cbeg_[hc] == nullptr)
-        __throw_bad_alloc();
+    void *buf = malloc(sizeof(__c_node));
+    if (buf == nullptr)
+      __throw_bad_alloc();
+    __cbeg_[hc] = __fn(buf, __c, p);
 
-    r->__c_ = __c;
-    r->__next_ = p;
     ++__csz_;
-    return r;
 }
 
 void

Modified: libcxx/trunk/test/libcxx/debug/containers/db_sequence_container_iterators.pass.cpp
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/libcxx/debug/containers/db_sequence_container_iterators.pass.cpp?rev=355367&r1=355366&r2=355367&view=diff
==============================================================================
--- libcxx/trunk/test/libcxx/debug/containers/db_sequence_container_iterators.pass.cpp (original)
+++ libcxx/trunk/test/libcxx/debug/containers/db_sequence_container_iterators.pass.cpp Mon Mar  4 18:10:31 2019
@@ -40,6 +40,7 @@ public:
   static void run() {
     Base::run();
     try {
+      SanityTest();
       FrontOnEmptyContainer();
 
       if constexpr (CT != CT_ForwardList) {
@@ -71,6 +72,12 @@ public:
   }
 
 private:
+  static void SanityTest() {
+    CHECKPOINT("sanity test");
+    Container C = {1, 1, 1, 1};
+    ::DoNotOptimize(&C);
+  }
+
   static void RemoveFirstElem() {
     // See llvm.org/PR35564
     CHECKPOINT("remove(<first-elem>)");

Modified: libcxx/trunk/utils/libcxx/test/config.py
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/utils/libcxx/test/config.py?rev=355367&r1=355366&r2=355367&view=diff
==============================================================================
--- libcxx/trunk/utils/libcxx/test/config.py (original)
+++ libcxx/trunk/utils/libcxx/test/config.py Mon Mar  4 18:10:31 2019
@@ -939,7 +939,7 @@ class Configuration(object):
 
             def add_ubsan():
                 self.cxx.flags += ['-fsanitize=undefined',
-                                   '-fno-sanitize=vptr,function,float-divide-by-zero',
+                                   '-fno-sanitize=float-divide-by-zero',
                                    '-fno-sanitize-recover=all']
                 self.exec_env['UBSAN_OPTIONS'] = 'print_stacktrace=1'
                 self.config.available_features.add('ubsan')




More information about the libcxx-commits mailing list