From 60ff58a5b8293071709411319864b301cb9ded47 Mon Sep 17 00:00:00 2001 From: Hugo Date: Fri, 9 Oct 2026 00:15:00 +0200 Subject: [PATCH 1/2] test(global-read-before-write): identify a global across files as the linker does Two static globals with the same name in two files, or a static and an external one, share one cross-TU entry: a write to one counts as a write to the other (#166). Cover both collisions, a static global written in another function of its own file, and an external global written in another file. --- run_test.py | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/run_test.py b/run_test.py index eb0f3c2..71b6bbc 100755 --- a/run_test.py +++ b/run_test.py @@ -2874,6 +2874,74 @@ def check_duplicate_if_cross_tu() -> bool: ) +def check_global_read_before_write_identity_across_files() -> bool: + """ + #166: across files, a global is identified as the linker sees it. A static global is its + own module's: a write to a same-named global of another file, static or external, is not a + write to it, while a write in another function of its own file still is. An external global + is shared by the files that define and declare it. + """ + print("=== Testing GlobalReadBeforeWrite global identity across files ===") + sources = { + "a.c": "static int table[4];\nint read_table(int i) { return table[i & 3]; }\n", + "b.c": "static int table[4];\nvoid write_table(int i) { table[i & 3] = 1; }\n" + "int read_b(int i) { return table[i & 3]; }\n", + "ext_table.c": "int table[4];\nvoid write_ext_table(int i) { table[i & 3] = 1; }\n", + "ext_def.c": "int shared[4];\nint read_shared(int i) { return shared[i & 3]; }\n", + "ext_use.c": "extern int shared[4];\nvoid write_shared(int i) { shared[i & 3] = 1; }\n", + "same_file.c": "static int cache[4];\nint read_cache(int i) { return cache[i & 3]; }\n" + "void fill_cache(int i) { cache[i & 3] = i; }\n", + "other.c": "int other(int i) { return i; }\n", + } + # (label, analyzed files, expected (function, confidence, write seen elsewhere) of every + # GlobalReadBeforeWrite diagnostic). + cases = [ + ( + "two static globals with the same name", + ["a.c", "b.c"], + {("read_table", 0.55, False), ("read_b", 0.5, True)}, + ), + ( + "a static and an external global with the same name", + ["a.c", "ext_table.c"], + {("read_table", 0.55, False)}, + ), + ("an external global written in another file", ["ext_def.c", "ext_use.c"], {("read_shared", 0.5, True)}), + ( + "a static global written in another function of its file", + ["same_file.c", "other.c"], + {("read_cache", 0.5, True)}, + ), + ] + ok = True + with tempfile.TemporaryDirectory(prefix="ct_global_identity_") as tmp: + root = Path(tmp) + for name, text in sources.items(): + (root / name).write_text(text) + for label, files, expected in cases: + result = run_analyzer([*(str(root / f) for f in files), "--format=json"]) + try: + payload = json.loads(result.stdout or "") + except json.JSONDecodeError: + ok = fail_check(f"{label}: no JSON output", (result.stdout or "") + (result.stderr or "")) + continue + observed = { + ( + d.get("location", {}).get("function"), + round(float(d.get("confidence", -1)), 2), + "another analyzed function/TU" in d.get("details", {}).get("message", ""), + ) + for d in payload.get("diagnostics", []) + if d.get("ruleId") == "GlobalReadBeforeWrite" + } + if observed != expected: + ok = fail_check(f"{label}: expected {sorted(expected)}, got {sorted(observed)}") + continue + print(f" ✅ {label}") + print() + return ok + + def check_size_minus_one_cross_tu() -> bool: """ #157: the length that a function defined in another file passes to a bounded write reaches @@ -5392,6 +5460,7 @@ def record_ok(ok: bool): check_uninitialized_cross_tu_no_effect, check_const_param_cross_tu, check_duplicate_if_cross_tu, + check_global_read_before_write_identity_across_files, check_size_minus_one_cross_tu, check_cross_tu_call_graph, check_null_deref_nested_inter_tu, From 4c64dd486f8da665039da2178c0cff747f4acac3 Mon Sep 17 00:00:00 2001 From: Hugo Date: Fri, 9 Oct 2026 00:16:38 +0200 Subject: [PATCH 2/2] fix(global-read-before-write): identify a global across files as the linker does The cross-TU summary keyed each global by its raw IR name, whatever its linkage. Two static globals with the same name in two files, or a static and an external one, shared one entry, so a write to one counted as a write to the other. Key an external global by its linker symbol, as the cross-TU function facts do (#157). Key a static global by its module too: it stays in the index, so a write in another function of its own file still counts, but no other file finds it. Fixes #166 --- .../GlobalReadBeforeWriteAnalysis.hpp | 2 ++ .../GlobalReadBeforeWriteAnalysis.cpp | 23 ++++++++++++++++--- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/include/analysis/GlobalReadBeforeWriteAnalysis.hpp b/include/analysis/GlobalReadBeforeWriteAnalysis.hpp index bdb8a65..e0b8c05 100644 --- a/include/analysis/GlobalReadBeforeWriteAnalysis.hpp +++ b/include/analysis/GlobalReadBeforeWriteAnalysis.hpp @@ -24,6 +24,8 @@ namespace ctrace::stack::analysis struct GlobalReadBeforeWriteSummaryIndex { + // Keyed by the linker symbol of an external global; a static global's key also names its + // module, so it is only found from that module, while the module is loaded (#166). std::unordered_map globals; }; diff --git a/src/analysis/GlobalReadBeforeWriteAnalysis.cpp b/src/analysis/GlobalReadBeforeWriteAnalysis.cpp index c70602c..6c04f5f 100644 --- a/src/analysis/GlobalReadBeforeWriteAnalysis.cpp +++ b/src/analysis/GlobalReadBeforeWriteAnalysis.cpp @@ -1,6 +1,9 @@ // SPDX-License-Identifier: Apache-2.0 #include "analysis/GlobalReadBeforeWriteAnalysis.hpp" +#include "analysis/AnalyzerUtils.hpp" + +#include #include #include #include @@ -49,6 +52,20 @@ namespace ctrace::stack::analysis return global; } + // A global as the linker sees it (#166). An external global is keyed by its linker symbol, + // shared by the files that define and declare it. A static global is its module's own: + // its key also carries the module, which lives as long as the index built from it. + std::string summaryKey(const llvm::GlobalVariable& global) + { + if (!global.hasLocalLinkage()) + return linkerSymbolName(global); + std::string key(1, '\0'); + key += std::to_string(reinterpret_cast(global.getParent())); + key += '\0'; + key += global.getName().str(); + return key; + } + const GlobalReadBeforeWriteGlobalSummary* lookupExternalSummary(const GlobalReadBeforeWriteSummaryIndex* externalSummaries, const llvm::GlobalVariable& global) @@ -56,7 +73,7 @@ namespace ctrace::stack::analysis if (!externalSummaries || !global.hasName() || global.getName().empty()) return nullptr; - const auto it = externalSummaries->globals.find(global.getName().str()); + const auto it = externalSummaries->globals.find(summaryKey(global)); if (it == externalSummaries->globals.end()) return nullptr; return &it->second; @@ -213,7 +230,7 @@ namespace ctrace::stack::analysis if (!global || !global->hasName() || global->getName().empty()) return; - auto& summary = out.globals[global->getName().str()]; + auto& summary = out.globals[summaryKey(*global)]; summary.hasAnyWrite = true; summary.zeroInitializedArray = summary.zeroInitializedArray || isTrackedDefinitionGlobal(*global); @@ -229,7 +246,7 @@ namespace ctrace::stack::analysis { if (!isTrackedDefinitionGlobal(global) || !global.hasName() || global.getName().empty()) continue; - out.globals[global.getName().str()].zeroInitializedArray = true; + out.globals[summaryKey(global)].zeroInitializedArray = true; } for (const llvm::Function& function : mod)