Fix shouldPatchFunction false negative when match is inside template args by pamarcos · Pull Request #101885 · ClickHouse/ClickHouse · GitHub
Skip to content

Fix shouldPatchFunction false negative when match is inside template args#101885

Merged
pamarcos merged 3 commits into
ClickHouse:masterfrom
pamarcos:fix-shouldPatchFunction-false-negative
Apr 8, 2026
Merged

Fix shouldPatchFunction false negative when match is inside template args#101885
pamarcos merged 3 commits into
ClickHouse:masterfrom
pamarcos:fix-shouldPatchFunction-false-negative

Conversation

@pamarcos

@pamarcos pamarcos commented Apr 6, 2026

Copy link
Copy Markdown
Member

shouldPatchFunction used std::string::find which returns only the first match. If that first match was inside template <...> arguments, the function returned false without searching for subsequent occurrences that might be outside template arguments.

The fix loops over all occurrences of the search string instead of stopping at the first one. It also replaces std::stack<bool> with a simple depth counter, which avoids undefined behavior from popping an empty stack on a stray >.

Closes #101808

Changelog category (leave one):

  • Bug Fix (user-visible misbehavior in an official stable release)

Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):

Fix shouldPatchFunction false negative in SYSTEM INSTRUMENT ADD when the search string first appears inside a template argument of the demangled symbol name.

Documentation entry for user-facing changes

  • Documentation is written (mandatory for new features)

Version info

  • Merged into: 26.4.1.653

…args

shouldPatchFunction used std::string::find() which returns only the first
match. If that match was inside template <...> arguments, the function
returned false without searching for subsequent occurrences that might be
outside template arguments.

Loop over all occurrences instead of stopping at the first one. Also
replace std::stack<bool> with a simple depth counter to avoid undefined
behavior from popping an empty stack on malformed >.

Closes ClickHouse#101808
@clickhouse-gh

clickhouse-gh Bot commented Apr 6, 2026

Copy link
Copy Markdown
Contributor

@clickhouse-gh clickhouse-gh Bot added the pr-bugfix Pull request with bugfix, not backported by default label Apr 6, 2026
@alexey-milovidov

Copy link
Copy Markdown
Member

The Stress test (arm_msan) failure is fixed by #101239, which should be merged first. After it is merged, please update the branch to include the fix.

@Algunenano Algunenano self-assigned this Apr 7, 2026
@alexey-milovidov

Copy link
Copy Markdown
Member

The test 02859_replicated_db_name_zookeeper is fixed in #101952

@clickhouse-gh

clickhouse-gh Bot commented Apr 8, 2026

Copy link
Copy Markdown
Contributor

LLVM Coverage Report

Metric Baseline Current Δ
Lines 83.90% 83.90% +0.00%
Functions 90.90% 90.90% +0.00%
Branches 76.40% 76.40% +0.00%

Changed lines: 100.00% (26/26) | lost baseline coverage: 1 line(s) · Uncovered code

Full report · Diff report

@pamarcos pamarcos enabled auto-merge April 8, 2026 10:58
@pamarcos pamarcos added this pull request to the merge queue Apr 8, 2026
Merged via the queue into ClickHouse:master with commit 0e2f4c6 Apr 8, 2026
163 checks passed
@pamarcos pamarcos deleted the fix-shouldPatchFunction-false-negative branch April 8, 2026 13:42
@robot-clickhouse-ci-1 robot-clickhouse-ci-1 added the pr-synced-to-cloud The PR is synced to the cloud repo label Apr 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-bugfix Pull request with bugfix, not backported by default pr-synced-to-cloud The PR is synced to the cloud repo

Projects

None yet

Development

Successfully merging this pull request may close these issues.

shouldPatchFunction false negative: only first find() match checked, misses valid matches after template args

4 participants