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
git-svn-id: https://llvm.org/svn/llvm-project/libcxx/trunk@355367 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/include/__debug b/include/__debug
index 6ccb72c..281cf66 100644
--- a/include/__debug
+++ b/include/__debug
@@ -250,16 +250,22 @@
__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/lib/abi/CHANGELOG.TXT b/lib/abi/CHANGELOG.TXT
index 95bdb71..60227e3 100644
--- a/lib/abi/CHANGELOG.TXT
+++ b/lib/abi/CHANGELOG.TXT
@@ -13,6 +13,32 @@
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/lib/abi/x86_64-apple-darwin.v1.abilist b/lib/abi/x86_64-apple-darwin.v1.abilist
index 6349acf..e861fca 100644
--- a/lib/abi/x86_64-apple-darwin.v1.abilist
+++ b/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/lib/abi/x86_64-apple-darwin.v2.abilist b/lib/abi/x86_64-apple-darwin.v2.abilist
index adabbf6..90ea1eb 100644
--- a/lib/abi/x86_64-apple-darwin.v2.abilist
+++ b/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/lib/abi/x86_64-unknown-linux-gnu.v1.abilist b/lib/abi/x86_64-unknown-linux-gnu.v1.abilist
index 8a31ff8..3051d4c 100644
--- a/lib/abi/x86_64-unknown-linux-gnu.v1.abilist
+++ b/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/src/debug.cpp b/src/debug.cpp
index 2e88b85..28a1f70 100644
--- a/src/debug.cpp
+++ b/src/debug.cpp
@@ -203,8 +203,8 @@
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 @@
}
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/test/libcxx/debug/containers/db_sequence_container_iterators.pass.cpp b/test/libcxx/debug/containers/db_sequence_container_iterators.pass.cpp
index d05f9df..6ead98e 100644
--- a/test/libcxx/debug/containers/db_sequence_container_iterators.pass.cpp
+++ b/test/libcxx/debug/containers/db_sequence_container_iterators.pass.cpp
@@ -40,6 +40,7 @@
static void run() {
Base::run();
try {
+ SanityTest();
FrontOnEmptyContainer();
if constexpr (CT != CT_ForwardList) {
@@ -71,6 +72,12 @@
}
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/utils/libcxx/test/config.py b/utils/libcxx/test/config.py
index c619086..6daf356 100644
--- a/utils/libcxx/test/config.py
+++ b/utils/libcxx/test/config.py
@@ -939,7 +939,7 @@
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')
