fix(uninitialized): a stopped callee in the same file counts as a declaration

src/analysis/UninitializedVarAnalysis.cpp

@@ -1897,9 +1897,9 @@ namespace ctrace::stack::analysis
         };
 
         static std::optional<ExternalReadSinkSignature>
-        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<ExternalReadSinkSpec>
-        resolveExternalReadSinkSpec(const llvm::CallBase& CB, const llvm::Function* callee)
+        resolveExternalReadSinkSpec(const llvm::CallBase& CB, const llvm::Function* callee,
+                                    bool asDeclaration)
         {
             const std::optional<ExternalReadSinkSignature> 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<ExternalReadSinkSignature> 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<UninitializedLocalReadIssue>* emittedIssues)
+            std::vector<UninitializedLocalReadIssue>* emittedIssues, bool asDeclaration)
         {
             const std::optional<ExternalReadSinkSpec> 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;

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<UninitializedLocalReadIssue>& issues, const char* func)
+        {
+            std::multiset<std::pair<Kind, std::string>> out;
+            for (const UninitializedLocalReadIssue& issue : issues)
+                if (issue.funcName == func)
+                    out.emplace(issue.kind, issue.varName);
+            return out;
+        };
+        const auto has = [](const std::multiset<std::pair<Kind, std::string>>& 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<UninitializedLocalReadIssue> 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<UninitializedLocalReadIssue> 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<const UninitializedSummaryIndex*>(nullptr),
+            /*fixpointIterationLimit=*/2);
+        const auto complete = [&](const char* name) -> std::optional<bool>
+        {
+            const auto it =
+                index.functions.find(linkerSymbolName(*loaded.module->getFunction(name)));
+            if (it == index.functions.end())
+                return std::nullopt;
+            return static_cast<bool>(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);

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);
+}