BugFix: SIGABRT in quantiles_sketch::deserialize(): dereferencing empty std::optional (libc++ verbose_abort)

Author: MaheshGPai

kll/include/kll_sketch_impl.hpp

@@ -24,6 +24,7 @@
 #include <iomanip>
 #include <sstream>
 #include <stdexcept>
+#include <type_traits>
 
 #include "conditional_forward.hpp"
 #include "count_zeros.hpp"
@@ -481,18 +482,22 @@ kll_sketch<T, C, A> kll_sketch<T, C, A>::deserialize(std::istream& is, const Ser
     read(is, levels.data(), sizeof(levels[0]) * num_levels);
   }
   levels[num_levels] = capacity;
-  optional<T> tmp; // space to deserialize min and max
   optional<T> min_item;
   optional<T> max_item;
   if (!is_single_item) {
-    sd.deserialize(is, &*tmp, 1);
+    // Space to deserialize min and max.
+    // serde::deserialize expects allocated but not initialized storage.
+    typename std::aligned_storage<sizeof(T), alignof(T)>::type tmp_storage;
+    T* tmp = reinterpret_cast<T*>(&tmp_storage);
+
+    sd.deserialize(is, tmp, 1);
     // serde call did not throw, repackage and cleanup
-    min_item.emplace(*tmp);
-    (*tmp).~T();
-    sd.deserialize(is, &*tmp, 1);
+    min_item.emplace(std::move(*tmp));
+    tmp->~T();
+    sd.deserialize(is, tmp, 1);
     // serde call did not throw, repackage and cleanup
-    max_item.emplace(*tmp);
-    (*tmp).~T();
+    max_item.emplace(std::move(*tmp));
+    tmp->~T();
   }
   A alloc(allocator);
   auto items_buffer_deleter = [capacity, &alloc](T* ptr) { alloc.deallocate(ptr, capacity); };
@@ -565,18 +570,22 @@ kll_sketch<T, C, A> kll_sketch<T, C, A>::deserialize(const void* bytes, size_t s
     ptr += copy_from_mem(ptr, levels.data(), sizeof(levels[0]) * num_levels);
   }
   levels[num_levels] = capacity;
-  optional<T> tmp; // space to deserialize min and max
   optional<T> min_item;
   optional<T> max_item;
   if (!is_single_item) {
-    ptr += sd.deserialize(ptr, end_ptr - ptr, &*tmp, 1);
+    // Space to deserialize min and max.
+    // serde::deserialize expects allocated but not initialized storage.
+    typename std::aligned_storage<sizeof(T), alignof(T)>::type tmp_storage;
+    T* tmp = reinterpret_cast<T*>(&tmp_storage);
+
+    ptr += sd.deserialize(ptr, end_ptr - ptr, tmp, 1);
     // serde call did not throw, repackage and cleanup
-    min_item.emplace(*tmp);
-    (*tmp).~T();
-    ptr += sd.deserialize(ptr, end_ptr - ptr, &*tmp, 1);
+    min_item.emplace(std::move(*tmp));
+    tmp->~T();
+    ptr += sd.deserialize(ptr, end_ptr - ptr, tmp, 1);
     // serde call did not throw, repackage and cleanup
-    max_item.emplace(*tmp);
-    (*tmp).~T();
+    max_item.emplace(std::move(*tmp));
+    tmp->~T();
   }
   A alloc(allocator);
   auto items_buffer_deleter = [capacity, &alloc](T* ptr) { alloc.deallocate(ptr, capacity); };

quantiles/include/quantiles_sketch_impl.hpp

@@ -25,6 +25,7 @@
 #include <stdexcept>
 #include <iomanip>
 #include <sstream>
+#include <type_traits>
 
 #include "count_zeros.hpp"
 #include "conditional_forward.hpp"
