From 99d39192cd2928ea8bf44233964fe120b201607a Mon Sep 17 00:00:00 2001 From: Blank_Answer <97771966+blankanswer@users.noreply.github.com> Date: Mon, 13 Jul 2026 05:21:54 +0000 Subject: [PATCH] fix(cpp): remap preprocessed recovery lines Signed-off-by: Blank_Answer <97771966+blankanswer@users.noreply.github.com> --- internal/cbm/cbm.c | 130 +++++++++++++++++++++-- internal/cbm/preprocessor.cpp | 192 +++++++++++++++++++++++++++++----- internal/cbm/preprocessor.h | 18 ++++ tests/test_extraction.c | 136 ++++++++++++++++++++++++ 4 files changed, 439 insertions(+), 37 deletions(-) diff --git a/internal/cbm/cbm.c b/internal/cbm/cbm.c index af454426f..8409aa9d8 100644 --- a/internal/cbm/cbm.c +++ b/internal/cbm/cbm.c @@ -849,6 +849,106 @@ static bool cbm_line_contains(const char *src, int src_len, uint32_t line, const return false; } +static bool cbm_identifier_char(char c) { + return isalnum((unsigned char)c) || c == '_'; +} + +/* Verify that the mapped original span contains callable-definition syntax, + * not merely the name at a macro invocation or call site. */ +static bool cbm_span_contains_callable_def(const char *src, int src_len, uint32_t start_line, + uint32_t end_line, const char *name) { + if (!src || src_len <= 0 || !name || !name[0] || start_line == 0 || end_line < start_line) { + return false; + } + int span_start = 0; + uint32_t line = 1; + while (span_start < src_len && line < start_line) { + if (src[span_start++] == '\n') { + line++; + } + } + if (line != start_line) { + return false; + } + int span_end = span_start; + while (span_end < src_len && line <= end_line) { + if (src[span_end++] == '\n') { + line++; + } + } + size_t name_len = strlen(name); + for (int pos = span_start; pos + (int)name_len <= span_end; pos++) { + if (strncmp(src + pos, name, name_len) != 0 || + (pos > 0 && cbm_identifier_char(src[pos - 1])) || + (pos + (int)name_len < src_len && cbm_identifier_char(src[pos + name_len]))) { + continue; + } + int before = pos; + while (before > span_start && isspace((unsigned char)src[before - 1])) { + before--; + } + if (before > span_start && strchr("(,=!?[.", src[before - 1])) { + continue; + } + int open = pos + (int)name_len; + while (open < span_end && isspace((unsigned char)src[open])) { + open++; + } + if (open >= span_end || src[open] != '(') { + continue; + } + int depth = 0; + int close = -1; + for (int i = open; i < span_end; i++) { + if (src[i] == '(') { + depth++; + } else if (src[i] == ')' && --depth == 0) { + close = i; + break; + } + } + if (close < 0) { + continue; + } + for (int i = close + 1; i < span_end; i++) { + if (src[i] == '{') { + return true; + } + if (src[i] == ';') { + break; + } + } + } + return false; +} + +/* Remap an expanded-source definition back to the original input file. Every + * line in the definition must be attributable to the main file; generated + * macro bodies, included headers, and ambiguous spans fail closed. */ +static bool cbm_remap_preprocessed_def(CBMDefinition *def, const CBMPreprocessedSource *pp) { + if (!def || !pp || !pp->original_line_by_expanded_line || !pp->belongs_to_main_file || + def->start_line == 0 || def->end_line < def->start_line || + def->end_line > (uint32_t)pp->expanded_line_count) { + return false; + } + + uint32_t original_start = pp->original_line_by_expanded_line[def->start_line]; + uint32_t original_end = pp->original_line_by_expanded_line[def->end_line]; + if (!original_start || !original_end || original_end < original_start) { + return false; + } + for (uint32_t line = def->start_line; line <= def->end_line; line++) { + if (!pp->belongs_to_main_file[line] || !pp->original_line_by_expanded_line[line]) { + return false; + } + } + + def->start_line = original_start; + def->end_line = original_end; + def->lines = (int)(original_end - original_start + 1); + return true; +} + static void cbm_subtract_recovered_regions(cbm_error_regions_t *regs, const CBMDefArray *defs) { int kept = 0; for (int i = 0; i < regs->count; i++) { @@ -1088,9 +1188,10 @@ static CBMFileResult *cbm_extract_file_impl(const char *source, int source_len, // Defs keep original-source line numbers; only CALLS are extracted from expanded source. if (language == CBM_LANG_C || language == CBM_LANG_CPP || language == CBM_LANG_CUDA) { uint64_t pp_start = now_ns(); - char *expanded = cbm_preprocess(source, source_len, rel_path, extra_defines, include_paths, - language != CBM_LANG_C); - if (expanded) { + CBMPreprocessedSource *preprocessed = cbm_preprocess_with_map( + source, source_len, rel_path, extra_defines, include_paths, language != CBM_LANG_C); + if (preprocessed && preprocessed->source) { + char *expanded = preprocessed->source; int expanded_len = (int)strlen(expanded); // Record calls count before second pass int calls_before = result->calls.count; @@ -1155,15 +1256,20 @@ static CBMFileResult *cbm_extract_file_impl(const char *source, int source_len, for (int i = defs_before; i < result->defs.count; i++) { CBMDefinition *d = &result->defs.items[i]; bool adopt = false; - for (int rj = 0; rj < raw_regs.count && !adopt; rj++) { - if (d->start_line <= raw_regs.ends[rj] && - d->end_line >= raw_regs.starts[rj]) { - adopt = true; + if (cbm_remap_preprocessed_def(d, preprocessed)) { + for (int rj = 0; rj < raw_regs.count && !adopt; rj++) { + if (d->start_line <= raw_regs.ends[rj] && + d->end_line >= raw_regs.starts[rj]) { + adopt = true; + } } } - if (adopt && - (!d->name || !cbm_line_contains(source, source_len, - d->start_line, d->name))) { + if (adopt && (!d->name || + !cbm_line_contains(source, source_len, d->start_line, + d->name) || + !cbm_span_contains_callable_def( + source, source_len, d->start_line, d->end_line, + d->name))) { adopt = false; } for (int j = 0; j < defs_before && adopt; j++) { @@ -1184,9 +1290,11 @@ static CBMFileResult *cbm_extract_file_impl(const char *source, int source_len, ts_tree_delete(pp_tree); } } - cbm_preprocess_free(expanded); + cbm_preprocessed_source_free(preprocessed); atomic_fetch_add(&total_files_preprocessed, 1); (void)calls_before; // used for future logging + } else { + cbm_preprocessed_source_free(preprocessed); } atomic_fetch_add(&total_preprocess_ns, now_ns() - pp_start); } diff --git a/internal/cbm/preprocessor.cpp b/internal/cbm/preprocessor.cpp index 69f91c35a..54cb073fe 100644 --- a/internal/cbm/preprocessor.cpp +++ b/internal/cbm/preprocessor.cpp @@ -10,45 +10,145 @@ #include #include #include +#include extern "C" { -char* cbm_preprocess( - const char* source, int source_len, - const char* filename, - const char** extra_defines, - const char** include_paths, - int cpp_mode -) { - if (!source || source_len <= 0) return NULL; - - // Fast-path: skip if no preprocessor directives worth expanding. - bool has_macros = false; +static bool has_preprocessor_work(const char *source, int source_len) { + if (!source || source_len <= 0) { + return false; + } for (int i = 0; i < source_len - 1; i++) { if (source[i] == '#') { // Skip whitespace after # int j = i + 1; - while (j < source_len && (source[j] == ' ' || source[j] == '\t')) j++; + while (j < source_len && (source[j] == ' ' || source[j] == '\t')) + j++; int remaining = source_len - j; if (remaining >= 6 && strncmp(source + j, "define", 6) == 0) { - has_macros = true; - break; + return true; } if (remaining >= 5 && strncmp(source + j, "ifdef", 5) == 0) { - has_macros = true; - break; + return true; } if (remaining >= 6 && strncmp(source + j, "ifndef", 6) == 0) { - has_macros = true; - break; + return true; } if (remaining >= 3 && strncmp(source + j, "if ", 3) == 0) { - has_macros = true; - break; + return true; } } } - if (!has_macros) return NULL; // NULL = no expansion needed, use original + return false; +} + +static int count_expanded_lines(const std::string &text) { + int count = 1; + for (char c : text) { + if (c == '\n') { + count++; + } + } + return count; +} + +static bool parse_line_directive(const char *line, size_t len, uint32_t *out_line, + std::string *out_file) { + size_t i = 0; + while (i < len && (line[i] == ' ' || line[i] == '\t')) { + i++; + } + if (i >= len || line[i++] != '#') { + return false; + } + while (i < len && (line[i] == ' ' || line[i] == '\t')) { + i++; + } + static const char prefix[] = "line"; + if (i + sizeof(prefix) - 1 > len || strncmp(line + i, prefix, sizeof(prefix) - 1) != 0) { + return false; + } + i += sizeof(prefix) - 1; + if (i >= len || (line[i] != ' ' && line[i] != '\t')) { + return false; + } + while (i < len && (line[i] == ' ' || line[i] == '\t')) { + i++; + } + if (i >= len || line[i] < '0' || line[i] > '9') { + return false; + } + uint64_t parsed_line = 0; + while (i < len && line[i] >= '0' && line[i] <= '9') { + parsed_line = parsed_line * 10u + (uint64_t)(line[i] - '0'); + if (parsed_line > UINT32_MAX) { + return false; + } + i++; + } + while (i < len && (line[i] == ' ' || line[i] == '\t')) { + i++; + } + if (i >= len || line[i++] != '"') { + return false; + } + size_t file_start = i; + while (i < len && line[i] != '"') { + i++; + } + if (i >= len) { + return false; + } + *out_line = (uint32_t)parsed_line; + *out_file = std::string(line + file_start, i - file_start); + return true; +} + +static bool build_line_map(const std::string &expanded, const std::string &main_file, + uint32_t *original_line_by_expanded_line, + uint8_t *belongs_to_main_file) { + std::string current_file = main_file; + uint32_t current_line = 1; + int expanded_line = 1; + size_t line_start = 0; + + while (line_start <= expanded.size()) { + size_t line_end = expanded.find('\n', line_start); + if (line_end == std::string::npos) { + line_end = expanded.size(); + } + + uint32_t directive_line = 0; + std::string directive_file; + if (parse_line_directive(expanded.c_str() + line_start, line_end - line_start, + &directive_line, &directive_file)) { + current_file = directive_file; + current_line = directive_line; + original_line_by_expanded_line[expanded_line] = 0; + belongs_to_main_file[expanded_line] = 0; + } else { + original_line_by_expanded_line[expanded_line] = current_line; + belongs_to_main_file[expanded_line] = current_file == main_file ? 1 : 0; + if (current_line < UINT32_MAX) { + current_line++; + } + } + + if (line_end == expanded.size()) { + break; + } + line_start = line_end + 1; + expanded_line++; + } + return true; +} + +CBMPreprocessedSource *cbm_preprocess_with_map(const char *source, int source_len, + const char *filename, const char **extra_defines, + const char **include_paths, int cpp_mode) { + if (!has_preprocessor_work(source, source_len)) { + return NULL; // NULL = no expansion needed, use original + } try { simplecpp::DUI dui; @@ -78,18 +178,58 @@ char* cbm_preprocess( // Clean up loaded file data simplecpp::cleanup(filedata); - char* out = (char*)malloc(result.size() + 1); - if (!out) return NULL; - memcpy(out, result.c_str(), result.size() + 1); - return out; + CBMPreprocessedSource *pp = (CBMPreprocessedSource *)calloc(1, sizeof(*pp)); + if (!pp) { + return NULL; + } + int line_count = count_expanded_lines(result); + pp->source = (char *)malloc(result.size() + 1); + pp->original_line_by_expanded_line = + (uint32_t *)calloc((size_t)line_count + 1u, sizeof(uint32_t)); + pp->belongs_to_main_file = (uint8_t *)calloc((size_t)line_count + 1u, sizeof(uint8_t)); + pp->expanded_line_count = line_count; + if (!pp->source || !pp->original_line_by_expanded_line || !pp->belongs_to_main_file) { + cbm_preprocessed_source_free(pp); + return NULL; + } + memcpy(pp->source, result.c_str(), result.size() + 1); + if (!build_line_map(result, files[0], pp->original_line_by_expanded_line, + pp->belongs_to_main_file)) { + cbm_preprocessed_source_free(pp); + return NULL; + } + return pp; } catch (...) { // Graceful fallback: return NULL = use original source return NULL; } } -void cbm_preprocess_free(char* expanded) { +char *cbm_preprocess(const char *source, int source_len, const char *filename, + const char **extra_defines, const char **include_paths, int cpp_mode) { + CBMPreprocessedSource *pp = cbm_preprocess_with_map(source, source_len, filename, extra_defines, + include_paths, cpp_mode); + if (!pp) { + return NULL; + } + char *out = pp->source; + pp->source = NULL; + cbm_preprocessed_source_free(pp); + return out; +} + +void cbm_preprocess_free(char *expanded) { free(expanded); } +void cbm_preprocessed_source_free(CBMPreprocessedSource *pp) { + if (!pp) { + return; + } + free(pp->source); + free(pp->original_line_by_expanded_line); + free(pp->belongs_to_main_file); + free(pp); +} + } // extern "C" diff --git a/internal/cbm/preprocessor.h b/internal/cbm/preprocessor.h index 06edbc087..72828c012 100644 --- a/internal/cbm/preprocessor.h +++ b/internal/cbm/preprocessor.h @@ -2,11 +2,19 @@ #define CBM_PREPROCESSOR_H #include +#include #ifdef __cplusplus extern "C" { #endif +typedef struct { + char *source; + uint32_t *original_line_by_expanded_line; // 1-based; 0 means directive/unmapped. + uint8_t *belongs_to_main_file; // 1-based; true only for the original input file. + int expanded_line_count; +} CBMPreprocessedSource; + // Preprocess C/C++ source: expand macros, evaluate #ifdef, resolve #include. // Returns malloc-allocated expanded source, or NULL if no expansion needed/on failure. // extra_defines: NULL-terminated array of "NAME=VALUE" strings (can be NULL). @@ -15,9 +23,19 @@ extern "C" { char *cbm_preprocess(const char *source, int source_len, const char *filename, const char **extra_defines, const char **include_paths, int cpp_mode); +// Preprocess and return source plus expanded-line -> original-line ownership map. +// Returns NULL if no expansion is needed or preprocessing fails. +// Free with cbm_preprocessed_source_free(). +CBMPreprocessedSource *cbm_preprocess_with_map(const char *source, int source_len, + const char *filename, const char **extra_defines, + const char **include_paths, int cpp_mode); + // Free preprocessed source returned by cbm_preprocess. void cbm_preprocess_free(char *expanded); +// Free preprocessed source and line maps returned by cbm_preprocess_with_map. +void cbm_preprocessed_source_free(CBMPreprocessedSource *pp); + #ifdef __cplusplus } #endif diff --git a/tests/test_extraction.c b/tests/test_extraction.c index 686691974..eed871ff7 100644 --- a/tests/test_extraction.c +++ b/tests/test_extraction.c @@ -8,6 +8,7 @@ #include "test_framework.h" #include "cbm.h" #include "../src/foundation/compat.h" /* cbm_clock_gettime (wide-flat scaling guard) */ +#include "../src/foundation/compat_fs.h" #include /* ── Helpers ───────────────────────────────────────────────────── */ @@ -3426,6 +3427,138 @@ TEST(extract_c_ifdef_split_brace_fn_recovered_issue961) { PASS(); } +static const char *CPP_PREPROC_SIGNATURE_GAP_SRC = + "struct Rect {};\n" + "struct IBinder {};\n" + "struct IRegionSamplingListener {};\n" + "typedef int status_t;\n" + "\n" + "class SurfaceFlinger {\n" + "public:\n" + " status_t addRegionSamplingListener(const Rect&, const IBinder&,\n" + " const IRegionSamplingListener&, bool);\n" + " status_t addRegionSamplingListener(const Rect&, const IBinder&,\n" + " const IRegionSamplingListener&);\n" + " void commit();\n" + " void composite();\n" + "};\n" + "\n" + "#ifdef FLYME_GRAPHICS_EXTEND_LUMARGB\n" + "status_t SurfaceFlinger::addRegionSamplingListener(const Rect& samplingArea,\n" + " const IBinder& stopLayerHandle,\n" + " const IRegionSamplingListener& listener,\n" + " const bool rgbSample) {\n" + "#else\n" + "status_t SurfaceFlinger::addRegionSamplingListener(const Rect& samplingArea,\n" + " const IBinder& stopLayerHandle,\n" + " const IRegionSamplingListener& listener) " + "{\n" + "#endif\n" + " return 0;\n" + "}\n" + "\n" + "void SurfaceFlinger::commit() {}\n" + "\n" + "void SurfaceFlinger::composite() {}\n"; + +/* #946 fixture from the original report: both preprocessor choices must keep + * raw definitions primary while recovering later methods at original lines. */ +TEST(extract_cpp_preproc_signature_gap_issue946) { + const char *defines[] = {"FLYME_GRAPHICS_EXTEND_LUMARGB", NULL}; + for (int enabled = 0; enabled < 2; enabled++) { + CBMFileResult *r = cbm_extract_file( + CPP_PREPROC_SIGNATURE_GAP_SRC, (int)strlen(CPP_PREPROC_SIGNATURE_GAP_SRC), CBM_LANG_CPP, + "t", "SurfaceFlinger.cpp", 0, enabled ? defines : NULL, NULL); + ASSERT_NOT_NULL(r); + const CBMDefinition *add = find_def(r, "addRegionSamplingListener"); + const CBMDefinition *commit = find_def(r, "commit"); + const CBMDefinition *composite = find_def(r, "composite"); + ASSERT_NOT_NULL(add); + ASSERT_NOT_NULL(commit); + ASSERT_NOT_NULL(composite); + ASSERT_EQ(add->start_line, 22u); + ASSERT_EQ(add->end_line, 27u); + ASSERT_EQ(commit->start_line, 29u); + ASSERT_EQ(commit->end_line, 29u); + ASSERT_EQ(composite->start_line, 31u); + ASSERT_EQ(composite->end_line, 31u); + cbm_free_result(r); + } + PASS(); +} + +/* Macro expansion can produce callable-looking AST nodes, but no callable + * definition exists in the original span; recovery must fail closed. */ +TEST(extract_cpp_preproc_macro_generated_callable_skipped_issue949) { + const char *src = "#define MAKE_FN(name) int name() { return 1; }\n" + "#ifdef ENABLE_GENERATED\n" + "MAKE_FN(generated)\n" + "#endif\n" + "int visible() { return 0; }\n"; + const char *defines[] = {"ENABLE_GENERATED", NULL}; + CBMFileResult *r = + cbm_extract_file(src, (int)strlen(src), CBM_LANG_CPP, "t", "macro.cpp", 0, defines, NULL); + ASSERT_NOT_NULL(r); + ASSERT_NULL(find_def(r, "generated")); + ASSERT_NOT_NULL(find_def(r, "visible")); + ASSERT_TRUE(r->parse_incomplete); + ASSERT_GTE(r->error_region_count, 1); + ASSERT_NOT_NULL(r->error_ranges); + cbm_free_result(r); + PASS(); +} + +/* #949 follow-up: an included header shifts physical lines in simplecpp's + * expanded output. The #1050 name-on-same-line guard skipped this recoverable + * definition; explicit source ownership mapping must restore its original + * coordinates while keeping header definitions out of the main file. */ +TEST(extract_c_ifdef_split_brace_after_include_remapped_issue949) { + char tmpdir[512]; + snprintf(tmpdir, sizeof(tmpdir), "%s/cbm_line_map_XXXXXX", cbm_tmpdir()); + ASSERT_NOT_NULL(cbm_mkdtemp(tmpdir)); + + char header_path[512]; + snprintf(header_path, sizeof(header_path), "%s/padding.h", tmpdir); + FILE *header = cbm_fopen(header_path, "wb"); + ASSERT_NOT_NULL(header); + for (int i = 0; i < 40; i++) { + ASSERT_GTE(fprintf(header, "static int header_pad_%d(void) { return %d; }\n", i, i), 0); + } + ASSERT_EQ(fclose(header), 0); + + const char *includes[] = {tmpdir, NULL}; + const char *src = "#include \"padding.h\"\n" + "static int before(void) { return 1; }\n" + "int shifted_split(int x) {\n" + "#ifdef _WIN32\n" + " if (before()) {\n" + "#else\n" + " if (x > 0) {\n" + "#endif\n" + " x += 1;\n" + " }\n" + " return x;\n" + "}\n" + "int after(void) { return 2; }\n"; + CBMFileResult *r = + cbm_extract_file(src, (int)strlen(src), CBM_LANG_C, "t", "shifted.c", 0, NULL, includes); + cbm_unlink(header_path); + cbm_rmdir(tmpdir); + + ASSERT_NOT_NULL(r); + const CBMDefinition *d = find_def(r, "shifted_split"); + ASSERT_NOT_NULL(d); + ASSERT_EQ(d->start_line, 3u); + ASSERT_EQ(d->end_line, 12u); + ASSERT_NULL(find_def(r, "header_pad_0")); + ASSERT_NOT_NULL(find_def(r, "after")); + ASSERT_FALSE(r->parse_incomplete); + ASSERT_EQ(r->error_region_count, 0); + ASSERT_NULL(r->error_ranges); + cbm_free_result(r); + PASS(); +} + /* #961 inverse guard: a clean C file must not gain duplicate or phantom * defs from the recovery path (it only engages on raw-parse ERROR regions). */ TEST(extract_c_clean_file_no_recovery_duplicates_issue961) { @@ -3865,6 +3998,9 @@ SUITE(extraction) { RUN_TEST(complexity_delegation_receivers_not_recursive_issue876); RUN_TEST(complexity_access_depth_and_params); RUN_TEST(extract_c_ifdef_split_brace_fn_recovered_issue961); + RUN_TEST(extract_cpp_preproc_signature_gap_issue946); + RUN_TEST(extract_cpp_preproc_macro_generated_callable_skipped_issue949); + RUN_TEST(extract_c_ifdef_split_brace_after_include_remapped_issue949); RUN_TEST(extract_c_clean_file_no_recovery_duplicates_issue961); RUN_TEST(walk_defs_no_truncation_over_4096_issue668); RUN_TEST(extract_rust_test_attr_marks_is_test_issue855);