fix(pipeline): guard env-access CONFIGURES and throws sources against…#982
Conversation
… directory-module collisions Follow-up to DeusData#787/DeusData#828, disclosed during DeusData#828's review: the same clobber class also affects two call sites DeusData#828 deliberately left out of scope. create_env_configures_for_file (pass_definitions.c) and resolve_file_throws (pass_parallel.c) both resolved their edge source via a direct cbm_gbuf_find_by_qn(enclosing_func_qn) lookup with no guard against the lookup landing on the shared Folder/Project node for directory-module languages (Java/Go) -- the exact DeusData#787 root cause, just unguarded at two more sites. A class-level construct (no enclosing function) has its enclosing_func_qn fall back to the MODULE QN, which for a directory-based module collides with the package's Folder node, conflating every same-package file's CONFIGURES/THROWS edges onto one arbitrary source. - create_env_configures_for_file: added the same cbm_pipeline_node_is_dir_container guard already used by the three DeusData#828-fixed finders, falling back to the per-file File node on a directory-container hit. - resolve_file_throws: was missing the `rel` parameter its sibling resolve_file_calls/usages/rw already carry, so it inlined a raw lookup instead of calling the shared, already-guarded find_source_node. Added `rel` and switched to find_source_node, matching its siblings exactly. Regression guard: tests/repro/repro_issue842.c, reproduce-first per the issue's request for "an DeusData#828-style multi-order red fixture": - CONFIGURES reproduces on a small 5-file sequential fixture (Go, since Java's System.getenv detection is separately broken -- extract_env_key_from_call reads a "function" field Java's method_invocation node doesn't have; noted in the test, out of scope here). - THROWS only reproduces past MIN_FILES_FOR_PARALLEL=50 (the worker-merge path) -- confirmed empirically that the small sequential fixture already attributes correctly even on unfixed HEAD, mirroring DeusData#787's own two-faced (sequential/parallel) bug. Padded with 60 filler classes, following repro_issue787.c's build_parallel_fixture precedent. RED on unfixed HEAD (both tests): 0 passed, 2 failed. GREEN with this fix: 2 passed, 0 failed. Full scripts/test.sh (5958 cases) passes clean with this change; scripts/lint.sh's two pre-existing failures (pass_lsp_cross.h parameter-name mismatch, extract_defs.c cppcheck AST parse) are unrelated and present identically on unfixed main. Fixes DeusData#842 Signed-off-by: Dhruvy0804 <dhruvgarg553@gmail.com>
|
Thanks. I have triaged this as a 0.9.1-rc bug fix. Because this touches pipeline edge emission and Makefile.cbm, review will be conservative: we need to confirm the build target change is only for the repro guard and that CONFIGURES/THROWS source resolution cannot reintroduce directory-module collisions. |
|
Merged — thank you! This is a model contribution: you picked up a review-disclosed follow-up (#842), mirrored the #828 guards exactly at the two remaining sites, built multi-order reproduce-first fixtures (including padding past the parallel threshold to catch the worker-merge face of the bug), and honestly scoped out the unrelated Java env-detection issue you found along the way. Please do file that one as its own issue — it deserves tracking. |
… directory-module collisions
Follow-up to #787/#828, disclosed during #828's review: the same clobber class also affects two call sites #828 deliberately left out of scope.
create_env_configures_for_file (pass_definitions.c) and resolve_file_throws (pass_parallel.c) both resolved their edge source via a direct cbm_gbuf_find_by_qn(enclosing_func_qn) lookup with no guard against the lookup landing on the shared Folder/Project node for directory-module languages (Java/Go) -- the exact #787 root cause, just unguarded at two more sites. A class-level construct (no enclosing function) has its enclosing_func_qn fall back to the MODULE QN, which for a directory-based module collides with the package's Folder node, conflating every same-package file's CONFIGURES/THROWS edges onto one arbitrary source.
relparameter its sibling resolve_file_calls/usages/rw already carry, so it inlined a raw lookup instead of calling the shared, already-guarded find_source_node. Addedreland switched to find_source_node, matching its siblings exactly.Regression guard: tests/repro/repro_issue842.c, reproduce-first per the issue's request for "an #828-style multi-order red fixture":
RED on unfixed HEAD (both tests): 0 passed, 2 failed. GREEN with this fix: 2 passed, 0 failed. Full scripts/test.sh (5958 cases) passes clean with this change; scripts/lint.sh's two pre-existing failures (pass_lsp_cross.h parameter-name mismatch, extract_defs.c cppcheck AST parse) are unrelated and present identically on unfixed main.
Fixes #842
What does this PR do?
Checklist
git commit -s) — required, CI rejectsunsigned commits (DCO, see CONTRIBUTING.md)
make -f Makefile.cbm test)make -f Makefile.cbm lint-ci)