From 8d261e6813806d4f1b98b94a947dce0d9072f82b Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Fri, 9 Jan 2026 04:10:10 +0100 Subject: [PATCH 1/3] Merge pull request #93663 from ClickHouse/fix-msan-tokeniterator Fix trash in `TokenExtractor` Conflicts committed as-is; the next commit resolves them: - src/Interpreters/ITokenExtractor.cpp (both modified) --- src/Interpreters/ITokenExtractor.cpp | 104 +++++++++++++++++- .../03784_msan_token_iterator.reference | 0 .../0_stateless/03784_msan_token_iterator.sql | 4 + 3 files changed, 107 insertions(+), 1 deletion(-) create mode 100644 tests/queries/0_stateless/03784_msan_token_iterator.reference create mode 100644 tests/queries/0_stateless/03784_msan_token_iterator.sql diff --git a/src/Interpreters/ITokenExtractor.cpp b/src/Interpreters/ITokenExtractor.cpp index fc77a7507cbcb..c3039963d5a31 100644 --- a/src/Interpreters/ITokenExtractor.cpp +++ b/src/Interpreters/ITokenExtractor.cpp @@ -55,7 +55,13 @@ bool NgramTokenExtractor::nextInString(const char * data, size_t length, size_t size_t sz = UTF8::seqLength(static_cast(data[*token_start + *token_length])); *token_length += sz; } +<<<<<<< HEAD *pos += UTF8::seqLength(static_cast(data[*pos])); +======= + /// The sequence length is determined by the first character, but we should always check it does not go beyond the buffer length. + token_length = std::min(token_length, length - token_start); + pos += UTF8::seqLength(static_cast(data[pos])); +>>>>>>> f9fc3b2322d (Merge pull request #93663 from ClickHouse/fix-msan-tokeniterator) return code_points == n; } @@ -80,7 +86,12 @@ bool NgramTokenExtractor::nextInStringLike(const char * data, size_t length, siz token.clear(); code_points = 0; escaped = false; +<<<<<<< HEAD *pos = ++i; +======= + ++i; + pos = i; +>>>>>>> f9fc3b2322d (Merge pull request #93663 from ClickHouse/fix-msan-tokeniterator) } else if (!escaped && data[i] == '\\') { @@ -90,7 +101,8 @@ bool NgramTokenExtractor::nextInStringLike(const char * data, size_t length, siz else { const size_t sz = UTF8::seqLength(static_cast(data[i])); - for (size_t j = 0; j < sz; ++j) + /// The sequence length is determined by the first character, but we should always check it does not go beyond the buffer length. + for (size_t j = 0; j < sz && i + j < length; ++j) token += data[i + j]; i += sz; ++code_points; @@ -253,8 +265,14 @@ bool DefaultTokenExtractor::nextInStringLike(const char * data, size_t length, s } else { +<<<<<<< HEAD const size_t sz = UTF8::seqLength(static_cast(data[*pos])); for (size_t j = 0; j < sz; ++j) +======= + const size_t sz = UTF8::seqLength(static_cast(data[pos])); + /// The sequence length is determined by the first character, but we should always check it does not go beyond the buffer length. + for (size_t j = 0; j < sz && pos < length; ++j) +>>>>>>> f9fc3b2322d (Merge pull request #93663 from ClickHouse/fix-msan-tokeniterator) { token += data[*pos]; ++*pos; @@ -374,7 +392,91 @@ bool NoOpTokenExtractor::nextInString(const char * /*data*/, size_t length, size bool NoOpTokenExtractor::nextInStringLike(const char * /*data*/, size_t /*length*/, size_t * /*token_start*/, String & /*token_length*/) const { +<<<<<<< HEAD throw Exception(ErrorCodes::NOT_IMPLEMENTED, "NoOpTokenExtractor::nextInStringLike is not implemented"); +======= + throw Exception(ErrorCodes::NOT_IMPLEMENTED, "ArrayTokenExtractor::nextInStringLike is not implemented"); +} + +SparseGramsTokenExtractor::SparseGramsTokenExtractor(size_t min_length, size_t max_length, std::optional min_cutoff_length_) + : ITokenExtractorHelper(Type::SparseGrams) + , sparse_grams_iterator(min_length, max_length, min_cutoff_length_) +{ +} + +bool SparseGramsTokenExtractor::nextInString(const char * data, size_t length, size_t & __restrict pos, size_t & __restrict token_start, size_t & __restrict token_length) const +{ + if (std::tie(data, length) != std::tie(previous_data, previous_len)) + { + previous_data = data; + previous_len = length; + sparse_grams_iterator.set(data, data + length); + } + + Pos next_begin; + Pos next_end; + if (!sparse_grams_iterator.get(next_begin, next_end)) + { + previous_data = nullptr; + previous_len = 0; + return false; + } + token_start = next_begin - data; + token_length = next_end - next_begin; + pos = token_start; + + return true; +} + +bool SparseGramsTokenExtractor::nextInStringLike(const char * data, size_t length, size_t & pos, String & token) const +{ + if (std::tie(data, length) != std::tie(previous_data, previous_len)) + { + previous_data = data; + previous_len = length; + sparse_grams_iterator.set(data, data + length); + } + + token.clear(); + + while (true) + { + Pos next_begin; + Pos next_end; + if (!sparse_grams_iterator.get(next_begin, next_end)) + { + previous_data = nullptr; + previous_len = 0; + return false; + } + bool match_substring = true; + for (size_t i = next_begin - data; i < static_cast(next_end - data);) + { + /// Escaped sequences are not supported by sparse grams tokenizers. + /// It requires pushing down this logic into sparse grams implementation, + /// because it changes the split into sparse grams. We don't want to do that now. + if (data[i] == '%' || data[i] == '_' || data[i] == '\\') + { + token.clear(); + match_substring = false; + break; + } + else + { + const size_t sz = UTF8::seqLength(static_cast(data[i])); + /// The sequence length is determined by the first character, but we should always check it does not go beyond the buffer length. + for (size_t j = 0; j < sz && i + j < length; ++j) + token.push_back(data[i + j]); + i += sz; + } + } + if (match_substring) + { + pos = next_begin - data; + return true; + } + } +>>>>>>> f9fc3b2322d (Merge pull request #93663 from ClickHouse/fix-msan-tokeniterator) } } diff --git a/tests/queries/0_stateless/03784_msan_token_iterator.reference b/tests/queries/0_stateless/03784_msan_token_iterator.reference new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/tests/queries/0_stateless/03784_msan_token_iterator.sql b/tests/queries/0_stateless/03784_msan_token_iterator.sql new file mode 100644 index 0000000000000..2ca62efb42beb --- /dev/null +++ b/tests/queries/0_stateless/03784_msan_token_iterator.sql @@ -0,0 +1,4 @@ +DROP TABLE IF EXISTS t0; +CREATE TABLE t0 (c0 Int, INDEX i1 substring(sipHash128(c0), 1, 4) TYPE ngrambf_v1(1, 8, 1, 0)); +INSERT INTO TABLE t0 (c0) SELECT number FROM numbers(28); +DROP TABLE t0; From 60fc76c73aea1fe8a1aad5512716f021b81e8e13 Mon Sep 17 00:00:00 2001 From: Andrey Zvonov <32552679+zvonand@users.noreply.github.com> Date: Sun, 25 Oct 2026 15:25:19 +0100 Subject: [PATCH 2/3] Resolve conflicts in backport of #93663 Applied the buffer-length checks in stable-25.8's pointer style (`*pos`, `*token_length`) and dropped the `SparseGramsTokenExtractor` code, which doesn't exist on this branch. --- src/Interpreters/ITokenExtractor.cpp | 105 +-------------------------- 1 file changed, 4 insertions(+), 101 deletions(-) diff --git a/src/Interpreters/ITokenExtractor.cpp b/src/Interpreters/ITokenExtractor.cpp index c3039963d5a31..a83b0858a988c 100644 --- a/src/Interpreters/ITokenExtractor.cpp +++ b/src/Interpreters/ITokenExtractor.cpp @@ -55,13 +55,9 @@ bool NgramTokenExtractor::nextInString(const char * data, size_t length, size_t size_t sz = UTF8::seqLength(static_cast(data[*token_start + *token_length])); *token_length += sz; } -<<<<<<< HEAD - *pos += UTF8::seqLength(static_cast(data[*pos])); -======= /// The sequence length is determined by the first character, but we should always check it does not go beyond the buffer length. - token_length = std::min(token_length, length - token_start); - pos += UTF8::seqLength(static_cast(data[pos])); ->>>>>>> f9fc3b2322d (Merge pull request #93663 from ClickHouse/fix-msan-tokeniterator) + *token_length = std::min(*token_length, length - *token_start); + *pos += UTF8::seqLength(static_cast(data[*pos])); return code_points == n; } @@ -86,12 +82,8 @@ bool NgramTokenExtractor::nextInStringLike(const char * data, size_t length, siz token.clear(); code_points = 0; escaped = false; -<<<<<<< HEAD - *pos = ++i; -======= ++i; - pos = i; ->>>>>>> f9fc3b2322d (Merge pull request #93663 from ClickHouse/fix-msan-tokeniterator) + *pos = i; } else if (!escaped && data[i] == '\\') { @@ -265,14 +257,9 @@ bool DefaultTokenExtractor::nextInStringLike(const char * data, size_t length, s } else { -<<<<<<< HEAD const size_t sz = UTF8::seqLength(static_cast(data[*pos])); - for (size_t j = 0; j < sz; ++j) -======= - const size_t sz = UTF8::seqLength(static_cast(data[pos])); /// The sequence length is determined by the first character, but we should always check it does not go beyond the buffer length. - for (size_t j = 0; j < sz && pos < length; ++j) ->>>>>>> f9fc3b2322d (Merge pull request #93663 from ClickHouse/fix-msan-tokeniterator) + for (size_t j = 0; j < sz && *pos < length; ++j) { token += data[*pos]; ++*pos; @@ -392,91 +379,7 @@ bool NoOpTokenExtractor::nextInString(const char * /*data*/, size_t length, size bool NoOpTokenExtractor::nextInStringLike(const char * /*data*/, size_t /*length*/, size_t * /*token_start*/, String & /*token_length*/) const { -<<<<<<< HEAD throw Exception(ErrorCodes::NOT_IMPLEMENTED, "NoOpTokenExtractor::nextInStringLike is not implemented"); -======= - throw Exception(ErrorCodes::NOT_IMPLEMENTED, "ArrayTokenExtractor::nextInStringLike is not implemented"); -} - -SparseGramsTokenExtractor::SparseGramsTokenExtractor(size_t min_length, size_t max_length, std::optional min_cutoff_length_) - : ITokenExtractorHelper(Type::SparseGrams) - , sparse_grams_iterator(min_length, max_length, min_cutoff_length_) -{ -} - -bool SparseGramsTokenExtractor::nextInString(const char * data, size_t length, size_t & __restrict pos, size_t & __restrict token_start, size_t & __restrict token_length) const -{ - if (std::tie(data, length) != std::tie(previous_data, previous_len)) - { - previous_data = data; - previous_len = length; - sparse_grams_iterator.set(data, data + length); - } - - Pos next_begin; - Pos next_end; - if (!sparse_grams_iterator.get(next_begin, next_end)) - { - previous_data = nullptr; - previous_len = 0; - return false; - } - token_start = next_begin - data; - token_length = next_end - next_begin; - pos = token_start; - - return true; -} - -bool SparseGramsTokenExtractor::nextInStringLike(const char * data, size_t length, size_t & pos, String & token) const -{ - if (std::tie(data, length) != std::tie(previous_data, previous_len)) - { - previous_data = data; - previous_len = length; - sparse_grams_iterator.set(data, data + length); - } - - token.clear(); - - while (true) - { - Pos next_begin; - Pos next_end; - if (!sparse_grams_iterator.get(next_begin, next_end)) - { - previous_data = nullptr; - previous_len = 0; - return false; - } - bool match_substring = true; - for (size_t i = next_begin - data; i < static_cast(next_end - data);) - { - /// Escaped sequences are not supported by sparse grams tokenizers. - /// It requires pushing down this logic into sparse grams implementation, - /// because it changes the split into sparse grams. We don't want to do that now. - if (data[i] == '%' || data[i] == '_' || data[i] == '\\') - { - token.clear(); - match_substring = false; - break; - } - else - { - const size_t sz = UTF8::seqLength(static_cast(data[i])); - /// The sequence length is determined by the first character, but we should always check it does not go beyond the buffer length. - for (size_t j = 0; j < sz && i + j < length; ++j) - token.push_back(data[i + j]); - i += sz; - } - } - if (match_substring) - { - pos = next_begin - data; - return true; - } - } ->>>>>>> f9fc3b2322d (Merge pull request #93663 from ClickHouse/fix-msan-tokeniterator) } } From d61cddac2542742227c411d9457219db592a31db Mon Sep 17 00:00:00 2001 From: Andrey Zvonov <32552679+zvonand@users.noreply.github.com> Date: Tue, 27 Oct 2026 10:49:53 +0100 Subject: [PATCH 3/3] Fix CI failures (Fast test) Added explicit `ENGINE = MergeTree ORDER BY ()` to the table in 03784_msan_token_iterator, since stable-25.8 requires an ORDER BY / PRIMARY KEY by default --- tests/queries/0_stateless/03784_msan_token_iterator.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/queries/0_stateless/03784_msan_token_iterator.sql b/tests/queries/0_stateless/03784_msan_token_iterator.sql index 2ca62efb42beb..925abbf847a8b 100644 --- a/tests/queries/0_stateless/03784_msan_token_iterator.sql +++ b/tests/queries/0_stateless/03784_msan_token_iterator.sql @@ -1,4 +1,4 @@ DROP TABLE IF EXISTS t0; -CREATE TABLE t0 (c0 Int, INDEX i1 substring(sipHash128(c0), 1, 4) TYPE ngrambf_v1(1, 8, 1, 0)); +CREATE TABLE t0 (c0 Int, INDEX i1 substring(sipHash128(c0), 1, 4) TYPE ngrambf_v1(1, 8, 1, 0)) ENGINE = MergeTree ORDER BY (); INSERT INTO TABLE t0 (c0) SELECT number FROM numbers(28); DROP TABLE t0;