@@ -393,18 +394,22 @@ auto quantiles_sketch<T, C, A>::deserialize(std::istream &is, const SerDe& serde
   const bool is_compact = (serial_version == 2) | ((flags_byte & (1 << flags::IS_COMPACT)) > 0);
   const bool is_sorted = (flags_byte & (1 << flags::IS_SORTED)) > 0;
 
-  optional<T> tmp; // space to deserialize min and max
   optional<T> min_item;
   optional<T> max_item;
 
-  serde.deserialize(is, &*tmp, 1);
+  // Space to deserialize min and max.
+  // serde::deserialize expects allocated but not initialized storage.
+  typename std::aligned_storage<sizeof(T), alignof(T)>::type tmp_storage;
+  T* tmp = reinterpret_cast<T*>(&tmp_storage);
+
+  serde.deserialize(is, tmp, 1);
   // serde call did not throw, repackage and cleanup
-  min_item.emplace(*tmp);
-  (*tmp).~T();
-  serde.deserialize(is, &*tmp, 1);
+  min_item.emplace(std::move(*tmp));
+  tmp->~T();
+  serde.deserialize(is, tmp, 1);
   // serde call did not throw, repackage and cleanup
-  max_item.emplace(*tmp);
-  (*tmp).~T();
+  max_item.emplace(std::move(*tmp));
+  tmp->~T();
 
   if (serial_version == 1) {
     read<uint64_t>(is); // no longer used
@@ -507,18 +512,22 @@ auto quantiles_sketch<T, C, A>::deserialize(const void* bytes, size_t size, cons
   const bool is_compact = (serial_version == 2) | ((flags_byte & (1 << flags::IS_COMPACT)) > 0);
   const bool is_sorted = (flags_byte & (1 << flags::IS_SORTED)) > 0;
 
-  optional<T> tmp; // space to deserialize min and max
   optional<T> min_item;
   optional<T> max_item;
 
-  ptr += serde.deserialize(ptr, end_ptr - ptr, &*tmp, 1);
+  // Space to deserialize min and max.
+  // serde::deserialize expects allocated but not initialized storage.
+  typename std::aligned_storage<sizeof(T), alignof(T)>::type tmp_storage;
+  T* tmp = reinterpret_cast<T*>(&tmp_storage);
+
+  ptr += serde.deserialize(ptr, end_ptr - ptr, tmp, 1);
   // serde call did not throw, repackage and cleanup
-  min_item.emplace(*tmp);
-  (*tmp).~T();
-  ptr += serde.deserialize(ptr, end_ptr - ptr, &*tmp, 1);
+  min_item.emplace(std::move(*tmp));
+  tmp->~T();
+  ptr += serde.deserialize(ptr, end_ptr - ptr, tmp, 1);
   // serde call did not throw, repackage and cleanup
-  max_item.emplace(*tmp);
-  (*tmp).~T();
+  max_item.emplace(std::move(*tmp));
+  tmp->~T();
 
   if (serial_version == 1) {
     uint64_t unused_long;

req/include/req_sketch_impl.hpp

@@ -22,6 +22,7 @@
 
 #include <sstream>
 #include <stdexcept>
+#include <type_traits>
 
 namespace datasketches {
 
@@ -461,7 +462,6 @@ req_sketch<T, C, A> req_sketch<T, C, A>::deserialize(std::istream& is, const Ser
   const bool hra = flags_byte & (1 << flags::IS_HIGH_RANK);
   if (is_empty) return req_sketch(k, hra, comparator, allocator);
 
-  optional<T> tmp; // space to deserialize min and max
   optional<T> min_item;
   optional<T> max_item;
 
@@ -472,14 +472,19 @@ req_sketch<T, C, A> req_sketch<T, C, A>::deserialize(std::istream& is, const Ser
   uint64_t n = 1;
   if (num_levels > 1) {
     n = read<uint64_t>(is);
-    sd.deserialize(is, &*tmp, 1);
+    // Space to deserialize min and max.
+    // serde::deserialize expects allocated but not initialized storage.
+    typename std::aligned_storage<sizeof(T), alignof(T)>::type tmp_storage;
+    T* tmp = reinterpret_cast<T*>(&tmp_storage);
+
+    sd.deserialize(is, tmp, 1);
     // serde call did not throw, repackage and cleanup
-    min_item.emplace(*tmp);
-    (*tmp).~T();
-    sd.deserialize(is, &*tmp, 1);
+    min_item.emplace(std::move(*tmp));
+    tmp->~T();
+    sd.deserialize(is, tmp, 1);
     // serde call did not throw, repackage and cleanup
-    max_item.emplace(*tmp);
-    (*tmp).~T();
+    max_item.emplace(std::move(*tmp));
+    tmp->~T();
   }
 
   if (raw_items) {
@@ -537,7 +542,6 @@ req_sketch<T, C, A> req_sketch<T, C, A>::deserialize(const void* bytes, size_t s
   const bool hra = flags_byte & (1 << flags::IS_HIGH_RANK);
   if (is_empty) return req_sketch(k, hra, comparator, allocator);
 
-  optional<T> tmp; // space to deserialize min and max
   optional<T> min_item;
   optional<T> max_item;
 
@@ -549,14 +553,19 @@ req_sketch<T, C, A> req_sketch<T, C, A>::deserialize(const void* bytes, size_t s
   if (num_levels > 1) {
     ensure_minimum_memory(end_ptr - ptr, sizeof(n));
     ptr += copy_from_mem(ptr, n);
-    ptr += sd.deserialize(ptr, end_ptr - ptr, &*tmp, 1);
+    // Space to deserialize min and max.
+    // serde::deserialize expects allocated but not initialized storage.
+    typename std::aligned_storage<sizeof(T), alignof(T)>::type tmp_storage;
+    T* tmp = reinterpret_cast<T*>(&tmp_storage);
+
+    ptr += sd.deserialize(ptr, end_ptr - ptr, tmp, 1);
     // serde call did not throw, repackage and cleanup
-    min_item.emplace(*tmp);
-    (*tmp).~T();
-    ptr += sd.deserialize(ptr, end_ptr - ptr, &*tmp, 1);
+    min_item.emplace(std::move(*tmp));
+    tmp->~T();
+    ptr += sd.deserialize(ptr, end_ptr - ptr, tmp, 1);
     // serde call did not throw, repackage and cleanup
-    max_item.emplace(*tmp);
-    (*tmp).~T();
+    max_item.emplace(std::move(*tmp));
+    tmp->~T();
   }
 
   if (raw_items) {

sampling/include/ebpps_sample_impl.hpp

@@ -28,6 +28,7 @@
 #include <cmath>
 #include <string>
 #include <sstream>
+#include <type_traits>
 
 namespace datasketches {
 
@@ -365,11 +366,15 @@ std::pair<ebpps_sample<T, A>, size_t> ebpps_sample<T, A>::deserialize(const uint
 
   optional<T> partial_item;
   if (has_partial) {
-    optional<T> tmp; // space to deserialize
-    ptr += sd.deserialize(ptr, end_ptr - ptr, &*tmp, 1);
+    // Space to deserialize.
+    // serde::deserialize expects allocated but not initialized storage.
+    typename std::aligned_storage<sizeof(T), alignof(T)>::type tmp_storage;
+    T* tmp = reinterpret_cast<T*>(&tmp_storage);
+
+    ptr += sd.deserialize(ptr, end_ptr - ptr, tmp, 1);
     // serde did not throw so place item and clean up
-    partial_item.emplace(*tmp);
-    (*tmp).~T();
+    partial_item.emplace(std::move(*tmp));
+    tmp->~T();
   }
 
   return std::pair<ebpps_sample<T,A>, size_t>(
@@ -400,11 +405,15 @@ ebpps_sample<T, A> ebpps_sample<T, A>::deserialize(std::istream& is, const SerDe
 
   optional<T> partial_item;
   if (has_partial) {
-    optional<T> tmp; // space to deserialize
-    sd.deserialize(is, &*tmp, 1);
+    // Space to deserialize.
+    // serde::deserialize expects allocated but not initialized storage.
+    typename std::aligned_storage<sizeof(T), alignof(T)>::type tmp_storage;
+    T* tmp = reinterpret_cast<T*>(&tmp_storage);
+
+    sd.deserialize(is, tmp, 1);
     // serde did not throw so place item and clean up
-    partial_item.emplace(*tmp);
-    (*tmp).~T();
+    partial_item.emplace(std::move(*tmp));
+    tmp->~T();
   }
 
   if (!is.good()) throw std::runtime_error("error reading from std::istream");