Skip to content

Commit 65a176e

Browse files
committed
fix(extract): recover C/C++ defs lost to #ifdef-split brace ERROR regions
A function whose body braces are split across #ifdef/#else branches (one open brace per branch, a single shared close) parses with an ERROR region on the raw source - both branches are present at once, so the braces are unbalanced - and the defs walk silently dropped it while its callers stayed as unresolved names (cbm_path_within_root, handle_process_kill). The preprocessed second pass (simplecpp) already exists for macro- hidden CALLS; it evaluates the conditionals, so one branch is chosen and the expanded tree parses clean, with same-file token lines aligned to the original. Recover definitions from it, adopting only defs that (a) intersect a raw-tree ERROR region, (b) have their name visible on the corresponding original source line (rejects header-inlined defs whose physical expanded lines alias unrelated raw lines when compile_commands include paths resolve), and (c) are absent by qualified name from the raw pass. Verified on the real instances: handle_process_kill (580-647) and cbm_path_within_root (5070-5085) both index with correct line ranges. Closes #961 Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
1 parent 8c9ac98 commit 65a176e

2 files changed

Lines changed: 138 additions & 0 deletions

File tree

internal/cbm/cbm.c

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -817,6 +817,38 @@ static bool cbm_region_is_recovered(uint32_t rs, uint32_t re, const CBMDefArray
817817
return covered_to >= re;
818818
}
819819

