[fix](ub) undefined behavior in FixedContainer (#39191) (#41088)

apache · Sep 22, 2024 · bb464c5 · bb464c5
1 parent 3438982
commit bb464c5
Show file tree

Hide file tree

Showing 4 changed files with 61 additions and 1 deletion.
diff --git a/be/src/exprs/hybrid_set.h b/be/src/exprs/hybrid_set.h
@@ -17,7 +17,13 @@
 
 #pragma once
 
+#include <glog/logging.h>
+
+#include <type_traits>
+
+#include "common/exception.h"
 #include "common/object_pool.h"
+#include "common/status.h"
 #include "exprs/runtime_filter.h"
 #include "runtime/decimalv2_value.h"
 #include "runtime/define_primitive_type.h"
@@ -60,8 +66,16 @@ class FixedContainer {
         }
     }
 
+    void check_size() {
+        if (N != _size) {
+            throw doris::Exception(ErrorCode::INTERNAL_ERROR,
+                                   "invalid size of FixedContainer<{}>: {}", N, _size);
+        }
+    }
+
     // Use '|' instead of '||' has better performance by test.
     ALWAYS_INLINE bool find(const T& value) const {
+        DCHECK_EQ(N, _size);
         if constexpr (N == 0) {
             return false;
         }
@@ -144,6 +158,12 @@ class FixedContainer {
     size_t _size;
 };
 
+template <typename T>
+struct IsFixedContainer : std::false_type {};
+
+template <typename T, size_t N>
+struct IsFixedContainer<FixedContainer<T, N>> : std::true_type {};
+
 /**
  * Dynamic Container uses phmap::flat_hash_set.
  * @tparam T Element Type
@@ -351,6 +371,11 @@ class HybridSet : public HybridSetBase {
         if constexpr (is_nullable) {
             null_map_data = null_map->data();
         }
+
+        if constexpr (IsFixedContainer<ContainerType>::value) {
+            _set.check_size();
+        }
+
         auto* __restrict result_data = results.data();
         for (size_t i = 0; i < rows; ++i) {
             if constexpr (!is_nullable && !is_negative) {
@@ -466,6 +491,11 @@ class StringSet : public HybridSetBase {
         if constexpr (is_nullable) {
             null_map_data = null_map->data();
         }
+
+        if constexpr (IsFixedContainer<ContainerType>::value) {
+            _set.check_size();
+        }
+
         auto* __restrict result_data = results.data();
         for (size_t i = 0; i < rows; ++i) {
             const auto& string_data = col.get_data_at(i).to_string();
@@ -596,6 +626,11 @@ class StringValueSet : public HybridSetBase {
         if constexpr (is_nullable) {
             null_map_data = null_map->data();
         }
+
+        if constexpr (IsFixedContainer<ContainerType>::value) {
+            _set.check_size();
+        }
+
         auto* __restrict result_data = results.data();
         for (size_t i = 0; i < rows; ++i) {
             uint32_t len = offset[i] - offset[i - 1];

diff --git a/be/src/vec/functions/in.h b/be/src/vec/functions/in.h
@@ -120,7 +120,7 @@ class FunctionIn : public IFunction {
                    context->get_arg_type(0)->type == PrimitiveType::TYPE_VARCHAR ||
                    context->get_arg_type(0)->type == PrimitiveType::TYPE_STRING) {
             // the StringValue's memory is held by FunctionContext, so we can use StringValueSet here directly
-            state->hybrid_set.reset(create_string_value_set((size_t)(context->get_num_args() - 1)));
+            state->hybrid_set.reset(create_string_value_set(get_size_with_out_null(context)));
         } else {
             state->hybrid_set.reset(
                     create_set(context->get_arg_type(0)->type, get_size_with_out_null(context)));

diff --git a/regression-test/data/nereids_syntax_p0/inpredicate.out b/regression-test/data/nereids_syntax_p0/inpredicate.out
@@ -31,3 +31,12 @@
 29	Supplier#000000029	VVSymB3fbwaN	ARGENTINA4	ARGENTINA	AMERICA	11-773-203-7342
 9	Supplier#000000009	,gJ6K2MKveYxQT	IRAN     6	IRAN	MIDDLE EAST	20-338-906-3675
 
+-- !in_predicate_11 --
+15	Supplier#000000015	DF35PepL5saAK	INDIA    0	INDIA	ASIA	18-687-542-7601
+
+-- !in_predicate_12 --
+
+-- !in_predicate_13 --
+
+-- !in_predicate_14 --
+
diff --git a/regression-test/suites/nereids_syntax_p0/inpredicate.groovy b/regression-test/suites/nereids_syntax_p0/inpredicate.groovy
@@ -61,5 +61,21 @@ suite("inpredicate") {
     order_qt_in_predicate_10 """
         SELECT * FROM supplier WHERE s_suppkey not in (15);
     """
+
+    order_qt_in_predicate_11 """
+        SELECT * FROM supplier WHERE s_suppkey in (15, null);
+    """
+
+    order_qt_in_predicate_12 """
+        SELECT * FROM supplier WHERE s_suppkey not in (15, null);
+    """
+
+    order_qt_in_predicate_13 """
+        SELECT * FROM supplier WHERE s_nation in ('PERU', 'ETHIOPIA', null);
+    """
+
+    order_qt_in_predicate_14 """
+        SELECT * FROM supplier WHERE s_nation not in ('PERU', 'ETHIOPIA', null);
+    """
 }