basic_fields uses intrusive base hooks:

fix #1270

basic_fields::value_type uses base hooks instead of member hooks,
otherwise MSVC can sometimes produce undefined behavior when
attempting to recover the base class from a data member in
certain build configurations.

CHANGELOG.md

@@ -1,3 +1,9 @@
+Version 186:
+
+* basic_fields uses intrusive base hooks
+
+--------------------------------------------------------------------------------
+
 Version 185:
 
 * Remove extraneous function

doc/qbk/09_releases.qbk

@@ -27,6 +27,8 @@
 
 * ([issue 1091]) Fix timer on websocket upgrade in examples
 
+* ([issue 1270]) `basic_fields` uses intrusive base hooks
+
 * Workaround for http-server-fast and libstdc++
 
 

include/boost/beast/http/fields.hpp

@@ -65,6 +65,8 @@ class basic_fields
 
     static std::size_t constexpr max_static_buffer = 4096;
 
+    struct element;
+
     using off_t = std::uint16_t;
 
 public:
@@ -76,24 +78,20 @@ public:
     {
         friend class basic_fields;
 
-        boost::asio::const_buffer
-        buffer() const;
-
-        value_type(field name,
-            string_view sname, string_view value);
-
-        boost::intrusive::list_member_hook<
-            boost::intrusive::link_mode<
-                boost::intrusive::normal_link>>
-                    list_hook_;
-        boost::intrusive::set_member_hook<
-            boost::intrusive::link_mode<
-                boost::intrusive::normal_link>>
-                    set_hook_;
         off_t off_;
         off_t len_;
         field f_;
 
+        char*
+        data() const;
+
+        boost::asio::const_buffer
+        buffer() const;
+
+    protected:
+        value_type(field name,
+            string_view sname, string_view value);
+
     public:
         /// Constructor (deleted)
         value_type(value_type const&) = delete;
@@ -118,13 +116,16 @@ public:
 
         The case-comparison operation is defined only for low-ASCII characters.
     */
+#if BOOST_BEAST_DOXYGEN
+    using key_compare = implementation_defined;
+#else
     struct key_compare : beast::iless
+#endif
     {
         /// Returns `true` if lhs is less than rhs using a strict ordering
-        template<class String>
         bool
         operator()(
-            String const& lhs,
+            string_view lhs,
             value_type const& rhs) const noexcept
         {
             if(lhs.size() < rhs.name_string().size())
@@ -135,11 +136,10 @@ public:
         }
 
         /// Returns `true` if lhs is less than rhs using a strict ordering
-        template<class String>
         bool
         operator()(
             value_type const& lhs,
-            String const& rhs) const noexcept
+            string_view rhs) const noexcept
         {
             if(lhs.name_string().size() < rhs.size())
                 return true;
@@ -170,30 +170,36 @@ public:
 #endif
 
 private:
-    using list_t = typename boost::intrusive::make_list<
+    struct element
-        value_type, boost::intrusive::member_hook<
+        : public boost::intrusive::list_base_hook<
-            value_type, boost::intrusive::list_member_hook<
             boost::intrusive::link_mode<
-                    boost::intrusive::normal_link>>,
+                boost::intrusive::normal_link>>
-                        &value_type::list_hook_>,
+        , public boost::intrusive::set_base_hook<
-                            boost::intrusive::constant_time_size<
+            boost::intrusive::link_mode<
-                                false>>::type;
+                boost::intrusive::normal_link>>
+        , public value_type
+    {
+        element(field name,
+            string_view sname, string_view value);
+    };
+
+    using list_t = typename boost::intrusive::make_list<
+        element,
+        boost::intrusive::constant_time_size<false>
+            >::type;
 
     using set_t = typename boost::intrusive::make_multiset<
-        value_type, boost::intrusive::member_hook<value_type,
+        element,
-            boost::intrusive::set_member_hook<
-                boost::intrusive::link_mode<
-                    boost::intrusive::normal_link>>,
-                        &value_type::set_hook_>,
         boost::intrusive::constant_time_size<true>,
-                                boost::intrusive::compare<key_compare>>::type;
+        boost::intrusive::compare<key_compare>
+            >::type;
 
     using align_type = typename
-        boost::type_with_alignment<alignof(value_type)>::type;
+        boost::type_with_alignment<alignof(element)>::type;
 
     using rebind_type = typename
         beast::detail::allocator_traits<Allocator>::
-            template rebind_alloc<align_type>;
+            template rebind_alloc<element>;
 
     using alloc_traits =
         beast::detail::allocator_traits<rebind_type>;
@@ -704,15 +710,15 @@ private:
     template<class OtherAlloc>
     friend class basic_fields;
 
-    value_type&
+    element&
     new_element(field name,
         string_view sname, string_view value);
 
     void
-    delete_element(value_type& e);
+    delete_element(element& e);
 
     void
-    set_element(value_type& e);
+    set_element(element& e);
 
     void
     realloc_string(string_view& dest, string_view s);

include/boost/beast/http/impl/fields.ipp

@@ -271,19 +271,38 @@ writer(basic_fields const& f,
 //------------------------------------------------------------------------------
 
 template<class Allocator>
+char*
 basic_fields<Allocator>::
 value_type::
-value_type(
+data() const
-    field name,
+{
-    string_view sname,
+    return const_cast<char*>(
-    string_view value)
+        reinterpret_cast<char const*>(
+            static_cast<element const*>(this) + 1));
+}
+
+template<class Allocator>
+boost::asio::const_buffer
+basic_fields<Allocator>::
+value_type::
+buffer() const
+{
+    return boost::asio::const_buffer{data(),
+        static_cast<std::size_t>(off_) + len_ + 2};
+}
+
+template<class Allocator>
+basic_fields<Allocator>::
+value_type::
+value_type(field name,
+    string_view sname, string_view value)
     : off_(static_cast<off_t>(sname.size() + 2))
     , len_(static_cast<off_t>(value.size()))
     , f_(name)
 {
     //BOOST_ASSERT(name == field::unknown ||
     //    iequals(sname, to_string(name)));
-    char* p = reinterpret_cast<char*>(this + 1);
+    char* p = data();
     p[off_-2] = ':';
     p[off_-1] = ' ';
     p[off_ + len_] = '\r';
@@ -293,7 +312,6 @@ value_type(
 }
 
 template<class Allocator>
-inline
 field
 basic_fields<Allocator>::
 value_type::
@@ -303,39 +321,32 @@ name() const
 }
 
 template<class Allocator>
-inline
 string_view const
 basic_fields<Allocator>::
 value_type::
 name_string() const
 {
-    return {reinterpret_cast<
+    return {data(),
-        char const*>(this + 1),
         static_cast<std::size_t>(off_ - 2)};
 }
 
 template<class Allocator>
-inline
 string_view const
 basic_fields<Allocator>::
 value_type::
 value() const
 {
-    return {reinterpret_cast<
+    return {data() + off_,
-        char const*>(this + 1) + off_,
         static_cast<std::size_t>(len_)};
 }
 
 template<class Allocator>
-inline
-boost::asio::const_buffer
 basic_fields<Allocator>::
-value_type::
+element::
-buffer() const
+element(field name,
+    string_view sname, string_view value)
+    : value_type(name, sname, value)
 {
-    return boost::asio::const_buffer{
-        reinterpret_cast<char const*>(this + 1),
-        static_cast<std::size_t>(off_) + len_ + 2};
 }
 
 //------------------------------------------------------------------------------
@@ -614,7 +625,7 @@ erase(const_iterator pos) ->
     auto& e = *next++;
     set_.erase(e);
     list_.erase(pos);
-    delete_element(const_cast<value_type&>(e));
+    delete_element(const_cast<element&>(e));
     return next;
 }
 
@@ -634,7 +645,7 @@ erase(string_view name)
 {
     std::size_t n =0;
     set_.erase_and_dispose(name, key_compare{},
-        [&](value_type* e)
+        [&](element* e)
         {
             ++n;
             list_.erase(list_.iterator_to(*e));
@@ -1138,7 +1149,7 @@ auto
 basic_fields<Allocator>::
 new_element(field name,
     string_view sname, string_view value) ->
-        value_type&
+        element&
 {
     if(sname.size() + 2 >
             (std::numeric_limits<off_t>::max)())
@@ -1155,33 +1166,29 @@ new_element(field name,
         static_cast<off_t>(value.size());
     auto a = rebind_type{this->get()};
     auto const p = alloc_traits::allocate(a,
-        (sizeof(value_type) + off + len + 2 + sizeof(align_type) - 1) /
+        (sizeof(element) + off + len + 2 + sizeof(align_type) - 1) /
             sizeof(align_type));
-    // VFALCO allocator can't call the constructor because its private
+    return *(new(p) element(name, sname, value));
-    //alloc_traits::construct(a, p, name, sname, value);
-    new(p) value_type{name, sname, value};
-    return *reinterpret_cast<value_type*>(p);
 }
 
 template<class Allocator>
 void
 basic_fields<Allocator>::
-delete_element(value_type& e)
+delete_element(element& e)
 {
     auto a = rebind_type{this->get()};
     auto const n =
-        (sizeof(value_type) + e.off_ + e.len_ + 2 + sizeof(align_type) - 1) /
+        (sizeof(element) + e.off_ + e.len_ + 2 + sizeof(align_type) - 1) /
             sizeof(align_type);
-    //alloc_traits::destroy(a, &e);
+    e.~element();
-    e.~value_type();
+    alloc_traits::deallocate(a, &e, n);
-    alloc_traits::deallocate(a,
+        //reinterpret_cast<align_type*>(&e), n);
-        reinterpret_cast<align_type*>(&e), n);
 }
 
 template<class Allocator>
 void
 basic_fields<Allocator>::
-set_element(value_type& e)
+set_element(element& e)
 {
     auto it = set_.lower_bound(
         e.name_string(), key_compare{});