From e548d971b3e471ca5a149201c39ba62ab1a72261 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Tue, 29 Sep 2026 00:54:37 +0200 Subject: [PATCH] fix(mcp): free the empty name-lookup result before trace_path's QN fallback handle_trace_call_path first looks the function up by bare name (cbm_store_find_nodes_by_name). find_nodes_generic allocates its initial 16-slot array before stepping the statement and hands it back even when zero rows match. When the bare name misses and the qualified_name fallback then resolves the node, the handler overwrote `nodes` with a fresh one-element array, leaking the empty 1 KB array on every trace_path call made with a qualified name - for the whole lifetime of the daemon. The malloc-failure branch of the fallback lost the same array. Release the zero-row array before the fallback replaces the pointer. The other find_nodes_* callers (get_code_snippet outline, search_code grep classification, detect_changes seeding, Cypher label scans) already free the container on zero rows; the not-found path in this handler did too. Proof (macOS leak lane, Homebrew clang 22.1.8, `make -f Makefile.cbm test-lsan LSAN_SUITES=mcp`): - with the new test, without the fix: 322 passed, then "ERROR: LeakSanitizer: detected memory leaks - Direct leak of 1024 byte(s) in 1 object(s) allocated from find_nodes_generic store.c:2771 <- handle_trace_call_path mcp.c:9176 <- test_tool_trace_call_path_qn_fallback_frees_name_miss", exit 1 (the only leak in the suite). - with the fix: 322 passed, 4 skipped, no leak report, exit 0. The new test pins its own precondition (the bare-name lookup of the qualified name returns zero rows) and that the trace resolves through the fallback, so it keeps exercising this path. LSan (default-on under Linux ASan, test-lsan on macOS) turns any regression red. Found while attributing CI on #2305. Signed-off-by: Martin Vogel --- src/mcp/mcp.c | 3 +++ tests/test_mcp.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+) diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index 78b8dccf96..7dd4a92977 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -9178,6 +9178,9 @@ static char *handle_trace_call_path(cbm_mcp_server_t *srv, const char *args) { if (node_count == 0) { cbm_node_t qn_node = {0}; if (cbm_store_find_node_by_qn(store, project, func_name, &qn_node) == CBM_STORE_OK) { + /* A zero-row name lookup still returns its allocated (empty) + * array; release it before the fallback replaces the pointer. */ + cbm_store_free_nodes(nodes, 0); nodes = malloc(sizeof(cbm_node_t)); if (nodes) { nodes[0] = qn_node; diff --git a/tests/test_mcp.c b/tests/test_mcp.c index 60649f5054..f8c4bb56a0 100644 --- a/tests/test_mcp.c +++ b/tests/test_mcp.c @@ -7187,6 +7187,67 @@ TEST(tool_trace_call_path_prefers_definition) { PASS(); } +/* Leak regression: trace_path by a qualified name resolves only through the + * qualified_name fallback. The bare-name lookup that precedes it returns a + * heap array even for zero rows (find_nodes_generic allocates its initial + * capacity up front), and the fallback used to overwrite that pointer — + * leaking ~1 KB per call for the daemon's lifetime. The sanitizer lanes + * (LSan: default-on under Linux ASan, `make -f Makefile.cbm test-lsan` on + * macOS) report the leak at exit; the assertions pin that this test really + * takes the fallback path. */ +TEST(tool_trace_call_path_qn_fallback_frees_name_miss) { + cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL); + cbm_store_t *st = cbm_mcp_server_store(srv); + const char *proj = "qnfb-proj"; + const char *qn = "qnfb-proj.src.qnfb_entry"; + cbm_mcp_server_set_project(srv, proj); + cbm_store_upsert_project(st, proj, "/tmp/qnfb"); + cbm_node_t entry = {.project = proj, + .label = "Function", + .name = "qnfb_entry", + .qualified_name = qn, + .file_path = "src/qnfb.c", + .start_line = 1, + .end_line = 10}; + cbm_node_t callee = {.project = proj, + .label = "Function", + .name = "qnfb_callee", + .qualified_name = "qnfb-proj.src.qnfb_callee", + .file_path = "src/qnfb.c", + .start_line = 20, + .end_line = 30}; + int64_t id_entry = cbm_store_upsert_node(st, &entry); + int64_t id_callee = cbm_store_upsert_node(st, &callee); + ASSERT_GT(id_entry, 0); + ASSERT_GT(id_callee, 0); + cbm_edge_t e = { + .project = proj, .source_id = id_entry, .target_id = id_callee, .type = "CALLS"}; + cbm_store_insert_edge(st, &e); + + /* Precondition: the qualified name is NOT a bare name, so the handler's + * first lookup misses and the fallback is the only way to resolve it. */ + cbm_node_t *by_name = NULL; + int by_name_count = -1; + ASSERT_EQ(cbm_store_find_nodes_by_name(st, proj, qn, &by_name, &by_name_count), CBM_STORE_OK); + ASSERT_EQ(by_name_count, 0); + cbm_store_free_nodes(by_name, by_name_count); + + char *resp = cbm_mcp_server_handle( + srv, "{\"jsonrpc\":\"2.0\",\"id\":63,\"method\":\"tools/call\"," + "\"params\":{\"name\":\"trace_call_path\",\"arguments\":{\"function_name\":" + "\"qnfb-proj.src.qnfb_entry\",\"project\":\"qnfb-proj\",\"direction\":\"outbound\"}}}"); + ASSERT_NOT_NULL(resp); + char *inner = extract_text_content(resp); + ASSERT_NOT_NULL(inner); + ASSERT_NULL(strstr(inner, "function not found")); + /* resolved through the fallback -> its outbound CALLS edge shows */ + ASSERT_NOT_NULL(strstr(inner, "qnfb_callee")); + free(inner); + free(resp); + cbm_mcp_server_free(srv); + PASS(); +} + /* CONTRACT PIN for the closed strategy vocabulary published by * trace_path(include_evidence:true). * @@ -20638,6 +20699,7 @@ SUITE(mcp) { RUN_TEST(tool_trace_reports_engine_saturation_as_lower_bound); RUN_TEST(store_bfs_edge_data_is_skippable_and_bounded); RUN_TEST(tool_trace_call_path_prefers_definition); + RUN_TEST(tool_trace_call_path_qn_fallback_frees_name_miss); RUN_TEST(trace_evidence_strategy_class_vocabulary_is_closed); RUN_TEST(tool_trace_path_evidence_is_opt_in_and_class_mapped); RUN_TEST(tool_trace_path_evidence_columns_match_header_issue1542);