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

llvm-svn: 355367

diff --git a/libcxx/include/__debug b/libcxx/include/__debug
index 6ccb72cb8ad0e25e416fcf7a87ea4129bf241f72..281cf6675c9bc47b9029563cfeacbc827ffcf231 100644 (file)
--- a/libcxx/include/__debug
+++ b/libcxx/include/__debug
@@ -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);

diff --git a/libcxx/lib/abi/CHANGELOG.TXT b/libcxx/lib/abi/CHANGELOG.TXT
index 95bdb714e682d227b98dfbdbd33465660c23a0ce..60227e3248a39cec35257542e19d2666b7d03ca9 100644 (file)
--- a/libcxx/lib/abi/CHANGELOG.TXT
+++ b/libcxx/lib/abi/CHANGELOG.TXT
@@ -12,6 +12,32 @@ Afterwards the ABI list should be updated to include the new changes.
 
 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
 -----------

diff --git a/libcxx/lib/abi/x86_64-apple-darwin.v1.abilist b/libcxx/lib/abi/x86_64-apple-darwin.v1.abilist
index 6349acf0534c61799d1625f8ce9035953ace279e..e861fcab8b38bd4f1014dd94c85a6a9897241ad4 100644 (file)
--- a/libcxx/lib/abi/x86_64-apple-darwin.v1.abilist
+++ b/libcxx/lib/abi/x86_64-apple-darwin.v1.abilist
@@ -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'}

diff --git a/libcxx/lib/abi/x86_64-apple-darwin.v2.abilist b/libcxx/lib/abi/x86_64-apple-darwin.v2.abilist
index adabbf6e0333176165f0c6631727e4349ff24d08..90ea1ebb0ec66a04a3ab91a8787cc3f4f12920d1 100644 (file)
--- a/libcxx/lib/abi/x86_64-apple-darwin.v2.abilist
+++ b/libcxx/lib/abi/x86_64-apple-darwin.v2.abilist
@@ -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'}

diff --git a/libcxx/lib/abi/x86_64-unknown-linux-gnu.v1.abilist b/libcxx/lib/abi/x86_64-unknown-linux-gnu.v1.abilist
index 8a31ff82ae9ddacc7b9f2f22cc622acd321cbcb4..3051d4cb72bce022dede54d0c0d031b23c1f8856 100644 (file)
--- a/libcxx/lib/abi/x86_64-unknown-linux-gnu.v1.abilist
+++ b/libcxx/lib/abi/x86_64-unknown-linux-gnu.v1.abilist
@@ -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'}

diff --git a/libcxx/src/debug.cpp b/libcxx/src/debug.cpp
index 2e88b859be333b59f1e46cc006699b5713e7f12a..28a1f70a59b02a2f8a784d602b6cf91ca3c5bc84 100644 (file)
--- a/libcxx/src/debug.cpp
+++ b/libcxx/src/debug.cpp
@@ -203,8 +203,8 @@ __libcpp_db::__insert_ic(void* __i, const void* __c)
     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

diff --git a/libcxx/test/libcxx/debug/containers/db_sequence_container_iterators.pass.cpp b/libcxx/test/libcxx/debug/containers/db_sequence_container_iterators.pass.cpp
index d05f9df3b98b960d6410af1de0681eaed787f98d..6ead98ebcb797cd3aff4def4d9c700fc6b88b41e 100644 (file)
--- a/libcxx/test/libcxx/debug/containers/db_sequence_container_iterators.pass.cpp
+++ b/libcxx/test/libcxx/debug/containers/db_sequence_container_iterators.pass.cpp
@@ -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>)");

diff --git a/libcxx/utils/libcxx/test/config.py b/libcxx/utils/libcxx/test/config.py
index c619086b61d62c5277bca405e2055e18b1fce51d..6daf356ef9f2f8c32f6aa29271d7e2d703c645d8 100644 (file)
--- a/libcxx/utils/libcxx/test/config.py
+++ b/libcxx/utils/libcxx/test/config.py
@@ -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')