[libcxx-commits] [PATCH] D73134: Add invariant on <long string cap> >= __min_cap and change brittle __was_long cases

[Martijn Vels via Phabricator via libcxx-commits]

mvels created this revision.
Herald added subscribers: libcxx-commits, christof.
Herald added a project: libc++.
mvels added a reviewer: EricWF.
Herald added a reviewer: mclow.lists.

The partial inlining optimization for short strings could allocate strings at __min_cap size. Code in __grow_by assumed cap == __min_cap - 1 as proof that the input is an allocated long string leading to bad deallocation.

- add the invariant that no long string should have <= min_cap capacity
- use actual __is_long() for '__was_long' observation in __grow_byt logic

Combined these defend against both the (previously not expressed) invariant and its consequences.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D73134

Files:
  libcxx/include/string


Index: libcxx/include/string
===================================================================
--- libcxx/include/string
+++ libcxx/include/string
@@ -2144,6 +2144,7 @@
     size_type __ms = max_size();
     if (__delta_cap > __ms - __old_cap - 1)
         this->__throw_length_error();
+    bool __was_long = __is_long();
     pointer __old_p = __get_pointer();
     size_type __cap = __old_cap < __ms / 2 - __alignment ?
                           __recommend(_VSTD::max(__old_cap + __delta_cap, 2 * __old_cap)) :
@@ -2159,7 +2160,7 @@
     if (__sec_cp_sz != 0)
         traits_type::copy(_VSTD::__to_address(__p) + __n_copy + __n_add,
                           _VSTD::__to_address(__old_p) + __n_copy + __n_del, __sec_cp_sz);
-    if (__old_cap+1 != __min_cap)
+    if (__was_long)
         __alloc_traits::deallocate(__alloc(), __old_p, __old_cap+1);
     __set_long_pointer(__p);
     __set_long_cap(__cap+1);
@@ -2176,6 +2177,7 @@
     size_type __ms = max_size();
     if (__delta_cap > __ms - __old_cap)
         this->__throw_length_error();
+    bool __was_long = __is_long();
     pointer __old_p = __get_pointer();
     size_type __cap = __old_cap < __ms / 2 - __alignment ?
                           __recommend(_VSTD::max(__old_cap + __delta_cap, 2 * __old_cap)) :
@@ -2190,7 +2192,7 @@
         traits_type::copy(_VSTD::__to_address(__p) + __n_copy + __n_add,
                           _VSTD::__to_address(__old_p) + __n_copy + __n_del,
                           __sec_cp_sz);
-    if (__old_cap+1 != __min_cap)
+    if (__was_long)
         __alloc_traits::deallocate(__alloc(), __old_p, __old_cap+1);
     __set_long_pointer(__p);
     __set_long_cap(__cap+1);
@@ -3153,6 +3155,7 @@
 {
     if (__res_arg > max_size())
         this->__throw_length_error();
+    bool __was_long = __is_long();
     size_type __cap = capacity();
     size_type __sz = size();
     __res_arg = _VSTD::max(__res_arg, __sz);
@@ -3161,9 +3164,8 @@
     {
         pointer __new_data, __p;
         bool __was_long, __now_long;
-        if (__res_arg == __min_cap - 1)
+        if (__was_long)
         {
-            __was_long = true;
             __now_long = false;
             __new_data = __get_short_pointer();
             __p = __get_long_pointer();
@@ -3826,6 +3828,8 @@
         return false;
     if (capacity() < __min_cap - 1)
         return false;
+    if (__is_long() && capacity() < __min_cap)
+        return false;
     if (data() == 0)
         return false;
     if (data()[size()] != value_type(0))


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D73134.239415.patch
Type: text/x-patch
Size: 2525 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/libcxx-commits/attachments/20200121/607ca116/attachment-0001.bin>