820+
/* #961: true when 1-based `line` of `src` contains `name` (used to verify a
821+
* def recovered from EXPANDED source really lives on that ORIGINAL line —
822+
* rejects header-inlined defs whose physical expanded lines alias unrelated
823+
* raw lines when compile_commands include paths are present). */
824+
static bool cbm_line_contains(const char *src, int src_len, uint32_t line, const char *name) {
825+
if (!src || !name || !name[0] || line == 0) {
826+
return false;
827+
}
828+
uint32_t cur = 1;
829+
int i = 0;
830+
while (i < src_len && cur < line) {
831+
if (src[i] == '\n') {
832+
cur++;
833+
}
834+
i++;
835+
}
836+
if (cur != line) {
837+
return false;
838+
}
839+
int end = i;
840+
while (end < src_len && src[end] != '\n') {
841+
end++;
842+
}
843+
size_t nlen = strlen(name);
844+
for (int j = i; j + (int)nlen <= end; j++) {
845+
if (strncmp(src + j, name, nlen) == 0) {
846+
return true;
847+
}
848+
}
849+
return false;
850+
}
851+
820852
static void cbm_subtract_recovered_regions(cbm_error_regions_t *regs, const CBMDefArray *defs) {
821853
int kept = 0;
822854
for (int i = 0; i < regs->count; i++) {
@@ -1103,6 +1135,52 @@ static CBMFileResult *cbm_extract_file_impl(const char *source, int source_len,
11031135
cbm_run_c_lsp(a, result, expanded, expanded_len, pp_root,
11041136
language != CBM_LANG_C);
11051137

1138+
/* #961: a def whose body braces are split across
1139+
* #ifdef/#else branches parses as an ERROR region on the
1140+
* RAW source (both branches present at once -> unbalanced
1141+
* braces), so the raw defs walk silently dropped it. The
1142+
* expanded tree parses clean (simplecpp picked one
1143+
* branch) and same-file token lines stay aligned, so
1144+
* recover defs from it — adopting ONLY those that
1145+
* intersect a raw ERROR region, whose name is visible on
1146+
* the raw source line, and whose QN the raw pass did not
1147+
* already extract. */
1148+
if (ts_node_has_error(root)) {
1149+
cbm_error_regions_t raw_regs = {{0}, {0}, 0};
1150+
cbm_collect_error_regions(root, &raw_regs);
1151+
if (raw_regs.count > 0) {
1152+
int defs_before = result->defs.count;
1153+
cbm_extract_definitions(&pp_ctx);
1154+
int w = defs_before;
1155+
for (int i = defs_before; i < result->defs.count; i++) {
1156+
CBMDefinition *d = &result->defs.items[i];
1157+
bool adopt = false;
1158+
for (int rj = 0; rj < raw_regs.count && !adopt; rj++) {
1159+
if (d->start_line <= raw_regs.ends[rj] &&
1160+
d->end_line >= raw_regs.starts[rj]) {
1161+
adopt = true;
1162+
}
1163+
}
1164+
if (adopt &&
1165+
(!d->name || !cbm_line_contains(source, source_len,
1166+
d->start_line, d->name))) {
1167+
adopt = false;
1168+
}
1169+
for (int j = 0; j < defs_before && adopt; j++) {
1170+
const char *q = result->defs.items[j].qualified_name;
1171+
if (q && d->qualified_name &&
1172+
strcmp(q, d->qualified_name) == 0) {
1173+
adopt = false;
1174+
}
1175+
}
1176+
if (adopt) {
1177+
result->defs.items[w++] = *d;
1178+
}
1179+
}
1180+
result->defs.count = w;
1181+
}
1182+
}
1183+
11061184
ts_tree_delete(pp_tree);
11071185
}
11081186
}

tests/test_extraction.c

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3321,6 +3321,64 @@ TEST(extract_js_member_call_flags_is_method) {
33213321
PASS();
33223322
}
33233323

3324+
/* #961: a C function whose body braces are split across #ifdef/#else
3325+
* branches (one open brace per branch, a single shared close) parses with
3326+
* an ERROR region on the raw source — both branches are present at once —
3327+
* and the defs walk silently dropped the function while its callers stayed
3328+
* (cbm_path_within_root, handle_process_kill). The preprocessed second
3329+
* pass (simplecpp picks one branch, same-file token lines stay aligned)
3330+
* must recover the definition with its original line. */
3331+
TEST(extract_c_ifdef_split_brace_fn_recovered_issue961) {
3332+
CBMFileResult *r = extract("static int a(void) { return 1; }\n"
3333+
"static int b(void) { return 2; }\n"
3334+
"int split_brace_fn(int x) {\n"
3335+
"#ifdef _WIN32\n"
3336+
" if (a() && b()) {\n"
3337+
"#else\n"
3338+
" if (a() || b()) {\n"
3339+
"#endif\n"
3340+
" x += 1;\n"
3341+
" }\n"
3342+
" return x;\n"
3343+
"}\n"
3344+
"int after_fn(int x) { return x; }\n",
3345+
CBM_LANG_C, "t", "split.c");
3346+
ASSERT_NOT_NULL(r);
3347+
const CBMDefinition *d = find_def(r, "split_brace_fn");
3348+
if (!d) {
3349+
fprintf(stderr, " [961] FAIL split_brace_fn dropped (defs walk lost the "
3350+
"#ifdef-split function)\n");
3351+
}
3352+
ASSERT_NOT_NULL(d);
3353+
ASSERT_EQ((int)d->start_line, 3);
3354+
/* Error recovery must stay localized: neighbours extract either way. */
3355+
ASSERT_NOT_NULL(find_def(r, "a"));
3356+
ASSERT_NOT_NULL(find_def(r, "after_fn"));
3357+
cbm_free_result(r);
3358+
PASS();
3359+
}
3360+
3361+
/* #961 inverse guard: a clean C file must not gain duplicate or phantom
3362+
* defs from the recovery path (it only engages on raw-parse ERROR regions). */
3363+
TEST(extract_c_clean_file_no_recovery_duplicates_issue961) {
3364+
CBMFileResult *r = extract("#ifdef _WIN32\n"
3365+
"static int w(void) { return 1; }\n"
3366+
"#else\n"
3367+
"static int u(void) { return 2; }\n"
3368+
"#endif\n"
3369+
"int use(int x) { return x; }\n",
3370+
CBM_LANG_C, "t", "clean.c");
3371+
ASSERT_NOT_NULL(r);
3372+
int use_count = 0;
3373+
for (int i = 0; i < r->defs.count; i++) {
3374+
if (r->defs.items[i].name && strcmp(r->defs.items[i].name, "use") == 0)
3375+
use_count++;
3376+
}
3377+
ASSERT_EQ(use_count, 1);
3378+
cbm_free_result(r);
3379+
PASS();
3380+
}
3381+
33243382
/* #668: walk_defs used a fixed `walk_defs_frame_t stack[4096]` — a ~160 KB
33253383
* C-stack frame that overflowed small thread stacks (the reporter's crash was in
33263384
* the "definitions pass" on a large SQL file), and whose `top < 4096` push guards
@@ -3737,6 +3795,8 @@ SUITE(extraction) {
37373795
RUN_TEST(complexity_chained_receiver_not_self);
37383796
RUN_TEST(complexity_go_method_receiver_self_recursion);
37393797
RUN_TEST(complexity_access_depth_and_params);
3798+
RUN_TEST(extract_c_ifdef_split_brace_fn_recovered_issue961);
3799+
RUN_TEST(extract_c_clean_file_no_recovery_duplicates_issue961);
37403800
RUN_TEST(walk_defs_no_truncation_over_4096_issue668);
37413801
RUN_TEST(extract_rust_test_attr_marks_is_test_issue855);
37423802

0 commit comments

Comments
 (0)