From 11551434aaf998e9044722f4fbdbec83bcd39ebb Mon Sep 17 00:00:00 2001 From: Hugo Date: Mon, 5 Oct 2026 11:47:35 +0200 Subject: [PATCH 01/16] test(input): files from compile directories of their own, with a relative model When jobs are automatic, the default, files compile in parallel and each one changes the working directory of the whole process to its compile command's directory (#184). The changes interleave: a file can compile from another file's directory, and the analyzer can end in one of them, where a relative resource model or cache directory no longer resolves. check_resource_model_across_compile_directories generates 8 files, each in a directory of its own with the one header it includes through a relative -I, and a resource model, given by a relative path, that alone defines the leak every file has. It requires: - with automatic jobs, --jobs=1 and --jobs=4, three runs each with empty caches: the model loads, the 8 leaks and no other are reported, and the compile IR and resource caches are under the start directory; - the relative model and cache paths anchored to the start directory (--print-effective-config); - no compile with a changed working directory, under automatic jobs, for compile commands that need none (--timing). On main, automatic jobs fail every run ('unit6.h' file not found, no output), --jobs=4 writes caches under the compile directories, the paths stay relative, and automatic jobs change the directory for every file. Refs #184 --- run_test.py | 161 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 161 insertions(+) diff --git a/run_test.py b/run_test.py index 02c0eee..f1c0f10 100755 --- a/run_test.py +++ b/run_test.py @@ -2073,6 +2073,166 @@ def check_unknown_alloca_virtual_callback_escape() -> bool: return True +def check_resource_model_across_compile_directories() -> bool: + """ + A resource model given by a relative path is loaded, and used, whatever the job count, while + the files compile from compile-command directories of their own. Each directory holds the one + header its file includes through a relative -I, so a file compiles only from its own + directory; the analyzer must come back to the directory it started from, where the model and + the relative cache directories are. Every run starts with empty caches, and a leak that only + the model defines must be reported in every file. The relative paths must be anchored to the + start directory, and parallel jobs must not change the working directory for files whose + compile commands need no directory of their own. + """ + print("=== Testing the resource model across compile-command directories ===") + unit_count = 8 + ok = True + with tempfile.TemporaryDirectory(prefix="ct_resource_model_cwd_") as tmp: + root = Path(tmp) + project = root / "project" + entries = [] + for i in range(unit_count): + unit_dir = project / f"unit{i}" + (unit_dir / "include").mkdir(parents=True) + (unit_dir / "include" / f"unit{i}.h").write_text( + "void* ct_open_widget(void);\nvoid ct_close_widget(void* w);\n" + ) + source = unit_dir / f"unit{i}.c" + source.write_text( + f'#include "unit{i}.h"\n\n' + f"int leak_widget_{i}(void)\n{{\n void* w = ct_open_widget();\n" + " return w != 0;\n}\n\n" + f"int keep_widget_{i}(void)\n{{\n void* w = ct_open_widget();\n" + " ct_close_widget(w);\n return 0;\n}\n" + ) + entries.append( + { + "directory": str(unit_dir), + "file": str(source), + "arguments": ["cc", "-Iinclude", "-c", str(source), "-o", f"unit{i}.o"], + } + ) + compdb = project / "compile_commands.json" + compdb.write_text(json.dumps(entries, indent=2)) + run_dir = root / "run" + run_dir.mkdir() + (run_dir / "widget-model.txt").write_text( + "acquire_ret ct_open_widget Widget\nrelease_arg ct_close_widget 0 Widget\n" + ) + sources = [entry["file"] for entry in entries] + expected = {f"leak_widget_{i}" for i in range(unit_count)} + + analyzer = str(Path(RUN_CONFIG.analyzer).resolve()) + + def run_from_start_dir(args): + try: + return subprocess.run( + [analyzer, *args], + cwd=run_dir, + capture_output=True, + text=True, + timeout=RUN_CONFIG.analyzer_timeout, + ) + except subprocess.TimeoutExpired: + return None + + start_dir = run_dir.resolve() + result = run_from_start_dir( + [ + "--print-effective-config", + f"--compile-commands={compdb}", + "--resource-model=widget-model.txt", + "--compile-ir-cache-dir=caches-config/compile-ir", + sources[0], + ] + ) + printed = (result.stdout + result.stderr) if result else "" + for line in ( + f"resource-model: {start_dir / 'widget-model.txt'}", + f"compile-ir-cache-dir: {start_dir / 'caches-config' / 'compile-ir'}", + ): + if line in printed: + print(f" ✅ effective configuration: {line.split(':')[0]} anchored to start") + else: + print(f" ❌ effective configuration lacks '{line}'") + ok = False + + absolute_compdb = project / "compile_commands_absolute.json" + absolute_compdb.write_text( + json.dumps( + [ + {**entry, "arguments": ["cc", f"-I{Path(entry['directory']) / 'include'}", + *entry["arguments"][2:]]} + for entry in entries + ], + indent=2, + ) + ) + # Automatic jobs, the default, run in parallel too. + result = run_from_start_dir( + ["--timing", f"--compile-commands={absolute_compdb}", + "--compile-ir-cache-dir=caches-timing/compile-ir", *sources] + ) + timing = (result.stdout + result.stderr) if result else "" + if result is None or result.returncode != 0: + print(" ❌ automatic jobs with absolute include paths: run failed") + ok = False + elif "input.compiler.invoke.cwd" in timing: + print(" ❌ automatic jobs changed the working directory for absolute commands") + ok = False + else: + print(" ✅ automatic jobs kept the working directory for absolute commands") + + for mode in ([], ["--jobs=1"], ["--jobs=4"]): + label = mode[0] if mode else "default jobs" + for attempt in range(1, 4): + caches = f"caches-{label.replace('=', '').replace(' ', '-')}-{attempt}" + cmd = [ + analyzer, + "--format=json", + f"--compile-commands={compdb}", + "--resource-model=widget-model.txt", + f"--compile-ir-cache-dir={caches}/compile-ir", + f"--resource-summary-cache-dir={caches}/resource", + *mode, + *sources, + ] + result = run_from_start_dir(cmd[1:]) + if result is None: + print(f" ❌ {label}, run {attempt}: timed out") + ok = False + continue + problems = [] + if "cannot open model file" in result.stderr: + problems.append("the model failed to load") + try: + diagnostics = json.loads(result.stdout or "").get("diagnostics", []) + except json.JSONDecodeError: + diagnostics = None + problems.append(f"no JSON output (exit {result.returncode})") + if diagnostics is not None: + found = { + d.get("location", {}).get("function") + for d in diagnostics + if d.get("ruleId") == "ResourceLifetime.MissingRelease" + } + if found != expected: + problems.append(f"leaks reported in {sorted(found)}") + for cache in ("compile-ir", "resource"): + if not (run_dir / caches / cache).is_dir(): + problems.append(f"no {cache} cache under the start directory") + strays = sorted(str(p.relative_to(root)) for p in project.rglob(caches)) + if strays: + problems.append(f"caches under the compile directories: {strays}") + if problems: + print(f" ❌ {label}, run {attempt}: {'; '.join(problems)}") + ok = False + else: + print(f" ✅ {label}, run {attempt}: model used in {unit_count} files") + print() + return ok + + def check_resource_lifetime_cross_tu() -> bool: """ Regression: cross-TU resource summaries must propagate acquire/release effects @@ -5055,6 +5215,7 @@ def record_ok(ok: bool): check_exclude_dir_filter, check_multi_tu_folder_analysis, check_resource_lifetime_cross_tu, + check_resource_model_across_compile_directories, check_ownership_cross_tu, check_ownership_wrapper_metadata, check_uninitialized_cross_tu, From 372b3f303a426e76f778031b8cc0977132beea47 Mon Sep 17 00:00:00 2001 From: Hugo Date: Mon, 5 Oct 2026 11:53:12 +0200 Subject: [PATCH 02/16] fix(input): one lock for every compile and the whole working-directory change A compile that needs its compile command's directory changes the working directory of the whole process (#184). Three things let that leak into the rest of the run: - InputPipeline chose its path with config.jobs > 1, but jobs is 0 when automatic, the default: parallel loads then changed the directory for every file. It now uses resolveConfiguredJobs, moved out of AnalyzerApp into StackUsageAnalyzer for both, so parallel runs try a compile from the current directory first, as with an explicit job count. - The change was made before taking the compile lock, and the separate lock of the fallback covered fallbacks only. Two threads could save and restore the directory in an interleaved order, compile in a directory not their own, and leave the process in one. The compile lock is now taken before the change and held until it is undone; the fallback lock goes. - The resource, escape and buffer models, the compile IR and resource summary caches, the --dump-ir output and the compile database were opened through paths relative to whatever the directory was at that time. They are now anchored to the directory the analyzer starts from, once the arguments are parsed. A compile from its command's directory still changes the directory of the whole process while it runs; removing that needs compilerlib to compile against a working directory of its own. Refs #184 --- include/StackUsageAnalyzer.hpp | 4 ++++ src/StackUsageAnalyzer.cpp | 10 ++++++++++ src/analysis/InputPipeline.cpp | 14 +++++++------- src/app/AnalyzerApp.cpp | 9 --------- src/cli/ArgParser.cpp | 16 ++++++++++++++++ 5 files changed, 37 insertions(+), 16 deletions(-) diff --git a/include/StackUsageAnalyzer.hpp b/include/StackUsageAnalyzer.hpp index 7c9ccbb..845fc66 100644 --- a/include/StackUsageAnalyzer.hpp +++ b/include/StackUsageAnalyzer.hpp @@ -127,6 +127,10 @@ namespace ctrace::stack std::uint32_t reservedFlags : 17 = 0; }; + // The number of jobs the analysis runs: the configured count, or the hardware concurrency when + // jobs are automatic (jobs is then 0). + unsigned resolveConfiguredJobs(const AnalysisConfig& config); + // Per-function result struct FunctionResult { diff --git a/src/StackUsageAnalyzer.cpp b/src/StackUsageAnalyzer.cpp index 07a956b..f0ffa90 100644 --- a/src/StackUsageAnalyzer.cpp +++ b/src/StackUsageAnalyzer.cpp @@ -5,13 +5,23 @@ #include "analyzer/HotspotProfiler.hpp" #include "analysis/InputPipeline.hpp" +#include #include #include +#include #include namespace ctrace::stack { + unsigned resolveConfiguredJobs(const AnalysisConfig& config) + { + if (!config.jobsAuto) + return std::max(1u, config.jobs); + + const unsigned hw = std::thread::hardware_concurrency(); + return hw == 0 ? 1u : hw; + } AnalysisResult analyzeModule(llvm::Module& mod, const AnalysisConfig& config) { diff --git a/src/analysis/InputPipeline.cpp b/src/analysis/InputPipeline.cpp index e08272e..47d99de 100644 --- a/src/analysis/InputPipeline.cpp +++ b/src/analysis/InputPipeline.cpp @@ -38,12 +38,14 @@ namespace ctrace::stack::analysis { namespace { - std::mutex gCompileWorkingDirMutex; // compilerlib drives an in-process clang CompilerInstance. clang/LLVM keep global // mutable state (target registry, cl options, timers); two worker threads compiling // at once hung intermittently on the first compile of a run. Compilation is // serialised here; module analysis stays parallel, and the IR cache makes the - // compile step cheap on repeated runs. + // compile step cheap on repeated runs. A compile that needs its compile command's + // directory changes the working directory of the whole process: it holds the same + // lock from before that change until it is undone, so no other compile runs in a + // directory that is not its own, and two changes never interleave. std::mutex gCompileInvokeMutex; std::string makeAbsolutePath(const std::string& path) @@ -883,6 +885,7 @@ namespace ctrace::stack::analysis return compilerlib::compile(compileArgs, outputMode); } + std::lock_guard compileLock(gCompileInvokeMutex); std::string cwdError; ScopedCurrentPath cwdGuard(workingDir, cwdError); if (!cwdError.empty()) @@ -891,7 +894,6 @@ namespace ctrace::stack::analysis return std::nullopt; } const ScopedHotspot hotspot(config.timing, "input.compiler.invoke.cwd"); - std::lock_guard compileLock(gCompileInvokeMutex); return compilerlib::compile(compileArgs, outputMode); }; @@ -902,16 +904,14 @@ namespace ctrace::stack::analysis retriedWithWorkingDir = false; std::optional res; const bool hasWorkingDir = !workingDir.empty(); - if (config.jobs > 1 && hasWorkingDir) + if (resolveConfiguredJobs(config) > 1 && hasWorkingDir) { // Optimistic fast path for multi-job runs: most compdb commands use absolute // paths. res = compileWithOptionalWorkingDir(compileArgs, outputMode, false); if (!res || !res->success) { - // Fallback keeps correctness for relative include paths and avoids process - // cwd races. - std::lock_guard lock(gCompileWorkingDirMutex); + // Fallback keeps correctness for relative include paths. res = compileWithOptionalWorkingDir(compileArgs, outputMode, true); retriedWithWorkingDir = true; } diff --git a/src/app/AnalyzerApp.cpp b/src/app/AnalyzerApp.cpp index 7a037ed..58f5410 100644 --- a/src/app/AnalyzerApp.cpp +++ b/src/app/AnalyzerApp.cpp @@ -97,15 +97,6 @@ struct NormalizedPathFilters std::vector excludeDirs; }; -static unsigned resolveConfiguredJobs(const AnalysisConfig& cfg) -{ - if (!cfg.jobsAuto) - return std::max(1u, cfg.jobs); - - const unsigned hw = std::thread::hardware_concurrency(); - return hw == 0 ? 1u : hw; -} - #if defined(CTRACE_STACK_ANALYZER_HAS_PTHREAD) static std::size_t resolveParallelWorkerStackBytes() { diff --git a/src/cli/ArgParser.cpp b/src/cli/ArgParser.cpp index d267786..027c2d3 100644 --- a/src/cli/ArgParser.cpp +++ b/src/cli/ArgParser.cpp @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -1658,6 +1659,21 @@ namespace ctrace::stack::cli parsed.inputFilenames.emplace_back(std::move(argStr)); } + // The analysis opens these while other threads may be compiling from the directory of + // their compile command: they are anchored to the directory the analyzer starts from. + std::error_code startDirError; + const std::filesystem::path startDir = std::filesystem::current_path(startDirError); + if (!startDirError) + { + for (std::string* path : + {&cfg.escapeModelPath, &cfg.bufferModelPath, &cfg.resourceModelPath, + &cfg.resourceSummaryCacheDir, &cfg.compileIRCacheDir, &cfg.dumpIRPath, + &parsed.compileCommandsPath}) + { + *path = resolveConfigRelativePath(*path, startDir); + } + } + return result; } From 5ee76e8f7a941f8fcf468f25c50c7362f31d34ef Mon Sep 17 00:00:00 2001 From: Hugo Date: Wed, 7 Oct 2026 23:44:22 +0200 Subject: [PATCH 03/16] test(input): a header of the start directory must not replace the command's Review of #184: a compile command's relative -I resolves against its directory, but the analyzer first compiled from the current directory and only used the command's directory when that compile failed. When both directories hold the header, the start directory's one was used, with no error: on main with --jobs>1, and with automatic jobs since the fix for the working-directory race. check_resource_model_across_compile_directories now puts, in the start directory, a header of the same name for every file, one that drops the leak. Each mode must still report the 8 leaks. On 3b2260e, automatic jobs and --jobs=4 report none; --jobs=1 reports all 8. The check no longer requires automatic jobs to compile commands with absolute paths without changing the working directory: that was the behavior that picks the wrong header. Refs #184 --- run_test.py | 36 +++++++----------------------------- 1 file changed, 7 insertions(+), 29 deletions(-) diff --git a/run_test.py b/run_test.py index f1c0f10..82f3786 100755 --- a/run_test.py +++ b/run_test.py @@ -2081,8 +2081,8 @@ def check_resource_model_across_compile_directories() -> bool: directory; the analyzer must come back to the directory it started from, where the model and the relative cache directories are. Every run starts with empty caches, and a leak that only the model defines must be reported in every file. The relative paths must be anchored to the - start directory, and parallel jobs must not change the working directory for files whose - compile commands need no directory of their own. + start directory. The start directory also holds a header of the same name for every file, + one that drops the leak: a file compiled from there instead of its own directory loses it. """ print("=== Testing the resource model across compile-command directories ===") unit_count = 8 @@ -2115,7 +2115,11 @@ def check_resource_model_across_compile_directories() -> bool: compdb = project / "compile_commands.json" compdb.write_text(json.dumps(entries, indent=2)) run_dir = root / "run" - run_dir.mkdir() + (run_dir / "include").mkdir(parents=True) + for i in range(unit_count): + (run_dir / "include" / f"unit{i}.h").write_text( + "#define ct_open_widget() ((void*)0)\nvoid ct_close_widget(void* w);\n" + ) (run_dir / "widget-model.txt").write_text( "acquire_ret ct_open_widget Widget\nrelease_arg ct_close_widget 0 Widget\n" ) @@ -2157,32 +2161,6 @@ def run_from_start_dir(args): print(f" ❌ effective configuration lacks '{line}'") ok = False - absolute_compdb = project / "compile_commands_absolute.json" - absolute_compdb.write_text( - json.dumps( - [ - {**entry, "arguments": ["cc", f"-I{Path(entry['directory']) / 'include'}", - *entry["arguments"][2:]]} - for entry in entries - ], - indent=2, - ) - ) - # Automatic jobs, the default, run in parallel too. - result = run_from_start_dir( - ["--timing", f"--compile-commands={absolute_compdb}", - "--compile-ir-cache-dir=caches-timing/compile-ir", *sources] - ) - timing = (result.stdout + result.stderr) if result else "" - if result is None or result.returncode != 0: - print(" ❌ automatic jobs with absolute include paths: run failed") - ok = False - elif "input.compiler.invoke.cwd" in timing: - print(" ❌ automatic jobs changed the working directory for absolute commands") - ok = False - else: - print(" ✅ automatic jobs kept the working directory for absolute commands") - for mode in ([], ["--jobs=1"], ["--jobs=4"]): label = mode[0] if mode else "default jobs" for attempt in range(1, 4): From adb3d4e9f17f5fbce7b864d90704fb50893562eb Mon Sep 17 00:00:00 2001 From: Hugo Date: Wed, 7 Oct 2026 23:45:04 +0200 Subject: [PATCH 04/16] fix(input): compile every command in its own directory A compile command whose directory differs from the current one was first compiled from the current directory, and from its own only when that failed. Relative paths then resolved against the wrong directory without any error when both held the file: another header, another IR. This "optimistic fast path" (2289afb) predates the lock that serializes every compile (967be74), so it no longer ran anything in parallel. A command with a directory is now always compiled there, under the compile lock, whatever the job count. The retry and its log suffix go, and resolveConfiguredJobs returns to AnalyzerApp, its only user again. Refs #184 --- include/StackUsageAnalyzer.hpp | 4 -- src/StackUsageAnalyzer.cpp | 10 ----- src/analysis/InputPipeline.cpp | 79 +++++++++------------------------- src/app/AnalyzerApp.cpp | 9 ++++ 4 files changed, 29 insertions(+), 73 deletions(-) diff --git a/include/StackUsageAnalyzer.hpp b/include/StackUsageAnalyzer.hpp index 845fc66..7c9ccbb 100644 --- a/include/StackUsageAnalyzer.hpp +++ b/include/StackUsageAnalyzer.hpp @@ -127,10 +127,6 @@ namespace ctrace::stack std::uint32_t reservedFlags : 17 = 0; }; - // The number of jobs the analysis runs: the configured count, or the hardware concurrency when - // jobs are automatic (jobs is then 0). - unsigned resolveConfiguredJobs(const AnalysisConfig& config); - // Per-function result struct FunctionResult { diff --git a/src/StackUsageAnalyzer.cpp b/src/StackUsageAnalyzer.cpp index f0ffa90..07a956b 100644 --- a/src/StackUsageAnalyzer.cpp +++ b/src/StackUsageAnalyzer.cpp @@ -5,23 +5,13 @@ #include "analyzer/HotspotProfiler.hpp" #include "analysis/InputPipeline.hpp" -#include #include #include -#include #include namespace ctrace::stack { - unsigned resolveConfiguredJobs(const AnalysisConfig& config) - { - if (!config.jobsAuto) - return std::max(1u, config.jobs); - - const unsigned hw = std::thread::hardware_concurrency(); - return hw == 0 ? 1u : hw; - } AnalysisResult analyzeModule(llvm::Module& mod, const AnalysisConfig& config) { diff --git a/src/analysis/InputPipeline.cpp b/src/analysis/InputPipeline.cpp index 47d99de..e67e02f 100644 --- a/src/analysis/InputPipeline.cpp +++ b/src/analysis/InputPipeline.cpp @@ -874,18 +874,19 @@ namespace ctrace::stack::analysis const std::vector bitcodeArgs = buildBitcodeCompileArgs(args, tempBitcodePath); - auto compileWithOptionalWorkingDir = - [&](const std::vector& compileArgs, compilerlib::OutputMode outputMode, - bool useWorkingDir) -> std::optional + // A compile command's relative paths are relative to its directory: compile there, + // even when the current directory also resolves them, to another header. + auto compileInWorkingDir = + [&](const std::vector& compileArgs, + compilerlib::OutputMode outputMode) -> std::optional { - if (!useWorkingDir) + std::lock_guard compileLock(gCompileInvokeMutex); + if (workingDir.empty()) { const ScopedHotspot hotspot(config.timing, "input.compiler.invoke"); - std::lock_guard compileLock(gCompileInvokeMutex); return compilerlib::compile(compileArgs, outputMode); } - std::lock_guard compileLock(gCompileInvokeMutex); std::string cwdError; ScopedCurrentPath cwdGuard(workingDir, cwdError); if (!cwdError.empty()) @@ -897,32 +898,6 @@ namespace ctrace::stack::analysis return compilerlib::compile(compileArgs, outputMode); }; - auto compileWithConfiguredWorkingDir = - [&](const std::vector& compileArgs, compilerlib::OutputMode outputMode, - bool& retriedWithWorkingDir) -> std::optional - { - retriedWithWorkingDir = false; - std::optional res; - const bool hasWorkingDir = !workingDir.empty(); - if (resolveConfiguredJobs(config) > 1 && hasWorkingDir) - { - // Optimistic fast path for multi-job runs: most compdb commands use absolute - // paths. - res = compileWithOptionalWorkingDir(compileArgs, outputMode, false); - if (!res || !res->success) - { - // Fallback keeps correctness for relative include paths. - res = compileWithOptionalWorkingDir(compileArgs, outputMode, true); - retriedWithWorkingDir = true; - } - } - else - { - res = compileWithOptionalWorkingDir(compileArgs, outputMode, hasWorkingDir); - } - return res; - }; - if (cachePaths.enabled) { auto cached = [&]() -> std::optional @@ -1044,7 +1019,6 @@ namespace ctrace::stack::analysis } } - bool retriedWithWorkingDir = false; bool compiledViaBitcode = false; std::optional res; std::optional sourceSnapshot; @@ -1057,20 +1031,14 @@ namespace ctrace::stack::analysis preferBitcodeCompile ? bitcodeArgs : args; appendDependencyCaptureArgs(cacheCompileArgs, cachePaths.depFile); - bool retriedForDependencyCompile = false; - res = compileWithConfiguredWorkingDir(cacheCompileArgs, - preferBitcodeCompile - ? compilerlib::OutputMode::ToFile - : compilerlib::OutputMode::ToMemory, - retriedForDependencyCompile); - retriedWithWorkingDir = retriedForDependencyCompile; + res = compileInWorkingDir(cacheCompileArgs, + preferBitcodeCompile ? compilerlib::OutputMode::ToFile + : compilerlib::OutputMode::ToMemory); if (preferBitcodeCompile && (!res || !res->success)) { - bool retriedForFallbackCompile = false; - auto fallbackResult = compileWithConfiguredWorkingDir( - args, compilerlib::OutputMode::ToMemory, retriedForFallbackCompile); - retriedWithWorkingDir = retriedWithWorkingDir || retriedForFallbackCompile; + auto fallbackResult = + compileInWorkingDir(args, compilerlib::OutputMode::ToMemory); if (fallbackResult) res = std::move(fallbackResult); } @@ -1100,21 +1068,17 @@ namespace ctrace::stack::analysis } else { - res = compileWithConfiguredWorkingDir(preferBitcodeCompile ? bitcodeArgs : args, - preferBitcodeCompile - ? compilerlib::OutputMode::ToFile - : compilerlib::OutputMode::ToMemory, - retriedWithWorkingDir); + res = compileInWorkingDir(preferBitcodeCompile ? bitcodeArgs : args, + preferBitcodeCompile ? compilerlib::OutputMode::ToFile + : compilerlib::OutputMode::ToMemory); if (res && res->success) { compiledViaBitcode = preferBitcodeCompile; } else if (preferBitcodeCompile) { - bool retriedForFallbackCompile = false; - auto fallbackResult = compileWithConfiguredWorkingDir( - args, compilerlib::OutputMode::ToMemory, retriedForFallbackCompile); - retriedWithWorkingDir = retriedWithWorkingDir || retriedForFallbackCompile; + auto fallbackResult = + compileInWorkingDir(args, compilerlib::OutputMode::ToMemory); if (fallbackResult) res = std::move(fallbackResult); } @@ -1140,8 +1104,7 @@ namespace ctrace::stack::analysis auto ms = std::chrono::duration_cast(compileEnd - compileStart) .count(); - coretrace::log(coretrace::Level::Info, "Compilation done in {} ms{}\n", ms, - retriedWithWorkingDir ? " (retry with working directory)" : ""); + coretrace::log(coretrace::Level::Info, "Compilation done in {} ms\n", ms); } std::string llvmIRForCache; @@ -1195,10 +1158,8 @@ namespace ctrace::stack::analysis { if (compiledViaBitcode) { - bool retriedForTextFallback = false; - auto textFallback = compileWithConfiguredWorkingDir( - args, compilerlib::OutputMode::ToMemory, retriedForTextFallback); - retriedWithWorkingDir = retriedWithWorkingDir || retriedForTextFallback; + auto textFallback = + compileInWorkingDir(args, compilerlib::OutputMode::ToMemory); if (!textFallback) return result; res = std::move(textFallback); diff --git a/src/app/AnalyzerApp.cpp b/src/app/AnalyzerApp.cpp index 58f5410..7a037ed 100644 --- a/src/app/AnalyzerApp.cpp +++ b/src/app/AnalyzerApp.cpp @@ -97,6 +97,15 @@ struct NormalizedPathFilters std::vector excludeDirs; }; +static unsigned resolveConfiguredJobs(const AnalysisConfig& cfg) +{ + if (!cfg.jobsAuto) + return std::max(1u, cfg.jobs); + + const unsigned hw = std::thread::hardware_concurrency(); + return hw == 0 ? 1u : hw; +} + #if defined(CTRACE_STACK_ANALYZER_HAS_PTHREAD) static std::size_t resolveParallelWorkerStackBytes() { From 6016573a9b775b119e938aca16a7bbb504fea0e3 Mon Sep 17 00:00:00 2001 From: Hugo Date: Wed, 7 Oct 2026 23:45:23 +0200 Subject: [PATCH 05/16] test(cli): a requested model that cannot be read must fail the run Review of #184: each analysis loads its model itself, logs a failure to stderr and goes on without it. A resource, escape or buffer model given on the command line but missing ends with code 0 and no finding, and run_code_analysis.py reports "CI gate passed". check_unreadable_model_fails gives each of the three options a missing file and a directory: the analyzer must exit with a nonzero code naming the path. run_code_analysis.py given a missing resource model must fail. On 274c1c1 all six runs exit with 0 and the CI script passes. A model that can be read but is malformed keeps its current handling (check_escape_model_rejects_unsupported_brackets); it is a separate question. Refs #184 --- run_test.py | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/run_test.py b/run_test.py index 82f3786..f59aece 100755 --- a/run_test.py +++ b/run_test.py @@ -3501,6 +3501,43 @@ def check_escape_model_rejects_unsupported_brackets() -> bool: return True +def check_unreadable_model_fails() -> bool: + """ + A model the command line asks for, but that cannot be read, is a configuration error: the + analyzer must stop with a nonzero code naming the file, and run_code_analysis.py must fail, + instead of analyzing without the model and reporting success. + """ + print("=== Testing that a requested model that cannot be read fails the run ===") + ok = True + with tempfile.TemporaryDirectory(prefix="ct_unreadable_model_") as tmp: + tmpdir = Path(tmp) + source = tmpdir / "leak.c" + source.write_text("void* acquire_handle(void);\nvoid leak(void) { (void)acquire_handle(); }\n") + missing = tmpdir / "missing-model.txt" + for option in ("--resource-model", "--escape-model", "--buffer-model"): + for path, what in ((missing, "missing file"), (tmpdir, "directory")): + result = run_analyzer_uncached([f"{option}={path}", str(source)]) + output = (result.stdout or "") + (result.stderr or "") + if result.returncode != 0 and str(path) in output: + print(f" ✅ {option}, {what}: code {result.returncode}, file named") + else: + print(f" ❌ {option}, {what}: code {result.returncode}, file named: {str(path) in output}") + ok = False + script = _run_ci_script( + ["--analyzer", str(RUN_CONFIG.analyzer), "--fail-on", "warning", + f"--analyzer-arg=--resource-model={missing}", str(source)], + jobs=1, + ) + if script.returncode != 0: + print(f" ✅ run_code_analysis.py with a missing model: code {script.returncode}") + else: + print(" ❌ run_code_analysis.py passed with a missing model") + print((script.stdout or "") + (script.stderr or "")) + ok = False + print() + return ok + + def check_docker_entrypoint_guardrails() -> bool: """ Regression: docker wrapper should only create compatibility symlinks under @@ -5207,6 +5244,7 @@ def record_ok(ok: bool): check_noreturn_cross_tu, check_use_after_free_advanced_inter_tu, check_escape_model_rejects_unsupported_brackets, + check_unreadable_model_fails, check_human_vs_json_parity, check_diagnostic_rule_coverage_regression, check_diagnostic_cwe_coverage, From e2e9f875e98b4dc4719f90e7cdc5829e99deeb66 Mon Sep 17 00:00:00 2001 From: Hugo Date: Wed, 7 Oct 2026 23:45:58 +0200 Subject: [PATCH 06/16] fix(app): stop when a requested model file cannot be read The resource, escape and buffer models are read by the analyses that use them, which log a failure and run without the model. A missing model then passed for a run that found nothing, and run_code_analysis.py for a passing CI gate. Before any input is loaded, every model given on the command line or in the config file must now be a readable file; otherwise the run stops with "Cannot read model file: " and exit code 1, which the CI script already turns into a failure. This check covers missing and unreadable files only. A model that can be read but holds malformed rules is still reported on stderr and skipped by the analysis: a separate question, noted in the README. Refs #184 --- README.md | 1 + src/app/AnalyzerApp.cpp | 24 ++++++++++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/README.md b/README.md index 5476cb8..caf8c04 100644 --- a/README.md +++ b/README.md @@ -249,6 +249,7 @@ Ready-to-adapt workflow examples: --escape-model= loads external noescape rules for stack pointer escape analysis (`noescape_arg`) --buffer-model= loads external buffer write rules for copy/string overflow checks (`bounded_write`/`unbounded_write`) --resource-model= loads external acquire/release rules for generic resource lifetime checks +A model path given to one of these three options that is not a readable file stops the run with an error (exit code 1). A model that can be read but holds malformed rules is reported on stderr, and the analysis runs without it. --resource-cross-tu enables cross-TU resource summaries for resource lifetime analysis (default: on) --no-resource-cross-tu disables cross-TU resource summaries --resource-summary-cache-dir= sets cache directory for cross-TU resource summaries (default: .cache/resource-lifetime) diff --git a/src/app/AnalyzerApp.cpp b/src/app/AnalyzerApp.cpp index 7a037ed..ed68226 100644 --- a/src/app/AnalyzerApp.cpp +++ b/src/app/AnalyzerApp.cpp @@ -835,6 +835,26 @@ static AppStatus configureDumpIRPath(const std::vector& inputFilena return AppStatus::success(); } +// A model the command line asks for is read by the analyses that use it, which only log a +// failure and run without it: an unreadable model would pass for a run that found nothing. +static AppStatus checkRequestedModelsReadable(const AnalysisConfig& cfg) +{ + const std::pair models[] = { + {"resource", &cfg.resourceModelPath}, + {"stack escape", &cfg.escapeModelPath}, + {"buffer", &cfg.bufferModelPath}, + }; + for (const auto& [kind, path] : models) + { + if (path->empty()) + continue; + std::error_code fsErr; + if (!std::filesystem::is_regular_file(*path, fsErr) || !std::ifstream(*path)) + return AppStatus::failure(std::string("Cannot read ") + kind + " model file: " + *path); + } + return AppStatus::success(); +} + static void printInterprocStatus(const AnalysisConfig& cfg, std::size_t inputCount, bool needsCrossTUResourceSummaries, bool needsCrossTUUninitializedSummaries, @@ -2616,6 +2636,10 @@ class RunPlanBuilder plan.sarifBaseDir = std::move(parsedArgs_.sarifBaseDir); plan.sarifOutPath = std::move(parsedArgs_.sarifOutPath); + AppStatus modelStatus = checkRequestedModelsReadable(plan.cfg); + if (!modelStatus.isOk()) + return AppResult::failure(std::move(modelStatus.error)); + if (parsedArgs_.compileCommandsExplicit) { AppStatus loadStatus = From d3824fdd8cd1c934b7a47e1832be7b9e26f6d92b Mon Sep 17 00:00:00 2001 From: Hugo Date: Thu, 8 Oct 2026 18:16:37 +0200 Subject: [PATCH 07/16] test(input): relative input paths with parallel compiles from other directories Review of #187: a compile command with a directory of its own is now always compiled there, and the change of directory holds for the whole process. Meanwhile, another thread resolves its own relative input path against that directory, to find its compile command and to compile it: with 24 files given by relative paths, --jobs=4 fails with "No compile command found for: src/unit03.c", where --jobs=1 and main succeed. check_resource_model_across_compile_directories now runs every mode with the files given by absolute paths, then by paths relative to the start directory, each run with empty caches of its own. Each must report the 8 leaks and name each file as it was given (#124). On e2e9f87, 5 of the 6 parallel runs with relative paths fail; --jobs=1 and absolute paths pass. Refs #184 --- run_test.py | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/run_test.py b/run_test.py index f59aece..893f6ba 100755 --- a/run_test.py +++ b/run_test.py @@ -2083,6 +2083,9 @@ def check_resource_model_across_compile_directories() -> bool: the model defines must be reported in every file. The relative paths must be anchored to the start directory. The start directory also holds a header of the same name for every file, one that drops the leak: a file compiled from there instead of its own directory loses it. + The files are given by absolute paths, then by paths relative to the start directory, which + must resolve there whatever directory another file is compiling from at the time; the + results name each file as it was given. """ print("=== Testing the resource model across compile-command directories ===") unit_count = 8 @@ -2161,10 +2164,16 @@ def run_from_start_dir(args): print(f" ❌ effective configuration lacks '{line}'") ok = False - for mode in ([], ["--jobs=1"], ["--jobs=4"]): - label = mode[0] if mode else "default jobs" + relative_sources = [os.path.relpath(source, run_dir) for source in sources] + runs = [ + (paths, given, mode) + for paths, given in (("absolute paths", sources), ("relative paths", relative_sources)) + for mode in ([], ["--jobs=1"], ["--jobs=4"]) + ] + for paths, given, mode in runs: + label = f"{paths}, {mode[0] if mode else 'default jobs'}" for attempt in range(1, 4): - caches = f"caches-{label.replace('=', '').replace(' ', '-')}-{attempt}" + caches = f"caches-{label.replace('=', '').replace(', ', '-').replace(' ', '-')}-{attempt}" cmd = [ analyzer, "--format=json", @@ -2173,7 +2182,7 @@ def run_from_start_dir(args): f"--compile-ir-cache-dir={caches}/compile-ir", f"--resource-summary-cache-dir={caches}/resource", *mode, - *sources, + *given, ] result = run_from_start_dir(cmd[1:]) if result is None: @@ -2196,6 +2205,13 @@ def run_from_start_dir(args): } if found != expected: problems.append(f"leaks reported in {sorted(found)}") + files = { + d.get("location", {}).get("file") + for d in diagnostics + if d.get("ruleId") == "ResourceLifetime.MissingRelease" + } + if found == expected and files != set(given): + problems.append(f"leaks not named as given: {sorted(files)}") for cache in ("compile-ir", "resource"): if not (run_dir / caches / cache).is_dir(): problems.append(f"no {cache} cache under the start directory") From c5911fb00cad4f218bf876b31ad4031168a4cc9d Mon Sep 17 00:00:00 2001 From: Hugo Date: Thu, 8 Oct 2026 18:18:36 +0200 Subject: [PATCH 08/16] fix(input): resolve relative input paths from the directory the run started in A compile command with a directory of its own is compiled from there, and the change of directory holds for the whole process while it lasts. loadModuleForAnalysis resolved a relative input against the current directory: to find its compile command, to compile it, for its compile IR cache key and to read it. Another thread could then resolve it from a compile command's directory, and with --jobs=4 an input given as src/unit03.c had no compile command. The directory the analyzer starts from is now recorded with the other anchored paths, as AnalysisConfig::inputBaseDir, before any input is loaded. Every access to an input goes through its path resolved from there. The path given is kept for messages and results (#124), and a file without a compile command is still compiled under the name given: no other thread changes the directory while a compile holds the lock. Refs #184 --- include/StackUsageAnalyzer.hpp | 2 ++ src/analysis/InputPipeline.cpp | 46 ++++++++++++++++------------------ src/cli/ArgParser.cpp | 1 + 3 files changed, 25 insertions(+), 24 deletions(-) diff --git a/include/StackUsageAnalyzer.hpp b/include/StackUsageAnalyzer.hpp index 7c9ccbb..27d9988 100644 --- a/include/StackUsageAnalyzer.hpp +++ b/include/StackUsageAnalyzer.hpp @@ -93,6 +93,8 @@ namespace ctrace::stack std::string smtSecondaryBackend; std::string smtBackend = "interval"; std::string dumpIRPath; + // The directory relative input paths resolve from; empty: the current one at load. + std::string inputBaseDir; std::string escapeModelPath; std::string bufferModelPath; std::string resourceModelPath; diff --git a/src/analysis/InputPipeline.cpp b/src/analysis/InputPipeline.cpp index e67e02f..a7bddfa 100644 --- a/src/analysis/InputPipeline.cpp +++ b/src/analysis/InputPipeline.cpp @@ -48,15 +48,6 @@ namespace ctrace::stack::analysis // directory that is not its own, and two changes never interleave. std::mutex gCompileInvokeMutex; - std::string makeAbsolutePath(const std::string& path) - { - std::error_code ec; - std::filesystem::path absPath = std::filesystem::absolute(path, ec); - if (ec) - return path; - return absPath.lexically_normal().generic_string(); - } - void appendIfMissing(std::vector& args, const std::string& flag) { if (std::find(args.begin(), args.end(), flag) == args.end()) @@ -661,14 +652,15 @@ namespace ctrace::stack::analysis return true; } - bool buildCompileArgs(const std::string& filename, LanguageType language, - const AnalysisConfig& config, std::vector& args, - std::string& workingDir, std::string& error) + bool buildCompileArgs(const std::string& filename, const std::string& inputPath, + LanguageType language, const AnalysisConfig& config, + std::vector& args, std::string& workingDir, + std::string& error) { const CompileCommand* command = nullptr; if (config.compilationDatabase) { - command = config.compilationDatabase->findCommandForFile(filename); + command = config.compilationDatabase->findCommandForFile(inputPath); } if (command) @@ -713,7 +705,7 @@ namespace ctrace::stack::analysis args.push_back("-g"); appendIfMissing(args, "-fno-discard-value-names"); const bool useAbsolutePath = (command != nullptr); - args.push_back(useAbsolutePath ? makeAbsolutePath(filename) : filename); + args.push_back(useAbsolutePath ? inputPath : filename); return true; } @@ -822,13 +814,19 @@ namespace ctrace::stack::analysis using ctrace::stack::analyzer::ScopedHotspot; ModuleLoadResult result; const ScopedHotspot totalHotspot(config.timing, "input.load_module.total"); + // A relative input resolves from the directory the analysis started in, not from the + // current one: another thread may be compiling from its compile command's directory, + // which the whole process shares meanwhile. std::error_code cwdErr; - std::filesystem::path baseDir = std::filesystem::current_path(cwdErr); + const std::filesystem::path baseDir = config.inputBaseDir.empty() + ? std::filesystem::current_path(cwdErr) + : std::filesystem::path(config.inputBaseDir); + const std::string inputPath = makeAbsolutePathFrom(filename, baseDir.string()); using Clock = std::chrono::steady_clock; auto compileStart = Clock::now(); { const ScopedHotspot hotspot(config.timing, "input.detect_language"); - result.language = detectLanguageFromFile(filename, ctx); + result.language = detectLanguageFromFile(inputPath, ctx); } if (result.language == LanguageType::Unknown) @@ -846,8 +844,8 @@ namespace ctrace::stack::analysis bool compileArgsReady = false; { const ScopedHotspot hotspot(config.timing, "input.build_compile_args"); - compileArgsReady = buildCompileArgs(filename, result.language, config, args, - workingDir, compileError); + compileArgsReady = buildCompileArgs(filename, inputPath, result.language, config, + args, workingDir, compileError); } if (!compileArgsReady) { @@ -859,7 +857,7 @@ namespace ctrace::stack::analysis coretrace::log(coretrace::Level::Info, "Compiling {}...\n", filename); const bool preferBitcodeCompile = (config.compileIRFormat == CompileIRFormat::BC); const CompileIRCachePaths cachePaths = - buildCompileIRCachePaths(config, filename, result.language, args, workingDir); + buildCompileIRCachePaths(config, inputPath, result.language, args, workingDir); std::error_code tempDirErr; std::filesystem::path tempDir = std::filesystem::temp_directory_path(tempDirErr); @@ -1051,7 +1049,7 @@ namespace ctrace::stack::analysis "input.cache.parse_dependencies"); return parseDepfileDependencies(cachePaths.depFile, workingDir); }(); - const std::string sourcePath = makeAbsolutePathFrom(filename, workingDir); + const std::string& sourcePath = inputPath; sourceSnapshot = [&]() -> std::optional { const ScopedHotspot hotspot(config.timing, @@ -1257,7 +1255,7 @@ namespace ctrace::stack::analysis cwdErr ? std::string() : baseDir.lexically_normal().generic_string(); const CompileIRCachePaths cachePaths = isTextIRInput - ? buildCompileIRCachePaths(config, filename, result.language, {}, cacheWorkingDir) + ? buildCompileIRCachePaths(config, inputPath, result.language, {}, cacheWorkingDir) : CompileIRCachePaths{}; if (isTextIRInput && cachePaths.enabled) @@ -1359,7 +1357,7 @@ namespace ctrace::stack::analysis const auto parseStart = Clock::now(); { const ScopedHotspot hotspot(config.timing, "input.parse_ir_file"); - result.module = llvm::parseIRFile(filename, err, ctx); + result.module = llvm::parseIRFile(inputPath, err, ctx); } if (config.timing) { @@ -1373,10 +1371,10 @@ namespace ctrace::stack::analysis { if (isTextIRInput && cachePaths.enabled) { - if (const auto sourceSnapshot = captureFileSnapshot(filename)) + if (const auto sourceSnapshot = captureFileSnapshot(inputPath)) { std::string sourceIR; - (void)readTextFile(filename, sourceIR); + (void)readTextFile(inputPath, sourceIR); std::string llvmBitcode; llvm::raw_string_ostream bitcodeStream(llvmBitcode); diff --git a/src/cli/ArgParser.cpp b/src/cli/ArgParser.cpp index 027c2d3..9348d6a 100644 --- a/src/cli/ArgParser.cpp +++ b/src/cli/ArgParser.cpp @@ -1672,6 +1672,7 @@ namespace ctrace::stack::cli { *path = resolveConfigRelativePath(*path, startDir); } + cfg.inputBaseDir = startDir.string(); } return result; From 934ba5c5913a6174c2fb655d5705a3fadf4b652a Mon Sep 17 00:00:00 2001 From: Hugo Date: Thu, 8 Oct 2026 18:19:37 +0200 Subject: [PATCH 09/16] test(input): a compile IR cache entry of the previous schema must be compiled again Review of #187: the compile IR cache keeps the IR that the previous pipeline built from the current directory, with a header of the same name as the compile command's own, under schema compile-ir-cache-v2. A run that now compiles from the command's directory reused it: 7 where a run with an empty cache gives 42. check_compile_ir_cache_rebuilds_previous_schema rewrites the entry of the current key as the previous pipeline left it: the previous schema, and the IR built with the start directory's header, with the sources and dependencies left valid. The next run must compile again, give 42 and rewrite the entry, and the run after it must reuse that entry. An entry stored under a previous key is never looked up again; the schema in the key covers it, not this check. On c5911fb the stale entry is reused: 7. Refs #184 --- run_test.py | 78 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 78 insertions(+) diff --git a/run_test.py b/run_test.py index 893f6ba..538241e 100755 --- a/run_test.py +++ b/run_test.py @@ -3517,6 +3517,83 @@ def check_escape_model_rejects_unsupported_brackets() -> bool: return True +def check_compile_ir_cache_rebuilds_previous_schema() -> bool: + """ + A file used to be compiled from the current directory, and from its compile command's only + when that failed: with a header of the same name in both, the compile IR cache kept the IR + built with the wrong one. Compiling from the command's directory changes the IR of an + unchanged command, so the cache schema changed. + + The schema is in two places: the cache key, and the metadata of each entry. An entry stored + under a previous key is never looked up again; this check covers the other one. It rewrites + the entry of the current key as the previous pipeline left it: the previous schema, and the + IR built with the start directory's header (7, where the command's header gives 42), with + the sources and dependencies left valid. The next run must compile again and give 42, and + rewrite the entry; the run after it must reuse that entry and give 42 again. + """ + print("=== Testing that compile IR cached under a previous schema is rebuilt ===") + ok = True + with tempfile.TemporaryDirectory(prefix="ct_compile_ir_schema_") as tmp: + root = Path(tmp) + (root / "start" / "include").mkdir(parents=True) + (root / "start" / "include" / "config.h").write_text("#define VALUE 7\n") + (root / "project" / "build" / "include").mkdir(parents=True) + (root / "project" / "build" / "include" / "config.h").write_text("#define VALUE 42\n") + source = root / "project" / "value.c" + source.write_text("#include \nint value(void) { return VALUE; }\n") + compdb = root / "project" / "compile_commands.json" + compdb.write_text(json.dumps([{ + "directory": str(root / "project" / "build"), + "file": str(source), + "arguments": ["cc", "-Iinclude", "-c", str(source)], + }])) + cache = root / "compile-ir" + analyzer = str(Path(RUN_CONFIG.analyzer).resolve()) + + def run(name): + dump = root / f"ir-{name}" + try: + result = subprocess.run( + [analyzer, "--timing", f"--compile-commands={compdb}", + "--compile-ir-format=ll", f"--compile-ir-cache-dir={cache}", + f"--dump-ir={dump}/", str(source)], + cwd=root / "start", capture_output=True, text=True, + timeout=RUN_CONFIG.analyzer_timeout, + ) + except subprocess.TimeoutExpired: + return None, False + returned = re.findall(r"ret i32 (\d+)", "".join(p.read_text() for p in dump.glob("*.ll"))) + return (returned[0] if returned else None), "Compilation cache hit" in result.stderr + + value, _ = run("empty") + entries = sorted(cache.glob("*.json")) + if value != "42" or len(entries) != 1: + print(f" ❌ with an empty cache: returns {value}, {len(entries)} cache entries") + return False + metadata = json.loads(entries[0].read_text()) + current = metadata["schema"] + ir_file = entries[0].with_suffix(".ll") + metadata["schema"] = "compile-ir-cache-v2" + entries[0].write_text(json.dumps(metadata)) + ir_file.write_text(ir_file.read_text().replace("ret i32 42", "ret i32 7")) + + value, hit = run("previous") + rewritten = json.loads(entries[0].read_text())["schema"] + if value == "42" and not hit and rewritten == current: + print(" ✅ an entry of the previous schema is compiled again and rewritten") + else: + ok = False + print(f" ❌ with the previous schema: returns {value}, cache hit {hit}, entry schema {rewritten}") + value, hit = run("rebuilt") + if value == "42" and hit: + print(" ✅ the rewritten entry is reused") + else: + ok = False + print(f" ❌ after the rebuild: returns {value}, cache hit {hit}") + print() + return ok + + def check_unreadable_model_fails() -> bool: """ A model the command line asks for, but that cannot be read, is a configuration error: the @@ -5260,6 +5337,7 @@ def record_ok(ok: bool): check_noreturn_cross_tu, check_use_after_free_advanced_inter_tu, check_escape_model_rejects_unsupported_brackets, + check_compile_ir_cache_rebuilds_previous_schema, check_unreadable_model_fails, check_human_vs_json_parity, check_diagnostic_rule_coverage_regression, From 7962221284abefcee4fdcbbd29d681f07eae34cf Mon Sep 17 00:00:00 2001 From: Hugo Date: Thu, 8 Oct 2026 18:19:47 +0200 Subject: [PATCH 10/16] fix(input): move the compile IR cache to schema v3 A compile command with a directory of its own is now always compiled from there, so an unchanged command can give other IR than before: the IR of a file whose header of the same name also sits in the start directory. The compile IR cache kept the previous IR under the same key and accepted it, and the fix had no effect on a cache filled before it. The schema becomes compile-ir-cache-v3, in the cache key and in the metadata of each entry: previous entries are no longer looked up, and an entry whose metadata names the previous schema is compiled again. Refs #184 --- src/analysis/InputPipeline.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/analysis/InputPipeline.cpp b/src/analysis/InputPipeline.cpp index a7bddfa..b19df82 100644 --- a/src/analysis/InputPipeline.cpp +++ b/src/analysis/InputPipeline.cpp @@ -192,7 +192,7 @@ namespace ctrace::stack::analysis } } - constexpr llvm::StringLiteral kCompileIRCacheSchema = "compile-ir-cache-v2"; + constexpr llvm::StringLiteral kCompileIRCacheSchema = "compile-ir-cache-v3"; struct FileSnapshot { From b78104ab91e1271deab37a36d4f28a7471298418 Mon Sep 17 00:00:00 2001 From: Hugo Date: Tue, 6 Oct 2026 07:19:56 +0200 Subject: [PATCH 11/16] test(ownership): summarize several parameters with four solves, as one per parameter computeSummary solves the whole function 4 * (parameters + pointee paths) + 1 times (#184): 101 solves for a function with 25, which makes the self-analysis run for minutes once the resource model loads. - A reference, written with the public solve(), builds the transformers as computeSummary does today: one solve per parameter and entry state, the other parameters Owned, and one with every parameter Owned for the fresh resources. computeSummary must give the same transformers, and be incomplete exactly when the reference is, with aliases, pointee paths, a loop, exceptional exits, and at the smallest budget the reference converges with and just below it. - Fresh resources beside several parameters: a returned one and one handed through a pointee, guaranteed then conditional. - Work: SummaryWork counts the solves and block visits of a summary. Six parameters and pointees take four solves and at most four times the block visits of one solve; a function without parameters, one. Refs #184 --- test/unit/ownership_engine_unit_tests.cpp | 261 ++++++++++++++++++++++ 1 file changed, 261 insertions(+) diff --git a/test/unit/ownership_engine_unit_tests.cpp b/test/unit/ownership_engine_unit_tests.cpp index decd634..0b7b0d9 100644 --- a/test/unit/ownership_engine_unit_tests.cpp +++ b/test/unit/ownership_engine_unit_tests.cpp @@ -5,7 +5,9 @@ #include "analysis/ownership/OwnershipEngine.hpp" #include "analysis/ownership/OwnershipFacts.hpp" +#include #include +#include #include namespace @@ -561,6 +563,263 @@ namespace } return r.failures == 0; } + // ---- summaries of several parameters at once ------------------------------------------- + + // The transformers of the parameters as computeSummary built them before it solved the + // parameters together: one solve per parameter and entry state, the other parameters Owned, + // and one solve with every parameter Owned for the fresh resources. + struct ReferenceTransformers + { + std::map normalParams; + std::map exceptionalParams; + std::map normalPointees; + std::map exceptionalPointees; + bool anyNormal = false; + bool anyExceptional = false; + bool incomplete = false; + }; + + AbstractState summaryEntry(const OwnershipFacts& f, std::size_t paramIndex, OwnState state) + { + const std::size_t total = f.paramLocations.size() + f.pointeeLocations.size(); + AbstractState entry = AbstractState::entry(2u * (f.siteCount + total), f.locations.size()); + for (std::size_t p = 0; p < total; ++p) + { + const ResourceId r = newInstanceOf(f.siteCount + static_cast(p)); + const LocationId loc = p < f.paramLocations.size() + ? f.paramLocations[p].second + : f.pointeeLocations[p - f.paramLocations.size()].second; + entry.locations[loc].add(r); + entry.resources[r] = StateSet::of(p == paramIndex ? state : OwnState::Owned); + } + return entry; + } + + ReferenceTransformers referenceTransformers(const OwnershipFacts& f, unsigned limit) + { + ReferenceTransformers ref; + const std::size_t total = f.paramLocations.size() + f.pointeeLocations.size(); + for (std::size_t p = 0; p < total; ++p) + { + const ResourceId r = newInstanceOf(f.siteCount + static_cast(p)); + ParamTransformer normal{}; + ParamTransformer exceptional{}; + for (const OwnState state : + {OwnState::NotOwned, OwnState::Owned, OwnState::Released, OwnState::Escaped}) + { + const OwnershipResult res = solve(f, summaryEntry(f, p, state), limit); + if (res.incomplete) + { + ref.incomplete = true; + return ref; + } + for (const ExitRecord& exit : res.exits) + { + ParamTransformer& t = exit.exceptional ? exceptional : normal; + t[static_cast(state)] |= exit.state.resources[r]; + if (exit.state.uncertain[r]) + t.uncertainInputs |= StateSet::of(state); + (exit.exceptional ? ref.anyExceptional : ref.anyNormal) = true; + } + } + if (p < f.paramLocations.size()) + { + ref.normalParams[f.paramLocations[p].first] = normal; + ref.exceptionalParams[f.paramLocations[p].first] = exceptional; + } + else + { + const ArgPath& path = f.pointeeLocations[p - f.paramLocations.size()].first; + ref.normalPointees[path] = normal; + ref.exceptionalPointees[path] = exceptional; + } + } + const OwnershipResult fresh = solve(f, summaryEntry(f, total, OwnState::Owned), limit); + if (fresh.incomplete) + { + ref.incomplete = true; + return ref; + } + for (const ExitRecord& exit : fresh.exits) + (exit.exceptional ? ref.anyExceptional : ref.anyNormal) = true; + return ref; + } + + // computeSummary gives what one solve per parameter gives, and is incomplete exactly when + // that is. + void expectSameAsReference(TestReport& r, const OwnershipFacts& f, unsigned limit, + const std::string& name) + { + const ReferenceTransformers ref = referenceTransformers(f, limit); + const FunctionOwnershipSummary s = computeSummary(f, limit); + r.expect(s.incomplete == ref.incomplete, + "Joint summary: " + name + ": incomplete exactly when one solve per parameter is"); + if (s.incomplete || ref.incomplete) + return; + r.expect( + s.normal.params == ref.normalParams && s.exceptional.params == ref.exceptionalParams && + s.normal.pointeeParams == ref.normalPointees && + s.exceptional.pointeeParams == ref.exceptionalPointees && + s.normal.present == ref.anyNormal && s.exceptional.present == ref.anyExceptional, + "Joint summary: " + name + ": same transformers as one solve per parameter"); + } + + ArgPath pathOf(unsigned argIndex, std::uint64_t offset) + { + ArgPath path; + path.argIndex = argIndex; + path.offset = offset; + return path; + } + + // Parameters 0 and 1 by value in locations 0 and 1; the pointee of argument 2 at offset 0 in + // location 2, at offset 8 in location 3. Location 4 is a local. + OwnershipFacts severalParams(std::size_t blocks, std::vector edges) + { + OwnershipFacts f = facts(blocks, std::move(edges), 5, 1); + f.paramLocations = {{0, 0}, {1, 1}}; + f.locations[2].kind = LocationKind::ArgPointee; + f.locations[2].path = pathOf(2, 0); + f.locations[3].kind = LocationKind::ArgPointee; + f.locations[3].path = pathOf(2, 8); + f.pointeeLocations = {{pathOf(2, 0), 2}, {pathOf(2, 8), 3}}; + return f; + } + + Event unknownCallOn(LocationId loc) + { + Event e; + e.kind = Event::Kind::UnknownCall; + e.args = {loc}; + return e; + } + + bool testJointSummaries(TestReport& r) + { + { + // Aliases: parameter 0 is stored into the pointee of argument 2, then released + // through the local that copies it; parameter 1 goes to an unknown call. + OwnershipFacts f = severalParams(1, {}); + f.blocks[0].events = {copy(4, 0), copy(2, 0, /*strong=*/false), + release(4), unknownCallOn(1), + copy(3, 1, false), exit()}; + expectSameAsReference(r, f, 0, "aliases"); + } + { + // Pointee paths: offset 8 is released, offset 0 escapes through its address, and a + // fresh resource is acquired into the local. + OwnershipFacts f = severalParams(1, {}); + f.blocks[0].events = {release(3), addressEscape(2), acquire(0, 4), release(4), exit()}; + expectSameAsReference(r, f, 0, "pointee paths"); + } + { + // A loop that may release parameter 0 on each turn, overwrites parameter 1, and + // copies the pointee at offset 0 into the local. + OwnershipFacts f = severalParams(3, {edge(0, 1), edge(1, 1), edge(1, 2)}); + f.blocks[1].events = { + callWith(0, StateSet::of(OwnState::Owned) | StateSet::of(OwnState::Released)), + copy(4, 2, false), overwrite(1, true, false)}; + f.blocks[2].events = {release(4), exit()}; + expectSameAsReference(r, f, 0, "loop"); + } + { + // Exceptional exits: an unknown call on parameter 1 may throw before parameter 0 + // is released on the normal path; the pointee at offset 8 is released on both. + OwnershipFacts f = severalParams(3, {edge(0, 1), edge(0, 2, {release(3), exit(true)})}); + f.blocks[0].events = {unknownCallOn(1)}; + f.blocks[1].events = {release(0), release(3), exit()}; + expectSameAsReference(r, f, 0, "exceptional exits"); + } + { + // Convergence budget: below the smallest budget at which one solve per parameter + // converges, both are incomplete; at that budget, the joint solves converge too. + OwnershipFacts f = severalParams(4, {edge(0, 1), edge(1, 2), edge(2, 1), edge(2, 3)}); + f.blocks[1].events = {copy(4, 0, false), callWith(1, StateSet::of(OwnState::Released))}; + f.blocks[2].events = {callWith(4, StateSet::of(OwnState::Escaped)), copy(0, 1, false)}; + f.blocks[3].events = {exit()}; + unsigned smallest = 0; + for (unsigned limit = 1; limit <= 64 && smallest == 0; ++limit) + { + if (!referenceTransformers(f, limit).incomplete) + smallest = limit; + } + r.expect(smallest > 1, + "Joint summary: the budget case converges, after more than one visit"); + if (smallest > 1) + { + expectSameAsReference(r, f, smallest - 1, "below the smallest converging budget"); + expectSameAsReference(r, f, smallest, "at the smallest converging budget"); + } + } + { + // Fresh resources beside several parameters: one returned, one handed through the + // pointee of argument 2 at offset 0; parameter 0 is released, parameter 1 escapes. + OwnershipFacts f = severalParams(1, {}); + f.siteCount = 2; + f.siteKinds = {"Widget", "Socket"}; + f.locations[4].kind = LocationKind::Return; + f.blocks[0].events = {release(0), unknownCallOn(1), acquire(0, 4), + acquire(1, 2), ret(4), exit()}; + expectSameAsReference(r, f, 0, "fresh resources"); + const FunctionOwnershipSummary s = computeSummary(f); + r.expect(s.normal.returns.certainty == Certainty::Guaranteed && + s.normal.returns.kind == "Widget", + "Joint summary: a fresh resource returned beside several parameters"); + r.expect(s.normal.outArgs.count(pathOf(2, 0)) == 1 && + s.normal.outArgs.at(pathOf(2, 0)).certainty == Certainty::Guaranteed && + s.normal.outArgs.at(pathOf(2, 0)).kind == "Socket" && + s.normal.outArgs.count(pathOf(2, 8)) == 0, + "Joint summary: a fresh resource handed through one pointee only"); + + // The same acquisitions, conditional. + f.blocks[0].events = {release(0), + unknownCallOn(1), + acquire(0, 4, true, Certainty::Conditional), + acquire(1, 2, true, Certainty::Conditional), + ret(4), + exit()}; + const FunctionOwnershipSummary c = computeSummary(f); + r.expect(c.normal.returns.certainty == Certainty::Conditional && + c.normal.outArgs.at(pathOf(2, 0)).certainty == Certainty::Conditional, + "Joint summary: conditional fresh resources beside several parameters"); + } + return r.failures == 0; + } + + bool testJointSummaryWork(TestReport& r) + { + // Six parameters and pointees through a loop: four solves, one per entry state, instead + // of four per parameter and one more for the fresh resources. + OwnershipFacts f = severalParams(3, {edge(0, 1), edge(1, 1), edge(1, 2)}); + f.paramLocations.push_back({3, 4}); + f.locations.resize(6); + f.locations[5].kind = LocationKind::ArgPointee; + f.locations[5].path = pathOf(4, 0); + f.pointeeLocations.push_back({pathOf(4, 0), 5}); + f.blocks[1].events = {callWith(0, StateSet::of(OwnState::Released)), copy(2, 1, false)}; + f.blocks[2].events = {exit()}; + expectSameAsReference(r, f, 0, "six parameters and pointees"); + + SummaryWork work; + (void)computeSummary(f, 0, &work); + const OwnershipResult allOwned = + solve(f, summaryEntry(f, f.paramLocations.size() + f.pointeeLocations.size(), + OwnState::Owned)); + r.expect(work.solves == 4, "Joint summary: four solves for six parameters and pointees (" + + std::to_string(work.solves) + ")"); + r.expect(allOwned.blockVisits > 0 && work.blockVisits <= 4 * allOwned.blockVisits, + "Joint summary: at most four times the block visits of one solve (" + + std::to_string(work.blockVisits) + " for " + + std::to_string(allOwned.blockVisits) + " in one solve)"); + + OwnershipFacts none = facts(1, {}, 1, 1); + none.blocks[0].events = {acquire(0, 0), release(0), exit()}; + SummaryWork noneWork; + (void)computeSummary(none, 0, &noneWork); + r.expect(noneWork.solves == 1, "Joint summary: one solve without parameters (" + + std::to_string(noneWork.solves) + ")"); + return r.failures == 0; + } } // namespace int main(int, char**) @@ -573,6 +832,8 @@ int main(int, char**) (void)testExitUncertainty(report); (void)testSummaries(report); (void)testSummaryMetadata(report); + (void)testJointSummaries(report); + (void)testJointSummaryWork(report); if (report.failures == 0) { std::cout << "All ownership engine unit tests passed.\n"; From 54eec080253dfb81bf848dd0273b18b50d3eb575 Mon Sep 17 00:00:00 2001 From: Hugo Date: Tue, 6 Oct 2026 07:20:44 +0200 Subject: [PATCH 12/16] feat(ownership): count the solves and block visits of a summary SummaryWork, an optional out-parameter of computeSummary, counts the solves a summary takes and the blocks their worklists process; solve() reports its own block visits. They only observe: no summary depends on them. With the summary computed as before, the test of a function with six parameters and pointees reports 25 solves and 100 block visits, where one solve visits 4 blocks. Refs #184 --- .../analysis/ownership/OwnershipEngine.hpp | 12 +++++++++++- src/analysis/ownership/OwnershipEngine.cpp | 19 +++++++++++++++---- 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/include/analysis/ownership/OwnershipEngine.hpp b/include/analysis/ownership/OwnershipEngine.hpp index 7212a41..ec3b8f5 100644 --- a/include/analysis/ownership/OwnershipEngine.hpp +++ b/include/analysis/ownership/OwnershipEngine.hpp @@ -29,6 +29,7 @@ namespace ctrace::stack::analysis::ownership std::vector in; // by block std::vector out; // by block std::vector exits; // in block/event order; empty when incomplete + std::uint64_t blockVisits = 0; // blocks the worklist processed; observation only bool incomplete = false; // budget exhausted: nothing below may be trusted std::uint8_t reservedPadding[7] = {}; }; @@ -62,6 +63,15 @@ namespace ctrace::stack::analysis::ownership /// the exits of each kind. Fresh resources: `returns` is Guaranteed when every normal /// exit returns exactly a fresh resource, Conditional when some do, Unknown otherwise /// (likewise `outArgs` for ArgPointee locations). + /// The work of one computeSummary, for measurement only: it never changes the summary. + struct SummaryWork + { + std::uint64_t blockVisits = 0; + std::uint32_t solves = 0; + std::uint32_t reservedPadding = 0; + }; + FunctionOwnershipSummary computeSummary(const OwnershipFacts& facts, - unsigned iterationLimit = 0); + unsigned iterationLimit = 0, + SummaryWork* work = nullptr); } // namespace ctrace::stack::analysis::ownership diff --git a/src/analysis/ownership/OwnershipEngine.cpp b/src/analysis/ownership/OwnershipEngine.cpp index 82ecbc5..271576a 100644 --- a/src/analysis/ownership/OwnershipEngine.cpp +++ b/src/analysis/ownership/OwnershipEngine.cpp @@ -304,6 +304,7 @@ namespace ctrace::stack::analysis::ownership const std::uint32_t block = worklist.front(); worklist.pop_front(); queued[block] = false; + ++result.blockVisits; AbstractState state = result.in[block]; for (const Event& e : facts.blocks[block].events) @@ -476,9 +477,20 @@ namespace ctrace::stack::analysis::ownership } } // namespace - FunctionOwnershipSummary computeSummary(const OwnershipFacts& facts, unsigned iterationLimit) + FunctionOwnershipSummary computeSummary(const OwnershipFacts& facts, unsigned iterationLimit, + SummaryWork* work) { FunctionOwnershipSummary summary; + const auto solveCounted = [&](const AbstractState& entry) + { + OwnershipResult res = solve(facts, entry, iterationLimit); + if (work) + { + ++work->solves; + work->blockVisits += res.blockVisits; + } + return res; + }; // Parameters (by value, then pointees) get their own resources, numbered after the // function's sites. @@ -513,7 +525,7 @@ namespace ctrace::stack::analysis::ownership ParamTransformer exceptional{}; for (const OwnState state : kAllStates) { - const OwnershipResult res = solve(facts, entryWith(p, state), iterationLimit); + const OwnershipResult res = solveCounted(entryWith(p, state)); if (res.incomplete) { summary.incomplete = true; @@ -543,8 +555,7 @@ namespace ctrace::stack::analysis::ownership } // Fresh resources: one solve with every parameter Owned (or none). - const OwnershipResult res = - solve(facts, entryWith(paramTotal, OwnState::Owned), iterationLimit); + const OwnershipResult res = solveCounted(entryWith(paramTotal, OwnState::Owned)); if (res.incomplete) { summary.incomplete = true; From e60a8443680a9cc6ccd9becaf731d02f4436b0f1 Mon Sep 17 00:00:00 2001 From: Hugo Date: Tue, 6 Oct 2026 07:21:26 +0200 Subject: [PATCH 13/16] perf(ownership): solve a summary's parameters together, once per entry state computeSummary solved the whole function once per parameter and entry state, the other parameters Owned, and once more with all of them Owned for the fresh resources: 4 * (parameters + pointee paths) + 1 solves, 101 for a function with 25 (#184). It now solves once per entry state, with every parameter in that state. Each parameter has a resource of its own; an event acts on a resource from that resource's own state, what a location holds never depends on any state, and paths join resource by resource. Each parameter therefore ends as in a solve of its own with the others Owned. The solve with every parameter Owned is the one the fresh resources were read off; without parameters it remains the only one. Four solves instead of 4 * P + 1; the test of six parameters and pointees goes from 25 solves and 100 block visits to 4 and 16. Refs #184 --- src/analysis/ownership/OwnershipEngine.cpp | 65 ++++++++++++---------- 1 file changed, 36 insertions(+), 29 deletions(-) diff --git a/src/analysis/ownership/OwnershipEngine.cpp b/src/analysis/ownership/OwnershipEngine.cpp index 271576a..089fd4e 100644 --- a/src/analysis/ownership/OwnershipEngine.cpp +++ b/src/analysis/ownership/OwnershipEngine.cpp @@ -504,65 +504,72 @@ namespace ctrace::stack::analysis::ownership : facts.pointeeLocations[p - facts.paramLocations.size()].second; }; - const auto entryWith = [&](std::size_t paramIndex, OwnState state) + // Every parameter gets a resource of its own, in the given entry state. + const auto entryWith = [&](OwnState state) { AbstractState entry = AbstractState::entry(resourceCount, facts.locations.size()); for (std::size_t p = 0; p < paramTotal; ++p) { const ResourceId r = newInstanceOf(paramSiteBase + static_cast(p)); entry.locations[paramLocationAt(p)].add(r); - entry.resources[r] = StateSet::of(p == paramIndex ? state : OwnState::Owned); + entry.resources[r] = StateSet::of(state); } return entry; }; + // One solve per entry state, every parameter in that state. An event acts on a resource + // from that resource's own state, and what a location holds never depends on any state; + // paths join resource by resource. So each parameter ends as it would in a solve where + // only it starts in that state and the others start Owned. The solve with every + // parameter Owned also gives the fresh resources; without parameters it is the only one. bool anyNormal = false; bool anyExceptional = false; - for (std::size_t p = 0; p < paramTotal; ++p) + std::vector normal(paramTotal); + std::vector exceptional(paramTotal); + OwnershipResult res; + for (const OwnState state : kAllStates) { - const ResourceId r = newInstanceOf(paramSiteBase + static_cast(p)); - ParamTransformer normal{}; - ParamTransformer exceptional{}; - for (const OwnState state : kAllStates) + if (paramTotal == 0 && state != OwnState::Owned) + continue; + OwnershipResult solved = solveCounted(entryWith(state)); + if (solved.incomplete) { - const OwnershipResult res = solveCounted(entryWith(p, state)); - if (res.incomplete) - { - summary.incomplete = true; - return summary; - } - for (const ExitRecord& exit : res.exits) + summary.incomplete = true; + return summary; + } + for (const ExitRecord& exit : solved.exits) + { + (exit.exceptional ? anyExceptional : anyNormal) = true; + for (std::size_t p = 0; p < paramTotal; ++p) { - ParamTransformer& t = exit.exceptional ? exceptional : normal; + const ResourceId r = + newInstanceOf(paramSiteBase + static_cast(p)); + ParamTransformer& t = exit.exceptional ? exceptional[p] : normal[p]; t[static_cast(state)] |= exit.state.resources[r]; if (exit.state.uncertain[r]) t.uncertainInputs |= StateSet::of(state); - (exit.exceptional ? anyExceptional : anyNormal) = true; } } + if (state == OwnState::Owned) + res = std::move(solved); + } + for (std::size_t p = 0; p < paramTotal; ++p) + { if (p < facts.paramLocations.size()) { const unsigned argIndex = facts.paramLocations[p].first; - summary.normal.params[argIndex] = normal; - summary.exceptional.params[argIndex] = exceptional; + summary.normal.params[argIndex] = normal[p]; + summary.exceptional.params[argIndex] = exceptional[p]; } else { const ArgPath& path = facts.pointeeLocations[p - facts.paramLocations.size()].first; - summary.normal.pointeeParams[path] = normal; - summary.exceptional.pointeeParams[path] = exceptional; + summary.normal.pointeeParams[path] = normal[p]; + summary.exceptional.pointeeParams[path] = exceptional[p]; } } - // Fresh resources: one solve with every parameter Owned (or none). - const OwnershipResult res = solveCounted(entryWith(paramTotal, OwnState::Owned)); - if (res.incomplete) - { - summary.incomplete = true; - return summary; - } - for (const ExitRecord& exit : res.exits) - (exit.exceptional ? anyExceptional : anyNormal) = true; + // Fresh resources: read off the solve with every parameter Owned. const ResourceId paramResourceLowerBound = newInstanceOf(paramSiteBase); summary.normal.returns.certainty = freshResourceCertainty( facts, res, false, LocationKind::Return, ArgPath{}, paramResourceLowerBound); From a9e75d16e7a87e4083a2023f082e2ed4a0d8cf66 Mon Sep 17 00:00:00 2001 From: Hugo Date: Wed, 7 Oct 2026 13:43:00 +0200 Subject: [PATCH 14/16] test(ownership): recompute only the summaries whose consulted summaries changed computeOwnershipSummaries recomputes every function of the module each round, up to 16, although a round may change a single summary (#184): the self-analysis computes 88 458 summaries for 53 files. A reference written in the test runs that fixed point: every function recomputed in index order each round until a round changes none, every summary incomplete past the budget. computeSummaryFixpoint must give the same summaries with no more computations: independent functions, a chain against the index order, recursion, mutual recursion, a change of the incomplete status alone, a consulted summary that changes later in the same round while the computations that consulted it changed nothing, and an exhausted budget, which takes exactly 16 rounds. Independent functions are computed once each, and the chain fewer times than every function each round. SummaryFixpointWork counts the computations, the reused summaries and the rounds; sameSummary compares summaries, incompleteness included. Refs #184 --- test/unit/ownership_engine_unit_tests.cpp | 162 ++++++++++++++++++++++ 1 file changed, 162 insertions(+) diff --git a/test/unit/ownership_engine_unit_tests.cpp b/test/unit/ownership_engine_unit_tests.cpp index 0b7b0d9..b3150cd 100644 --- a/test/unit/ownership_engine_unit_tests.cpp +++ b/test/unit/ownership_engine_unit_tests.cpp @@ -5,7 +5,9 @@ #include "analysis/ownership/OwnershipEngine.hpp" #include "analysis/ownership/OwnershipFacts.hpp" +#include #include +#include #include #include #include @@ -820,6 +822,165 @@ namespace std::to_string(noneWork.solves) + ")"); return r.failures == 0; } + // ---- summaries of functions that consult each other's ------------------------------------- + + // A small integer carried by a summary, so that a synthetic computation can depend on the + // summaries it consults. + int valueOf(const FunctionOwnershipSummary& s) + { + return s.normal.returns.kind.empty() ? 0 : std::stoi(s.normal.returns.kind); + } + FunctionOwnershipSummary withValue(int value, bool incomplete = false) + { + FunctionOwnershipSummary s; + s.normal.returns.kind = std::to_string(value); + s.incomplete = incomplete; + return s; + } + + using Compute = std::function; + + // The fixed point as computeOwnershipSummaries ran it before it reused summaries: every + // function recomputed in index order each round until a round changes none; past maxRounds + // rounds, every summary is incomplete. + std::vector referenceFixpoint(std::size_t count, + const Compute& compute, + unsigned maxRounds, + std::uint64_t& computations) + { + std::vector summaries(count); + const ConsultSummary consult = [&](std::size_t j) -> const FunctionOwnershipSummary& + { return summaries[j]; }; + for (unsigned round = 0; round < maxRounds; ++round) + { + bool changed = false; + for (std::size_t i = 0; i < count; ++i) + { + FunctionOwnershipSummary next = compute(i, consult); + ++computations; + if (!sameSummary(summaries[i], next)) + { + summaries[i] = std::move(next); + changed = true; + } + } + if (!changed) + return summaries; + } + for (FunctionOwnershipSummary& s : summaries) + s.incomplete = true; + return summaries; + } + + // computeSummaryFixpoint gives what the reference gives, with fewer computations or as many. + // Returns its work. + SummaryFixpointWork expectSameFixpoint(TestReport& r, std::size_t count, const Compute& compute, + unsigned maxRounds, const std::string& name) + { + std::uint64_t referenceComputations = 0; + const auto expected = referenceFixpoint(count, compute, maxRounds, referenceComputations); + SummaryFixpointWork work; + const auto got = computeSummaryFixpoint(count, compute, maxRounds, &work); + bool same = got.size() == expected.size(); + for (std::size_t i = 0; same && i < got.size(); ++i) + same = sameSummary(got[i], expected[i]); + r.expect(same, + "Summary fixpoint: " + name + ": the summaries of recomputing every function"); + r.expect(work.computations <= referenceComputations, + "Summary fixpoint: " + name + ": no more computations than recomputing all (" + + std::to_string(work.computations) + " for " + + std::to_string(referenceComputations) + ")"); + return work; + } + + bool testSummaryFixpoint(TestReport& r) + { + { + // Independent functions: one round computes them, the next changes nothing; none + // of them consulted a summary, so none is computed twice. + const Compute compute = [](std::size_t i, const ConsultSummary&) + { return withValue(static_cast(i)); }; + const SummaryFixpointWork work = expectSameFixpoint(r, 5, compute, 16, "independent"); + r.expect(work.computations == 5 && work.rounds == 2, + "Summary fixpoint: independent functions are computed once (" + + std::to_string(work.computations) + " computations, " + + std::to_string(work.rounds) + " rounds)"); + } + { + // A chain against the index order: function i consults function i + 1, which a round + // only reaches after it, so the value moves one function per round. A function is + // computed again only once the one it consults has changed. + const Compute compute = [](std::size_t i, const ConsultSummary& consult) + { return withValue(i == 5 ? 1 : valueOf(consult(i + 1)) + 1); }; + const SummaryFixpointWork work = expectSameFixpoint(r, 6, compute, 16, "chain"); + r.expect(work.computations < 6 * work.rounds, + "Summary fixpoint: a chain recomputes fewer than every function each round (" + + std::to_string(work.computations) + " computations, " + + std::to_string(work.rounds) + " rounds)"); + } + { + // Recursion: a function that consults itself until it reaches 3, and one that copies it. + const Compute compute = [](std::size_t i, const ConsultSummary& consult) + { + return i == 0 ? withValue(std::min(valueOf(consult(0)) + 1, 3)) + : withValue(valueOf(consult(0))); + }; + expectSameFixpoint(r, 2, compute, 16, "recursion"); + } + { + // Mutual recursion: each consults the other. + const Compute compute = [](std::size_t i, const ConsultSummary& consult) + { + return i == 0 ? withValue(std::min(valueOf(consult(1)) + 1, 4)) + : withValue(valueOf(consult(0))); + }; + expectSameFixpoint(r, 2, compute, 16, "mutual recursion"); + } + { + // Only the incomplete status changes: function 1 becomes incomplete, its value kept, + // once function 2 reaches 2; function 0 copies the status of function 1. + const Compute compute = [](std::size_t i, const ConsultSummary& consult) + { + if (i == 2) + return withValue(std::min(valueOf(consult(2)) + 1, 2)); + if (i == 1) + return withValue(7, valueOf(consult(2)) >= 2); + return withValue(0, consult(1).incomplete); + }; + std::uint64_t ignored = 0; + const auto expected = referenceFixpoint(3, compute, 16, ignored); + r.expect(expected[0].incomplete, + "Summary fixpoint: the incomplete case does reach function 0"); + expectSameFixpoint(r, 3, compute, 16, "incomplete status"); + } + { + // A summary consulted, then changed later in the same round: function 0 consults + // function 1, which a round reaches after it, and keeps its initial summary until + // function 1 reaches 2. Its computations leave its summary unchanged until then, and + // the change of function 1 comes after them in each round: function 0 must still be + // computed again. + const Compute compute = [](std::size_t i, const ConsultSummary& consult) + { + if (i == 1) + return withValue(std::min(valueOf(consult(1)) + 1, 2)); + return valueOf(consult(1)) >= 2 ? withValue(20) : FunctionOwnershipSummary{}; + }; + std::uint64_t ignored = 0; + r.expect(valueOf(referenceFixpoint(2, compute, 16, ignored)[0]) == 20, + "Summary fixpoint: the changed-input case does reach function 0"); + expectSameFixpoint(r, 2, compute, 16, "input changed later in the round"); + } + { + // No fixed point within the budget: every summary ends incomplete, after as many + // rounds as the budget allows. + const Compute compute = [](std::size_t i, const ConsultSummary& consult) + { return i == 0 ? withValue(valueOf(consult(0)) + 1) : withValue(5); }; + const SummaryFixpointWork work = expectSameFixpoint(r, 2, compute, 16, "budget"); + r.expect(work.rounds == 16, "Summary fixpoint: the budget bounds the rounds (" + + std::to_string(work.rounds) + ")"); + } + return r.failures == 0; + } } // namespace int main(int, char**) @@ -834,6 +995,7 @@ int main(int, char**) (void)testSummaryMetadata(report); (void)testJointSummaries(report); (void)testJointSummaryWork(report); + (void)testSummaryFixpoint(report); if (report.failures == 0) { std::cout << "All ownership engine unit tests passed.\n"; From eb4c750167407d389dc9b4249ccf9a5fa906d7fd Mon Sep 17 00:00:00 2001 From: Hugo Date: Wed, 7 Oct 2026 13:44:37 +0200 Subject: [PATCH 15/16] refactor(ownership): run the summary fixed point through computeSummaryFixpoint The fixed point of computeOwnershipSummaries moves into the ownership engine, as computeSummaryFixpoint, which knows nothing of LLVM. It runs the same rounds in the same order, starts from the same empty summaries, stops after the first round that changes nothing, and past 16 rounds marks every summary incomplete. The summaries of the module's functions are now read through consult; those of other modules, which do not change here, are still read directly. sameSummary, the comparison of summaries, moves into the engine with the same fields; ownershipSummaryEquals keeps its interface and calls it. Every function is still recomputed each round: the work tests report 10 computations for 5 independent functions and 42 for the chain. Refs #184 --- .../analysis/ownership/OwnershipEngine.hpp | 24 +++++++ src/analysis/ResourceLifetimeAnalysis.cpp | 67 +++++++------------ src/analysis/ownership/OwnershipEngine.cpp | 44 ++++++++++++ 3 files changed, 94 insertions(+), 41 deletions(-) diff --git a/include/analysis/ownership/OwnershipEngine.hpp b/include/analysis/ownership/OwnershipEngine.hpp index ec3b8f5..750384b 100644 --- a/include/analysis/ownership/OwnershipEngine.hpp +++ b/include/analysis/ownership/OwnershipEngine.hpp @@ -6,6 +6,7 @@ #include "analysis/ownership/OwnershipDomain.hpp" #include "analysis/ownership/OwnershipFacts.hpp" +#include #include #include #include @@ -74,4 +75,27 @@ namespace ctrace::stack::analysis::ownership FunctionOwnershipSummary computeSummary(const OwnershipFacts& facts, unsigned iterationLimit = 0, SummaryWork* work = nullptr); + /// Whether two summaries are the same, incompleteness included. + bool sameSummary(const FunctionOwnershipSummary& a, const FunctionOwnershipSummary& b); + + /// The work of one computeSummaryFixpoint, for measurement only: it never changes a summary. + struct SummaryFixpointWork + { + std::uint64_t computations = 0; // summaries computed + std::uint64_t reused = 0; // summaries kept: none of those they consulted changed + std::uint32_t rounds = 0; + std::uint32_t reservedPadding = 0; + }; + + using ConsultSummary = std::function; + + /// The summaries of count functions that may consult each other's. Round by round, in index + /// order, compute(i, consult) gives the summary of function i, reading the current summary of + /// function j through consult(j), until a round changes none; past maxRounds rounds, every + /// summary is incomplete. Every summary starts empty. compute must read the summaries of these + /// functions through consult only, and depend on nothing else that changes. + std::vector computeSummaryFixpoint( + std::size_t count, + const std::function& compute, + unsigned maxRounds, SummaryFixpointWork* work = nullptr); } // namespace ctrace::stack::analysis::ownership diff --git a/src/analysis/ResourceLifetimeAnalysis.cpp b/src/analysis/ResourceLifetimeAnalysis.cpp index 0734d89..8751790 100644 --- a/src/analysis/ResourceLifetimeAnalysis.cpp +++ b/src/analysis/ResourceLifetimeAnalysis.cpp @@ -2116,18 +2116,10 @@ namespace ctrace::stack::analysis return true; } - static bool exitTransformerEquals(const ownership::ExitTransformer& a, - const ownership::ExitTransformer& b) - { - return a.present == b.present && a.returns == b.returns && a.outArgs == b.outArgs && - a.params == b.params && a.pointeeParams == b.pointeeParams; - } - bool ownershipSummaryEquals(const ownership::FunctionOwnershipSummary& lhs, const ownership::FunctionOwnershipSummary& rhs) { - return lhs.incomplete == rhs.incomplete && exitTransformerEquals(lhs.normal, rhs.normal) && - exitTransformerEquals(lhs.exceptional, rhs.exceptional); + return ownership::sameSummary(lhs, rhs); } /// Transformer summaries of every analysed definition, iterated to a fixpoint so that @@ -2153,43 +2145,36 @@ namespace ctrace::stack::analysis if (!F.isDeclaration() && shouldAnalyze(F)) functions.push_back(&F); } - // Start from "no effect" transformers so a recursive callee is optimistic first - // and widens monotonically. - for (llvm::Function* F : functions) - summaries[F] = FunctionOwnershipSummary{}; - - const ownership::SummaryLookup lookup{ - [&](const llvm::Function& callee) -> const FunctionOwnershipSummary* - { - if (const auto it = summaries.find(&callee); it != summaries.end()) - return &it->second; - const auto ext = externalByName.find( - ctrace_tools::canonicalizeMangledName(callee.getName().str())); - return ext == externalByName.end() ? nullptr : ext->second; - }}; + std::unordered_map indexOf; + for (std::size_t i = 0; i < functions.size(); ++i) + indexOf.emplace(functions[i], i); + // Every summary starts as "no effect", so a recursive callee is optimistic first and + // widens monotonically. The summaries of this module are read through consult, which + // lets the fixed point know what each computation consulted; those of other modules do + // not change here. const llvm::DataLayout& DL = mod.getDataLayout(); constexpr unsigned kMaxRounds = 16; - for (unsigned round = 0; round < kMaxRounds; ++round) - { - bool changed = false; - for (llvm::Function* F : functions) + const std::vector computed = ownership::computeSummaryFixpoint( + functions.size(), + [&](std::size_t i, const ownership::ConsultSummary& consult) { + const ownership::SummaryLookup lookup{ + [&](const llvm::Function& callee) -> const FunctionOwnershipSummary* + { + if (const auto it = indexOf.find(&callee); it != indexOf.end()) + return &consult(it->second); + const auto ext = externalByName.find( + ctrace_tools::canonicalizeMangledName(callee.getName().str())); + return ext == externalByName.end() ? nullptr : ext->second; + }}; const ownership::CollectedFunction collected = - ownership::collectOwnershipFacts(*F, model, lookup, DL); - FunctionOwnershipSummary next = ownership::computeSummary(collected.facts); - if (!ownershipSummaryEquals(summaries[F], next)) - { - summaries[F] = std::move(next); - changed = true; - } - } - if (!changed) - return summaries; - } - // No fixpoint within budget: nothing computed here may be trusted. - for (auto& [F, summary] : summaries) - summary.incomplete = true; + ownership::collectOwnershipFacts(*functions[i], model, lookup, DL); + return ownership::computeSummary(collected.facts); + }, + kMaxRounds); + for (std::size_t i = 0; i < functions.size(); ++i) + summaries[functions[i]] = computed[i]; return summaries; } diff --git a/src/analysis/ownership/OwnershipEngine.cpp b/src/analysis/ownership/OwnershipEngine.cpp index 089fd4e..d8fd3aa 100644 --- a/src/analysis/ownership/OwnershipEngine.cpp +++ b/src/analysis/ownership/OwnershipEngine.cpp @@ -600,4 +600,48 @@ namespace ctrace::stack::analysis::ownership summary.exceptional.present = anyExceptional; return summary; } + + bool sameSummary(const FunctionOwnershipSummary& a, const FunctionOwnershipSummary& b) + { + const auto sameExit = [](const ExitTransformer& x, const ExitTransformer& y) + { + return x.present == y.present && x.returns == y.returns && x.outArgs == y.outArgs && + x.params == y.params && x.pointeeParams == y.pointeeParams; + }; + return a.incomplete == b.incomplete && sameExit(a.normal, b.normal) && + sameExit(a.exceptional, b.exceptional); + } + + std::vector computeSummaryFixpoint( + std::size_t count, + const std::function& compute, + unsigned maxRounds, SummaryFixpointWork* work) + { + std::vector summaries(count); + const ConsultSummary consult = [&](std::size_t j) -> const FunctionOwnershipSummary& + { return summaries[j]; }; + for (unsigned round = 0; round < maxRounds; ++round) + { + if (work) + ++work->rounds; + bool changed = false; + for (std::size_t i = 0; i < count; ++i) + { + FunctionOwnershipSummary next = compute(i, consult); + if (work) + ++work->computations; + if (!sameSummary(summaries[i], next)) + { + summaries[i] = std::move(next); + changed = true; + } + } + if (!changed) + return summaries; + } + // No fixpoint within budget: nothing computed here may be trusted. + for (FunctionOwnershipSummary& summary : summaries) + summary.incomplete = true; + return summaries; + } } // namespace ctrace::stack::analysis::ownership From ecf7f07a7bc3e4667fea6af628a7f58c062ca4c9 Mon Sep 17 00:00:00 2001 From: Hugo Date: Wed, 7 Oct 2026 13:45:43 +0200 Subject: [PATCH 16/16] perf(ownership): keep a summary while none of those it consulted changed computeSummaryFixpoint recomputed every function each round. A summary is now kept when none of the summaries its last computation consulted has changed since: computing it again would give the same summary, as the computation depends on nothing else that changes. Each summary has a version raised when it changes, its incompleteness included; each computation records the version of every summary it consults, its own when recursive. The rounds, their order, the stop after a round that changes nothing and the incomplete marking after 16 rounds are unchanged. The work tests go from 10 to 5 computations for 5 independent functions and from 42 to 21 for the chain. Refs #184 --- src/analysis/ownership/OwnershipEngine.cpp | 26 ++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/src/analysis/ownership/OwnershipEngine.cpp b/src/analysis/ownership/OwnershipEngine.cpp index d8fd3aa..3157795 100644 --- a/src/analysis/ownership/OwnershipEngine.cpp +++ b/src/analysis/ownership/OwnershipEngine.cpp @@ -4,7 +4,9 @@ #include #include #include +#include #include +#include namespace ctrace::stack::analysis::ownership { @@ -618,8 +620,12 @@ namespace ctrace::stack::analysis::ownership unsigned maxRounds, SummaryFixpointWork* work) { std::vector summaries(count); - const ConsultSummary consult = [&](std::size_t j) -> const FunctionOwnershipSummary& - { return summaries[j]; }; + // A summary's version grows each time it changes. A computation records the version of + // every summary it consults; while none of them has changed since, computing again would + // give the same summary, so it is kept. + std::vector version(count, 0); + std::vector>>> consulted( + count); for (unsigned round = 0; round < maxRounds; ++round) { if (work) @@ -627,12 +633,28 @@ namespace ctrace::stack::analysis::ownership bool changed = false; for (std::size_t i = 0; i < count; ++i) { + if (consulted[i] && + std::all_of(consulted[i]->begin(), consulted[i]->end(), [&](const auto& input) + { return version[input.first] == input.second; })) + { + if (work) + ++work->reused; + continue; + } + std::vector> inputs; + const ConsultSummary consult = [&](std::size_t j) -> const FunctionOwnershipSummary& + { + inputs.emplace_back(j, version[j]); + return summaries[j]; + }; FunctionOwnershipSummary next = compute(i, consult); if (work) ++work->computations; + consulted[i] = std::move(inputs); if (!sameSummary(summaries[i], next)) { summaries[i] = std::move(next); + ++version[i]; changed = true; } }