diff --git a/src/analysis/UninitializedVarAnalysis.cpp b/src/analysis/UninitializedVarAnalysis.cpp index f7344c7..42738a4 100644 --- a/src/analysis/UninitializedVarAnalysis.cpp +++ b/src/analysis/UninitializedVarAnalysis.cpp @@ -1897,9 +1897,9 @@ namespace ctrace::stack::analysis }; static std::optional - resolveExternalReadSinkSignature(const llvm::Function* callee) + resolveExternalReadSinkSignature(const llvm::Function* callee, bool asDeclaration) { - if (!callee || !callee->isDeclaration() || callee->isIntrinsic()) + if (!callee || !asDeclaration || callee->isIntrinsic()) return std::nullopt; const llvm::StringRef calleeName = canonicalExternalCalleeName(callee->getName()); @@ -1921,10 +1921,11 @@ namespace ctrace::stack::analysis }; static std::optional - resolveExternalReadSinkSpec(const llvm::CallBase& CB, const llvm::Function* callee) + resolveExternalReadSinkSpec(const llvm::CallBase& CB, const llvm::Function* callee, + bool asDeclaration) { const std::optional signature = - resolveExternalReadSinkSignature(callee); + resolveExternalReadSinkSignature(callee, asDeclaration); if (!signature) return std::nullopt; @@ -2098,9 +2099,10 @@ namespace ctrace::stack::analysis } static bool declarationCallArgMayWriteThrough(const llvm::CallBase& CB, - const llvm::Function* callee, unsigned argIdx) + const llvm::Function* callee, unsigned argIdx, + bool asDeclaration) { - if (!callee || !callee->isDeclaration() || callee->isIntrinsic()) + if (!callee || !asDeclaration || callee->isIntrinsic()) return false; if (argIdx >= CB.arg_size()) return false; @@ -2113,7 +2115,7 @@ namespace ctrace::stack::analysis // Known output sinks (write/send/fwrite families) consume this pointer as read-only. if (const std::optional sink = - resolveExternalReadSinkSignature(callee)) + resolveExternalReadSinkSignature(callee, asDeclaration)) { if (argIdx == sink->pointerArgIndex) return false; @@ -2176,9 +2178,9 @@ namespace ctrace::stack::analysis const llvm::CallBase& CB, const llvm::Function* callee, const TrackedObjectContext& tracked, const llvm::DataLayout& DL, InitRangeState& initialized, llvm::BitVector* writeSeen, - FunctionSummary* currentSummary) + FunctionSummary* currentSummary, bool asDeclaration) { - if (!callee || !callee->isDeclaration()) + if (!callee || !asDeclaration) return false; const llvm::StringRef calleeName = normalizeDeclarationCalleeName(callee->getName()); @@ -2220,26 +2222,24 @@ namespace ctrace::stack::analysis return true; } - static void applyExternalDeclarationCallWriteEffects(const llvm::CallBase& CB, - const llvm::Function* callee, - const TrackedObjectContext& tracked, - const llvm::DataLayout& DL, - InitRangeState& initialized, - llvm::BitVector* writeSeen, - FunctionSummary* currentSummary) + static void applyExternalDeclarationCallWriteEffects( + const llvm::CallBase& CB, const llvm::Function* callee, + const TrackedObjectContext& tracked, const llvm::DataLayout& DL, + InitRangeState& initialized, llvm::BitVector* writeSeen, + FunctionSummary* currentSummary, bool asDeclaration) { - if (!callee || !callee->isDeclaration()) + if (!callee || !asDeclaration) return; if (tryApplyBoundedMemTransferDeclarationWriteEffects( - CB, callee, tracked, DL, initialized, writeSeen, currentSummary)) + CB, callee, tracked, DL, initialized, writeSeen, currentSummary, asDeclaration)) { return; } for (unsigned argIdx = 0; argIdx < CB.arg_size(); ++argIdx) { - if (!declarationCallArgMayWriteThrough(CB, callee, argIdx)) + if (!declarationCallArgMayWriteThrough(CB, callee, argIdx, asDeclaration)) continue; const llvm::Value* ptrOperand = CB.getArgOperand(argIdx); @@ -2254,10 +2254,10 @@ namespace ctrace::stack::analysis const TrackedObjectContext& tracked, const llvm::DataLayout& DL, const InitRangeState& initialized, llvm::BitVector* readBeforeInitSeen, FunctionSummary* currentSummary, - std::vector* emittedIssues) + std::vector* emittedIssues, bool asDeclaration) { const std::optional sink = - resolveExternalReadSinkSpec(CB, callee); + resolveExternalReadSinkSpec(CB, callee, asDeclaration); if (!sink) return; if (sink->pointerArgIndex >= CB.arg_size()) @@ -3142,7 +3142,6 @@ namespace ctrace::stack::analysis const llvm::Function* callee = directCallee(*CB); const FunctionSummary* calleeSummary = nullptr; - bool imported = false; if (callee) { auto itSummary = summaries.find(callee); @@ -3163,21 +3162,19 @@ namespace ctrace::stack::analysis symbolName ? externalSummariesByName->find(*symbolName) : externalSummariesByName->find(linkerSymbolName(*callee)); if (itExternal != externalSummariesByName->end()) - { calleeSummary = &itExternal->second; - imported = true; - } } } + bool asDeclaration = callee && callee->isDeclaration(); if (calleeSummary && !calleeSummary->complete) { - // The caller is no more complete than what it used. Imported, an incomplete - // summary counts as absent for its effects: the call gets the presumption for - // declarations, exactly as an unknown function (#157). + // The caller is no more complete than what it used. Imported or defined in this + // module, an incomplete summary counts as absent for its effects: the call gets + // the presumption for declarations, exactly as an unknown function (#157, #168). if (currentSummary) currentSummary->complete = false; - if (imported) - calleeSummary = nullptr; + calleeSummary = nullptr; + asDeclaration = true; } const bool hasSummary = (calleeSummary != nullptr); if (!hasSummary) @@ -3186,11 +3183,14 @@ namespace ctrace::stack::analysis constructedSeen, defaultCtorSeen, currentSummary); applyExternalDeclarationCallReadEffects(*CB, callee, tracked, DL, initialized, readBeforeInitSeen, currentSummary, - emittedIssues); + emittedIssues, asDeclaration); applyExternalDeclarationCallWriteEffects(*CB, callee, tracked, DL, initialized, - writeSeen, currentSummary); - applyUnsummarizedDefinedCallWriteEffects(*CB, callee, tracked, DL, initialized, - writeSeen, currentSummary); + writeSeen, currentSummary, asDeclaration); + if (!asDeclaration) + { + applyUnsummarizedDefinedCallWriteEffects(*CB, callee, tracked, DL, initialized, + writeSeen, currentSummary); + } } if (!callee) return; diff --git a/test/unit/analyzer_module_unit_tests.cpp b/test/unit/analyzer_module_unit_tests.cpp index 3f31d39..c084e0b 100644 --- a/test/unit/analyzer_module_unit_tests.cpp +++ b/test/unit/analyzer_module_unit_tests.cpp @@ -847,6 +847,86 @@ namespace return report.failures == 0; } + // #168: in one file too, the summary of a dataflow stopped before its fixpoint is absent + // for its effects (spec ยง4.1): a call to it reports what a call to a declaration reports. + bool testUninitializedIncompleteSummariesInOneFile(const std::filesystem::path& repoRoot, + TestReport& report) + { + using namespace ctrace::stack::analysis; + const ctrace::stack::AnalysisConfig config; + LoadedModule loaded; + std::string loadError; + const std::filesystem::path source = repoRoot / "test/unit/uninit_incomplete_same_file.c"; + if (!loadModuleFromSource(source, config, loaded, loadError)) + { + report.expect(false, + "UninitIncompleteOneFile setup: failed to load module: " + loadError); + return false; + } + + auto analyzeAll = [](const llvm::Function&) { return true; }; + using Kind = UninitializedLocalIssueKind; + // The issues of one function, as (kind, variable) pairs. + const auto issuesOf = + [](const std::vector& issues, const char* func) + { + std::multiset> out; + for (const UninitializedLocalReadIssue& issue : issues) + if (issue.funcName == func) + out.emplace(issue.kind, issue.varName); + return out; + }; + const auto has = [](const std::multiset>& issues, Kind kind) + { + return std::any_of(issues.begin(), issues.end(), + [kind](const auto& issue) { return issue.first == kind; }); + }; + + // Automatic budget: both callees converge, and their summaries are exact. + const std::vector converged = + analyzeUninitializedLocalReads(*loaded.module, analyzeAll, nullptr); + report.expect(issuesOf(converged, "write_then_read").empty(), + "UninitIncompleteOneFile: converged produce initializes x"); + report.expect(issuesOf(converged, "pass_to_reader") + .count({Kind::ReadBeforeDefiniteInitViaCall, "x"}) == 1, + "UninitIncompleteOneFile: converged consume reads x"); + + // Budget of two iterations: the callees stop before their fixpoint, their callers do not. + const std::vector stopped = + analyzeUninitializedLocalReads(*loaded.module, analyzeAll, nullptr, + /*fixpointIterationLimit=*/2); + report.expect(has(issuesOf(stopped, "produce"), Kind::AnalysisIncomplete) && + has(issuesOf(stopped, "consume"), Kind::AnalysisIncomplete), + "UninitIncompleteOneFile: the budget stops produce and consume"); + report.expect(issuesOf(stopped, "write_then_read") == + issuesOf(stopped, "write_then_read_decl"), + "UninitIncompleteOneFile: a call to stopped produce reports as a call to a " + "declaration"); + report.expect(issuesOf(stopped, "pass_to_reader") == + issuesOf(stopped, "pass_to_reader_decl"), + "UninitIncompleteOneFile: a call to stopped consume reports as a call to a " + "declaration"); + + // Looked up by linker symbol, which is portable: "f" on Linux, "_f" on macOS. + const UninitializedSummaryIndex index = buildUninitializedSummaryIndex( + *loaded.module, analyzeAll, static_cast(nullptr), + /*fixpointIterationLimit=*/2); + const auto complete = [&](const char* name) -> std::optional + { + const auto it = + index.functions.find(linkerSymbolName(*loaded.module->getFunction(name))); + if (it == index.functions.end()) + return std::nullopt; + return static_cast(it->second.complete); + }; + report.expect(complete("write_then_read") == false && complete("pass_to_reader") == false, + "UninitIncompleteOneFile: a caller of a stopped callee is incomplete"); + report.expect(complete("write_then_read_decl") == true && + complete("pass_to_reader_decl") == true, + "UninitIncompleteOneFile: a caller of a declaration stays complete"); + return report.failures == 0; + } + // A cyclic group stopped at its iteration cap (#157): the driver must mark its summaries // incomplete, and must not when the group converges. bool testCrossTUDriverMarksUnconvergedCycles(TestReport& report) @@ -2284,6 +2364,7 @@ int main(int argc, char** argv) (void)testLinkerSymbolName(report); (void)testUninitializedSummaryKeysAreLinkerSymbols(repoRoot, report); (void)testUninitializedIncompleteSummaries(repoRoot, report); + (void)testUninitializedIncompleteSummariesInOneFile(repoRoot, report); (void)testCrossTUDriverMarksUnconvergedCycles(report); (void)testProgramPointRanges(repoRoot, report); (void)testProgramPointRangesUnsignedReadings(repoRoot, report); diff --git a/test/unit/uninit_incomplete_same_file.c b/test/unit/uninit_incomplete_same_file.c new file mode 100644 index 0000000..2d23964 --- /dev/null +++ b/test/unit/uninit_incomplete_same_file.c @@ -0,0 +1,69 @@ +// SPDX-License-Identifier: Apache-2.0 +// Input for testUninitializedIncompleteSummariesInOneFile (#168). The blocks of produce and +// consume are laid out before their predecessors, so each dataflow iteration carries the entry +// state one block further: a budget of two iterations stops them before their fixpoint. +// Their callers have a single block and converge within it. +void produce_decl(int* p, int n); +int consume_decl(int* p, int n); + +// Writes *p on every path. +void produce(int* p, int n) +{ + goto L1; +L5: + if (n > 0) + *p = n; + else + *p = 0; + return; +L4: + goto L5; +L3: + goto L4; +L2: + goto L3; +L1: + goto L2; +} + +// Reads *p. +int consume(int* p, int n) +{ + goto L1; +L5: + return *p + n; +L4: + goto L5; +L3: + goto L4; +L2: + goto L3; +L1: + goto L2; +} + +int write_then_read(int n) +{ + int x; + produce(&x, n); + return x; +} + +int write_then_read_decl(int n) +{ + int x; + produce_decl(&x, n); + return x; +} + +int pass_to_reader(int n) +{ + int x; + return consume(&x, n); +} + +int pass_to_reader_decl(int n) +{ + int x; + return consume_decl(&x, n); +}