Skip to content

Commit db330bf

Browse files
authored
Merge pull request DeusData#1050 from DeusData/fix/961-ifdef-split-defs
fix(extract): recover C/C++ defs lost to #ifdef-split brace ERROR regions
2 parents be7bece + 65a176e commit db330bf

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
@@ -3389,6 +3389,64 @@ TEST(extract_js_member_call_flags_is_method) {
33893389
PASS();
33903390
}
33913391

3392+
/* #961: a C function whose body braces are split across #ifdef/#else
3393+
* branches (one open brace per branch, a single shared close) parses with
3394+
* an ERROR region on the raw source — both branches are present at once —
3395+
* and the defs walk silently dropped the function while its callers stayed
3396+
* (cbm_path_within_root, handle_process_kill). The preprocessed second
3397+
* pass (simplecpp picks one branch, same-file token lines stay aligned)
3398+
* must recover the definition with its original line. */
3399+
TEST(extract_c_ifdef_split_brace_fn_recovered_issue961) {
3400+
CBMFileResult *r = extract("static int a(void) { return 1; }\n"
3401+
"static int b(void) { return 2; }\n"
3402+
"int split_brace_fn(int x) {\n"
3403+
"#ifdef _WIN32\n"
3404+
" if (a() && b()) {\n"
3405+
"#else\n"
3406+
" if (a() || b()) {\n"
3407+
"#endif\n"
3408+
" x += 1;\n"
3409+
" }\n"
3410+
" return x;\n"
3411+
"}\n"
3412+
"int after_fn(int x) { return x; }\n",
3413+
CBM_LANG_C, "t", "split.c");
3414+
ASSERT_NOT_NULL(r);
3415+
const CBMDefinition *d = find_def(r, "split_brace_fn");
3416+
if (!d) {
3417+
fprintf(stderr, " [961] FAIL split_brace_fn dropped (defs walk lost the "
3418+
"#ifdef-split function)\n");
3419+
}
3420+
ASSERT_NOT_NULL(d);
3421+
ASSERT_EQ((int)d->start_line, 3);
3422+
/* Error recovery must stay localized: neighbours extract either way. */
3423+
ASSERT_NOT_NULL(find_def(r, "a"));
3424+
ASSERT_NOT_NULL(find_def(r, "after_fn"));
3425+
cbm_free_result(r);
3426+
PASS();
3427+
}
3428+
3429+
/* #961 inverse guard: a clean C file must not gain duplicate or phantom
3430+
* defs from the recovery path (it only engages on raw-parse ERROR regions). */
3431+
TEST(extract_c_clean_file_no_recovery_duplicates_issue961) {
3432+
CBMFileResult *r = extract("#ifdef _WIN32\n"
3433+
"static int w(void) { return 1; }\n"
3434+
"#else\n"
3435+
"static int u(void) { return 2; }\n"
3436+
"#endif\n"
3437+
"int use(int x) { return x; }\n",
3438+
CBM_LANG_C, "t", "clean.c");
3439+
ASSERT_NOT_NULL(r);
3440+
int use_count = 0;
3441+
for (int i = 0; i < r->defs.count; i++) {
3442+
if (r->defs.items[i].name && strcmp(r->defs.items[i].name, "use") == 0)
3443+
use_count++;
3444+
}
3445+
ASSERT_EQ(use_count, 1);
3446+
cbm_free_result(r);
3447+
PASS();
3448+
}
3449+
33923450
/* #668: walk_defs used a fixed `walk_defs_frame_t stack[4096]` — a ~160 KB
33933451
* C-stack frame that overflowed small thread stacks (the reporter's crash was in
33943452
* the "definitions pass" on a large SQL file), and whose `top < 4096` push guards
@@ -3806,6 +3864,8 @@ SUITE(extraction) {
38063864
RUN_TEST(complexity_go_method_receiver_self_recursion);
38073865
RUN_TEST(complexity_delegation_receivers_not_recursive_issue876);
38083866
RUN_TEST(complexity_access_depth_and_params);
3867+
RUN_TEST(extract_c_ifdef_split_brace_fn_recovered_issue961);
3868+
RUN_TEST(extract_c_clean_file_no_recovery_duplicates_issue961);
38093869
RUN_TEST(walk_defs_no_truncation_over_4096_issue668);
38103870
RUN_TEST(extract_rust_test_attr_marks_is_test_issue855);
38113871

0 commit comments

Comments
 (0)