mirror of
https://github.com/DeusData/codebase-memory-mcp.git
synced 2026-11-04 10:48:20 +00:00
fix(resolver): what outlives a spilled result is copied out of it
Over the memory budget each result is parked on disk and read back only for a moment. The Java, Kotlin and C# declaration index kept qualified names from the freed result, and a resolve worker read them. TypeScript's merged label did the same. Both are found by ASan with CBM_MEM_SPILL=1; every resolver suite now runs clean that way. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
This commit is contained in:
1 parent
91880fbcbf
commit
415d9af266
4 files changed
+92
-11
No files matched your search
@@ -211,22 +211,38 @@ static char oo_def_class(const char *label) {
|
||||
return cbm_label_is_type_like(label) ? 'T' : 'F';
|
||||
}
|
||||
|
||||
/* A spilled result goes back to the store as soon as its file is harvested, so a qualified
|
||||
* name the index keeps is copied into the index. A definition found here is valid only
|
||||
* during the harvest. */
|
||||
bool oo_defs_init(oo_defs_t *defs, CBMArena *arena, const CBMFileResult *file) {
|
||||
defs->arena = arena;
|
||||
cbm_arena_init(&defs->scratch);
|
||||
defs->by_line = cbm_ht_create((uint32_t)file->defs.count + 8);
|
||||
if (!defs->by_line)
|
||||
if (!defs->by_line) {
|
||||
oo_defs_free(defs);
|
||||
return false;
|
||||
}
|
||||
for (int i = 0; i < file->defs.count; i++) {
|
||||
const CBMDefinition *def = &file->defs.items[i];
|
||||
if (!def->name || !def->qualified_name)
|
||||
continue;
|
||||
const char *key = cbm_arena_sprintf(arena, "%c%u:%s", oo_def_class(def->label),
|
||||
const char *key = cbm_arena_sprintf(&defs->scratch, "%c%u:%s", oo_def_class(def->label),
|
||||
def->start_line, def->name);
|
||||
if (!key)
|
||||
if (!key) {
|
||||
oo_defs_free(defs);
|
||||
return false;
|
||||
}
|
||||
/* First writer wins: overloads on one line share a qualified name. */
|
||||
if (!cbm_ht_get(defs->by_line, key))
|
||||
cbm_ht_set(defs->by_line, key, (void *)def);
|
||||
if (cbm_ht_get(defs->by_line, key))
|
||||
continue;
|
||||
CBMDefinition *copy = cbm_arena_alloc(&defs->scratch, sizeof(*copy));
|
||||
const char *qualified_name = cbm_arena_strdup(arena, def->qualified_name);
|
||||
if (!copy || !qualified_name) {
|
||||
oo_defs_free(defs);
|
||||
return false;
|
||||
}
|
||||
*copy = *def;
|
||||
copy->qualified_name = qualified_name;
|
||||
cbm_ht_set(defs->by_line, key, copy);
|
||||
}
|
||||
return true;
|
||||
}
|
||||
@@ -234,6 +250,7 @@ bool oo_defs_init(oo_defs_t *defs, CBMArena *arena, const CBMFileResult *file) {
|
||||
void oo_defs_free(oo_defs_t *defs) {
|
||||
cbm_ht_free(defs->by_line);
|
||||
defs->by_line = NULL;
|
||||
cbm_arena_destroy(&defs->scratch);
|
||||
}
|
||||
|
||||
const CBMDefinition *oo_defs_find(const oo_defs_t *defs, char kind, uint32_t line,
|
||||
|
||||
@@ -297,7 +297,7 @@ const oo_callable_t *oo_lookup_ext_static(const cbm_oo_index_t *ix, const char *
|
||||
/* Per-file definition lookup while harvesting: extraction's exact node names. */
|
||||
typedef struct {
|
||||
CBMHashTable *by_line;
|
||||
CBMArena *arena;
|
||||
CBMArena scratch; /* the table's keys and definition copies, gone with the harvest */
|
||||
} oo_defs_t;
|
||||
bool oo_defs_init(oo_defs_t *defs, CBMArena *arena, const CBMFileResult *file);
|
||||
void oo_defs_free(oo_defs_t *defs);
|
||||
|
||||
@@ -330,8 +330,8 @@ static bool ecma_type_only_label(const char *label) {
|
||||
|
||||
/* TypeScript facts about two same-QN declarations of one file. Returns true when
|
||||
* they are one declared entity, which then needs no ambiguity verdict. */
|
||||
static bool ecma_merge_target(target_proof_t *prior, const CBMDefinition *definition,
|
||||
const char *file) {
|
||||
static bool ecma_merge_target(CBMArena *arena, target_proof_t *prior,
|
||||
const CBMDefinition *definition, const char *file) {
|
||||
if (strcmp(prior->file, file))
|
||||
return false;
|
||||
bool same_label = !strcmp(prior->label, definition->label);
|
||||
@@ -348,7 +348,7 @@ static bool ecma_merge_target(target_proof_t *prior, const CBMDefinition *defini
|
||||
}
|
||||
if (!same_label && !prior->merged_label &&
|
||||
ecma_type_only_label(prior->label) != ecma_type_only_label(definition->label)) {
|
||||
prior->merged_label = definition->label;
|
||||
prior->merged_label = cbm_arena_strdup(arena, definition->label);
|
||||
prior->merged_start_line = definition->start_line;
|
||||
prior->merged_end_line = definition->end_line;
|
||||
return true;
|
||||
@@ -399,7 +399,7 @@ static bool add_target(cbm_shared_resolver_t *resolver, const CBMDefinition *def
|
||||
if (prior) {
|
||||
if ((language == CBM_LANG_TYPESCRIPT || language == CBM_LANG_TSX ||
|
||||
language == CBM_LANG_ARKTS) &&
|
||||
ecma_merge_target(prior, definition, file))
|
||||
ecma_merge_target(&resolver->arena, prior, definition, file))
|
||||
return true;
|
||||
bool same_file = !strcmp(prior->file, file);
|
||||
bool same_owner = same_file && !strcmp(prior->label, definition->label);
|
||||
|
||||
@@ -10,6 +10,7 @@
|
||||
#include "pipeline/pipeline.h"
|
||||
#include "store/store.h"
|
||||
#include "sqlite3.h"
|
||||
#include "resolver_quality.h"
|
||||
#include <stdio.h>
|
||||
#include <stdlib.h>
|
||||
#include <string.h>
|
||||
@@ -893,7 +894,70 @@ TEST(resolver_oo_rust_context_failure_keeps_java_imports) {
|
||||
PASS();
|
||||
}
|
||||
|
||||
/* Over the memory budget every result is parked on disk and read back only for a moment.
|
||||
* The declaration index outlives each read, so it must own the names it keeps: it held
|
||||
* pointers into the freed result, and a resolve worker read them. */
|
||||
TEST(resolver_oo_declaration_index_outlives_a_spilled_result) {
|
||||
char root[256];
|
||||
snprintf(root, sizeof(root), "/tmp/cbm_oo_spill_XXXXXX");
|
||||
ASSERT_NOT_NULL(cbm_mkdtemp(root));
|
||||
char repo[512], database[512];
|
||||
snprintf(repo, sizeof(repo), "%s/repo", root);
|
||||
snprintf(database, sizeof(database), "%s/index.db", root);
|
||||
bool ok = cbm_mkdir_p(repo, 0755) == true;
|
||||
static const RQ_Source sources[] = {
|
||||
{.path = "app/Store.java",
|
||||
.text = "package app;\nclass Store {\n static int count;\n int read() { return count; }\n}\n"},
|
||||
{.path = "app/Use.java",
|
||||
.text = "package app;\nclass Use {\n int run(Store store) { return store.read(); }\n}\n"}};
|
||||
for (size_t i = 0; ok && i < sizeof(sources) / sizeof(sources[0]); i++)
|
||||
ok = th_write_file(TH_PATH(repo, sources[i].path), sources[i].text) == 0;
|
||||
for (int i = 0; ok && i < 55; i++) {
|
||||
char path[40];
|
||||
snprintf(path, sizeof(path), "SpillPadding%02d.java", i);
|
||||
ok = th_write_file(TH_PATH(repo, path), "// unrelated source file\n") == 0;
|
||||
}
|
||||
static const RQ_Expected expected[] = {
|
||||
{.case_id = "spilled-member-call",
|
||||
.site = {.file = "app/Use.java", .caller = "app.Use.run", .needle = "store.read()"},
|
||||
.relation = "CALLS",
|
||||
.target = {.file = "app/Store.java",
|
||||
.suffix = "app.Store.read",
|
||||
.label = "Method",
|
||||
.declaration_line = 4,
|
||||
.declaration_text = " int read() { return count; }"},
|
||||
.mode = RQ_REQUIRED},
|
||||
{.case_id = "spilled-static-field",
|
||||
.site = {.file = "app/Store.java", .caller = "app.Store.read", .needle = "count", .occurrence_index = 1},
|
||||
.relation = "USAGE",
|
||||
.target = {.file = "app/Store.java",
|
||||
.suffix = "app.Store.count",
|
||||
.label = "Field",
|
||||
.declaration_line = 3,
|
||||
.declaration_text = " static int count;"},
|
||||
.mode = RQ_REQUIRED}};
|
||||
cbm_setenv("CBM_MEM_SPILL", "1", 1);
|
||||
cbm_setenv("CBM_WORKERS", "4", 1);
|
||||
cbm_pipeline_t *pipeline = ok ? cbm_pipeline_new(repo, database, CBM_MODE_FULL) : NULL;
|
||||
ok = pipeline && cbm_pipeline_set_project_name(pipeline, "oo-spill") &&
|
||||
cbm_pipeline_run(pipeline) == 0;
|
||||
cbm_pipeline_free(pipeline);
|
||||
cbm_unsetenv("CBM_WORKERS");
|
||||
cbm_unsetenv("CBM_MEM_SPILL");
|
||||
cbm_store_t *store = ok ? cbm_store_open_path_existing(database) : NULL;
|
||||
RQ_Report report = {0};
|
||||
ok = store && rq_check(store, "oo-spill", "oo-spill", sources, 2, expected, 2, NULL, 0, &report,
|
||||
stderr);
|
||||
if (store)
|
||||
cbm_store_close(store);
|
||||
th_cleanup(root);
|
||||
ASSERT(ok);
|
||||
ASSERT_EQ(report.found, 2);
|
||||
PASS();
|
||||
}
|
||||
|
||||
SUITE(resolver_oo) {
|
||||
RUN_TEST(resolver_oo_declaration_index_outlives_a_spilled_result);
|
||||
RUN_TEST(resolver_oo_java_static_selection);
|
||||
RUN_TEST(resolver_oo_java_static_selection_parallel);
|
||||
RUN_TEST(resolver_oo_kotlin_static_selection);
|
||||
|
||||
Reference in new issue
Block a user