Skip to content

fix(c-lsp): guard pending template call resolution#322

Merged
DeusData merged 1 commit into
DeusData:mainfrom
romanornr:upstream/c-lsp-template-call-guard
May 30, 2026
Merged

fix(c-lsp): guard pending template call resolution#322
DeusData merged 1 commit into
DeusData:mainfrom
romanornr:upstream/c-lsp-template-call-guard

Conversation

@romanornr

Copy link
Copy Markdown
Contributor

Summary

  • Add defensive bounds checks while resolving pending C++ template calls
  • Avoid reading past signature parameter arrays when recovery-mode parses produce extra call arguments
  • Add a regression test covering mismatched template call argument counts

Test plan

  • make -f Makefile.cbm test -j$(nproc)

Notes

This was found while indexing a large Bitcoin-derived C++ codebase, where malformed/recovery-mode AST shapes could expose unsafe assumptions in the C/C++ LSP resolver.

@DeusData DeusData added bug Something isn't working stability/performance Server crashes, OOM, hangs, high CPU/memory parsing/quality Graph extraction bugs, false positives, missing edges labels May 8, 2026
@DeusData
DeusData merged commit 05f0a26 into DeusData:main May 30, 2026
@DeusData

Copy link
Copy Markdown
Owner

Thank you, @romanornr! 🙏 This is a clean, surgical fix for a real segfault (#215). The root-cause framing was spot-on — the call site genuinely can carry more arguments than the parsed signature knows about (invalid code, macros, variadic calls, or parser recovery), and the old loop walked param_types[i] straight past the NULL sentinel. Clamping to min(call_arg_count, formal_count) is exactly right, and I appreciated the belt-and-suspenders touches: doing the arena strdups before bumping pending_tc_count (so a half-initialized entry can never exist), plus the defensive NULL guards before strcmp. The invoke(w, 1, 2) regression test reproduces the overflow path perfectly.

Merged via squash (05f0a26b) — your authorship is preserved. Verified locally: build clean, all 3,617 tests pass including your new clsp_nocrash_template_extra_call_args. This also helps close out the stability cluster tracked in #390. Thanks again!

DeusData pushed a commit that referenced this pull request May 30, 2026
…t args

Root cause: unbound template parameters flowed through cbm_type_substitute
as NULL, and call-expression return handling then dereferenced the NULL
substituted type. Reproduced on MangoHud's src/hud_elements.cpp.

- type_rep: cbm_type_substitute now returns the original param `t` (not
  NULL) when a type arg is unbound, in both the TYPE_PARAM and named-param
  paths — unbound params stay intact instead of poisoning the type.
- c_lsp: guard `if (!ret) return cbm_type_unknown();` before unwrapping
  the substituted return type; guard the substitute result before use; add
  template_function call-node resolution (qualified/scoped/plain names).
- extract_defs: factor template-inner lookup into find_cpp_template_inner_node,
  which recurses into nested template_declarations and also handles
  field_declaration; reuse it from unwrap_template_inner.
- tests: clsp_nocrash_template_function_multi_param_nested_call (the nested
  reproducer) and typerep_substitute_unbound_param_preserved.

Distilled from #360 onto current main. The formal-parameter-count clamp
that overlapped this PR already landed via #322, so only the additive
crash fixes are taken here. Contributes to the C++ stability cluster (#390;
relates to #215/#312).
DeusData pushed a commit that referenced this pull request May 30, 2026
- cbm_arena_alloc returns NULL on a NULL arena (both arena.c copies)
  instead of dereferencing it — defense-in-depth against the NULL-arena
  type-allocation path that could crash the LSP type layer.
- discover: add "vendored" to the always-skip directory list.

Distilled from #374, taking the parts not already on main. The PR's
ts_lsp tuple-arena allocation, the C/C++ template formal-count clamp,
and the type_rep unbound-param preservation it also carried all landed
independently in v0.7.0 / via #322 / #360, so only these two
defensive improvements remain. Relates to #390.
DeusData added a commit that referenced this pull request May 30, 2026
Issue #355 reported a segfault extracting a macro-heavy xxhash C header
(dhw/xx_hash.h, not available to us). Add a guard that runs the vendored
xxhash.h (~7.5k lines, same macro-dense family) through C extraction under
ASan. It passes on current main — the C-LSP crash hardening (#322/#323/
#360) appears to cover this class. Kept as a regression guard; #355 stays
open pending the reporter's confirmation/file.
zaruous pushed a commit to zaruous/codebase-memory-mcp that referenced this pull request May 31, 2026
- cbm_arena_alloc returns NULL on a NULL arena (both arena.c copies)
  instead of dereferencing it — defense-in-depth against the NULL-arena
  type-allocation path that could crash the LSP type layer.
- discover: add "vendored" to the always-skip directory list.

Distilled from DeusData#374, taking the parts not already on main. The PR's
ts_lsp tuple-arena allocation, the C/C++ template formal-count clamp,
and the type_rep unbound-param preservation it also carried all landed
independently in v0.7.0 / via DeusData#322 / DeusData#360, so only these two
defensive improvements remain. Relates to DeusData#390.
zaruous pushed a commit to zaruous/codebase-memory-mcp that referenced this pull request May 31, 2026
Fixes the OOB read in c_resolve_pending_template_calls when a call site has more arguments than the parsed function signature (issue DeusData#215). Verified locally: build clean, all 3,617 tests pass including the new clsp_nocrash_template_extra_call_args regression.
zaruous pushed a commit to zaruous/codebase-memory-mcp that referenced this pull request May 31, 2026
…t args

Root cause: unbound template parameters flowed through cbm_type_substitute
as NULL, and call-expression return handling then dereferenced the NULL
substituted type. Reproduced on MangoHud's src/hud_elements.cpp.

- type_rep: cbm_type_substitute now returns the original param `t` (not
  NULL) when a type arg is unbound, in both the TYPE_PARAM and named-param
  paths — unbound params stay intact instead of poisoning the type.
- c_lsp: guard `if (!ret) return cbm_type_unknown();` before unwrapping
  the substituted return type; guard the substitute result before use; add
  template_function call-node resolution (qualified/scoped/plain names).
- extract_defs: factor template-inner lookup into find_cpp_template_inner_node,
  which recurses into nested template_declarations and also handles
  field_declaration; reuse it from unwrap_template_inner.
- tests: clsp_nocrash_template_function_multi_param_nested_call (the nested
  reproducer) and typerep_substitute_unbound_param_preserved.

Distilled from DeusData#360 onto current main. The formal-parameter-count clamp
that overlapped this PR already landed via DeusData#322, so only the additive
crash fixes are taken here. Contributes to the C++ stability cluster (DeusData#390;
relates to DeusData#215/DeusData#312).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working parsing/quality Graph extraction bugs, false positives, missing edges stability/performance Server crashes, OOM, hangs, high CPU/memory

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants