diff --git a/internal/cbm/lsp/oo_core.c b/internal/cbm/lsp/oo_core.c index cc120c918..1d6fd730a 100644 --- a/internal/cbm/lsp/oo_core.c +++ b/internal/cbm/lsp/oo_core.c @@ -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, diff --git a/internal/cbm/lsp/oo_internal.h b/internal/cbm/lsp/oo_internal.h index 1caadc56a..a3bc02c7b 100644 --- a/internal/cbm/lsp/oo_internal.h +++ b/internal/cbm/lsp/oo_internal.h @@ -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); diff --git a/src/pipeline/pass_resolver.c b/src/pipeline/pass_resolver.c index 12d7fe862..07286da36 100644 --- a/src/pipeline/pass_resolver.c +++ b/src/pipeline/pass_resolver.c @@ -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); diff --git a/tests/test_resolver_oo.c b/tests/test_resolver_oo.c index f3fe62527..343447f5f 100644 --- a/tests/test_resolver_oo.c +++ b/tests/test_resolver_oo.c @@ -10,6 +10,7 @@ #include "pipeline/pipeline.h" #include "store/store.h" #include "sqlite3.h" +#include "resolver_quality.h" #include #include #include @@ -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);