Fix audit findings: soundness, coverage reporting, docs, and review follow-ups #1
11 changed files with 240 additions and 28 deletions
fix(summary): key function summaries by USR, not qualified name
Problem: SummaryStore keyed summaries by getQualifiedNameAsString (plus "@file" for internal linkage), which overloads share. When a TU calls a function whose body lives in another TU, the lookup returns whichever overload was summarized last. Example: `take(int*, int)` dereferences p, `take(int*, long)` checks it, and a call `take(q, 1L)` with a null q was reported as a null-deref because it inherited the other overload's nonnull precondition. Lookups also used a different spelling (no "@file") than stores for static functions, so their summaries were never found at call sites. Fix: summaryStoreKey()/summaryStoreKeyForCall() return the declaration's USR (semanticFunctionId: USR, TU-qualified for internal linkage). Every clang-side store, lookup, dependency edge and staleness check uses it: summary_encoder, null-deref, div-by-zero, OOB and the summary priming pipeline, which still selects targets by the call graph's qualified names. Rust summaries keep their own keys. Now that static callees' summaries are found, null-deref's summary-precondition path applies the same DEBUGASSERT and static-parameter-contract filters as its callee-deref path. Test: tests/unit/test_summary_key_identity.cpp covers per-overload summaries, key identity across redeclarations and distinct keys across namespaces, and the cross-TU overload call. That call reported a false null-deref before the fix; a control call to the dereferencing overload still reports one. Tests that hand-built the old keys (test_symbolic_summary, test_cross_tu_security) now derive them from the declaration. Full suite: 140/140 (rust_frontend's 500 ms timeout test flaked once at load average 22 and passed on four reruns).
commit
46aaac427a
|
|
@ -23,12 +23,20 @@ namespace fold {
|
||||||
[[nodiscard]] std::string calleeName(const clang::CallExpr* ce);
|
[[nodiscard]] std::string calleeName(const clang::CallExpr* ce);
|
||||||
|
|
||||||
// Qualified callee name with a TU-local suffix for internal linkage
|
// Qualified callee name with a TU-local suffix for internal linkage
|
||||||
// to disambiguate `static` functions across TUs. Form used by the
|
// to disambiguate `static` functions across TUs. Human-readable; overloads
|
||||||
// summary cache to key per-function summaries.
|
// share it, so it must not key per-function data (see summaryStoreKey).
|
||||||
[[nodiscard]] std::string qualifiedFunctionName(const clang::FunctionDecl* fd);
|
[[nodiscard]] std::string qualifiedFunctionName(const clang::FunctionDecl* fd);
|
||||||
|
|
||||||
[[nodiscard]] std::string qualifiedCalleeName(const clang::CallExpr* ce);
|
[[nodiscard]] std::string qualifiedCalleeName(const clang::CallExpr* ce);
|
||||||
|
|
||||||
|
// Key for detect::SummaryStore: the declaration's USR (semanticFunctionId),
|
||||||
|
// equal across redeclarations and TUs for one external function and distinct
|
||||||
|
// for overloads. Empty for a null declaration.
|
||||||
|
[[nodiscard]] std::string summaryStoreKey(const clang::FunctionDecl* fd);
|
||||||
|
|
||||||
|
// summaryStoreKey of a call's direct callee; empty for indirect calls.
|
||||||
|
[[nodiscard]] std::string summaryStoreKeyForCall(const clang::CallExpr* ce);
|
||||||
|
|
||||||
// True when `fd` can be the C library function its name suggests: not a class
|
// True when `fd` can be the C library function its name suggests: not a class
|
||||||
// member, and declared at global scope, in namespace std, or extern "C".
|
// member, and declared at global scope, in namespace std, or extern "C".
|
||||||
// Library models (malloc/free, taint sources and sinks) must check this; the
|
// Library models (malloc/free, taint sources and sinks) must check this; the
|
||||||
|
|
|
||||||
|
|
@ -23,6 +23,7 @@
|
||||||
#include "fold/frontend/comp_db.hpp"
|
#include "fold/frontend/comp_db.hpp"
|
||||||
#include "fold/ir/call_graph.hpp"
|
#include "fold/ir/call_graph.hpp"
|
||||||
#include "fold/symbolic/encoder.hpp"
|
#include "fold/symbolic/encoder.hpp"
|
||||||
|
#include "fold/symbolic/encoder_callee_names.hpp"
|
||||||
#include "fold/symbolic/summary_encoder.hpp"
|
#include "fold/symbolic/summary_encoder.hpp"
|
||||||
|
|
||||||
namespace fold::cli {
|
namespace fold::cli {
|
||||||
|
|
@ -86,19 +87,22 @@ bool computeSummariesForFunctions(clang::tooling::CompilationDatabase& db,
|
||||||
if (qualified_name.empty() || !target_names_.contains(qualified_name))
|
if (qualified_name.empty() || !target_names_.contains(qualified_name))
|
||||||
return true;
|
return true;
|
||||||
|
|
||||||
|
// The graph names functions by qualified name; the
|
||||||
|
// store keys them by USR so overloads stay apart.
|
||||||
|
const std::string summary_key = ::fold::summaryStoreKey(declaration);
|
||||||
const std::uint64_t source_hash =
|
const std::uint64_t source_hash =
|
||||||
detect::hashFunctionBody(declaration, source_manager);
|
detect::hashFunctionBody(declaration, source_manager);
|
||||||
auto& store = detect::SummaryStore::instance();
|
auto& store = detect::SummaryStore::instance();
|
||||||
const auto* previous = store.lookup(qualified_name);
|
const auto* previous = store.lookup(summary_key);
|
||||||
if (previous && store.isStale(qualified_name, source_hash)) {
|
if (previous && store.isStale(summary_key, source_hash)) {
|
||||||
store.invalidate(qualified_name);
|
store.invalidate(summary_key);
|
||||||
previous = nullptr;
|
previous = nullptr;
|
||||||
}
|
}
|
||||||
const std::string before =
|
const std::string before =
|
||||||
previous ? previous->serialize() : std::string{};
|
previous ? previous->serialize() : std::string{};
|
||||||
|
|
||||||
if (!force_recompute_ && previous &&
|
if (!force_recompute_ && previous &&
|
||||||
!store.isStale(qualified_name, source_hash)) {
|
!store.isStale(summary_key, source_hash)) {
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -106,15 +110,15 @@ bool computeSummariesForFunctions(clang::tooling::CompilationDatabase& db,
|
||||||
if (auto summary = ::fold::summarizeLeafFunctionSymbolically(
|
if (auto summary = ::fold::summarizeLeafFunctionSymbolically(
|
||||||
declaration, context_)) {
|
declaration, context_)) {
|
||||||
const std::string after = summary->serialize();
|
const std::string after = summary->serialize();
|
||||||
store.store(qualified_name, *summary);
|
store.store(summary_key, *summary);
|
||||||
store.markComputed(qualified_name, source_hash);
|
store.markComputed(summary_key, source_hash);
|
||||||
markChanged(after, before);
|
markChanged(after, before);
|
||||||
}
|
}
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
(void)::fold::ensureSymbolicSummary(declaration, context_);
|
(void)::fold::ensureSymbolicSummary(declaration, context_);
|
||||||
if (const auto* current = store.lookup(qualified_name))
|
if (const auto* current = store.lookup(summary_key))
|
||||||
markChanged(current->serialize(), before);
|
markChanged(current->serialize(), before);
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -34,6 +34,7 @@
|
||||||
#include "fold/fuzz/param_schema.hpp"
|
#include "fold/fuzz/param_schema.hpp"
|
||||||
#include "fold/ir/normalize.hpp"
|
#include "fold/ir/normalize.hpp"
|
||||||
#include "fold/symbolic/encoder.hpp"
|
#include "fold/symbolic/encoder.hpp"
|
||||||
|
#include "fold/symbolic/encoder_callee_names.hpp"
|
||||||
#include "fold/symbolic/expr_hash.hpp"
|
#include "fold/symbolic/expr_hash.hpp"
|
||||||
#include "fold/symbolic/interval.hpp"
|
#include "fold/symbolic/interval.hpp"
|
||||||
#include "fold/symbolic/safety_observer.hpp"
|
#include "fold/symbolic/safety_observer.hpp"
|
||||||
|
|
@ -609,7 +610,7 @@ class DivByZeroObserver : public SafetyObserver {
|
||||||
const symbolic::SymbolicSummary* summary = nullptr;
|
const symbolic::SymbolicSummary* summary = nullptr;
|
||||||
{
|
{
|
||||||
SolverStats::ScopedPhaseTimer _t(&stats_, &SolverStats::summary_lookup_ms);
|
SolverStats::ScopedPhaseTimer _t(&stats_, &SolverStats::summary_lookup_ms);
|
||||||
summary = detect::SummaryStore::instance().lookup(callee_name);
|
summary = detect::SummaryStore::instance().lookup(summaryStoreKey(callee));
|
||||||
}
|
}
|
||||||
if (!summary || summary->param_pre.empty())
|
if (!summary || summary->param_pre.empty())
|
||||||
return false;
|
return false;
|
||||||
|
|
|
||||||
|
|
@ -36,6 +36,7 @@
|
||||||
#include "fold/ir/def_use.hpp"
|
#include "fold/ir/def_use.hpp"
|
||||||
#include "fold/symbolic/cfg_helpers.hpp"
|
#include "fold/symbolic/cfg_helpers.hpp"
|
||||||
#include "fold/symbolic/encoder.hpp"
|
#include "fold/symbolic/encoder.hpp"
|
||||||
|
#include "fold/symbolic/encoder_callee_names.hpp"
|
||||||
#include "fold/symbolic/expr_hash.hpp"
|
#include "fold/symbolic/expr_hash.hpp"
|
||||||
#include "fold/symbolic/heap_model.hpp"
|
#include "fold/symbolic/heap_model.hpp"
|
||||||
#include "fold/symbolic/safety_observer.hpp"
|
#include "fold/symbolic/safety_observer.hpp"
|
||||||
|
|
@ -765,7 +766,7 @@ class NullDerefObserver : public SafetyObserver {
|
||||||
const symbolic::SymbolicSummary* summary = nullptr;
|
const symbolic::SymbolicSummary* summary = nullptr;
|
||||||
{
|
{
|
||||||
SolverStats::ScopedPhaseTimer _t(&stats_, &SolverStats::summary_lookup_ms);
|
SolverStats::ScopedPhaseTimer _t(&stats_, &SolverStats::summary_lookup_ms);
|
||||||
summary = detect::SummaryStore::instance().lookup(callee_name);
|
summary = detect::SummaryStore::instance().lookup(summaryStoreKey(callee));
|
||||||
}
|
}
|
||||||
if (!summary || summary->param_pre.empty())
|
if (!summary || summary->param_pre.empty())
|
||||||
return false;
|
return false;
|
||||||
|
|
@ -790,6 +791,13 @@ class NullDerefObserver : public SafetyObserver {
|
||||||
// — skip both the solver query and the emission.
|
// — skip both the solver query and the emission.
|
||||||
if (vd && env && env->isDefinitelyNonNull(vd))
|
if (vd && env && env->isDefinitelyNonNull(vd))
|
||||||
continue;
|
continue;
|
||||||
|
// Same contract filters as the callee-deref path below; static
|
||||||
|
// callees reach here now that summaries are keyed by USR.
|
||||||
|
if ((vd && current_debugassert_nonnull_.count(vd->getNameAsString()) > 0) ||
|
||||||
|
staticCalleeParamContractProvesNonNull(callee, idx)) {
|
||||||
|
++stats_.abstract_unsat;
|
||||||
|
continue;
|
||||||
|
}
|
||||||
z3::check_result result = checkPreconditionViolation(*predicate, path, loc);
|
z3::check_result result = checkPreconditionViolation(*predicate, path, loc);
|
||||||
if (result == z3::unsat || !emitted.insert(idx).second)
|
if (result == z3::unsat || !emitted.insert(idx).second)
|
||||||
continue;
|
continue;
|
||||||
|
|
|
||||||
|
|
@ -34,6 +34,7 @@
|
||||||
#include "fold/detect/z3_model_support.hpp"
|
#include "fold/detect/z3_model_support.hpp"
|
||||||
#include "fold/fuzz/param_schema.hpp"
|
#include "fold/fuzz/param_schema.hpp"
|
||||||
#include "fold/symbolic/encoder.hpp"
|
#include "fold/symbolic/encoder.hpp"
|
||||||
|
#include "fold/symbolic/encoder_callee_names.hpp"
|
||||||
#include "fold/symbolic/encoder_detail.hpp"
|
#include "fold/symbolic/encoder_detail.hpp"
|
||||||
#include "fold/symbolic/expr_hash.hpp"
|
#include "fold/symbolic/expr_hash.hpp"
|
||||||
#include "fold/symbolic/heap_model.hpp"
|
#include "fold/symbolic/heap_model.hpp"
|
||||||
|
|
@ -406,7 +407,7 @@ class OOBObserver : public SafetyObserver {
|
||||||
const auto* call = call_ir.asClangCallExpr();
|
const auto* call = call_ir.asClangCallExpr();
|
||||||
(void)env;
|
(void)env;
|
||||||
const auto* callee = call ? call->getDirectCallee() : nullptr;
|
const auto* callee = call ? call->getDirectCallee() : nullptr;
|
||||||
const std::string summary_name = qualifiedFunctionName(callee);
|
const std::string summary_name = summaryStoreKey(callee);
|
||||||
const std::string callee_name = callee ? callee->getQualifiedNameAsString() : std::string{};
|
const std::string callee_name = callee ? callee->getQualifiedNameAsString() : std::string{};
|
||||||
if (callee_name.empty())
|
if (callee_name.empty())
|
||||||
return;
|
return;
|
||||||
|
|
|
||||||
|
|
@ -7,6 +7,8 @@
|
||||||
#include <clang/AST/ExprCXX.h>
|
#include <clang/AST/ExprCXX.h>
|
||||||
#include <clang/Basic/SourceManager.h>
|
#include <clang/Basic/SourceManager.h>
|
||||||
|
|
||||||
|
#include "fold/frontend/stable_identity.hpp"
|
||||||
|
|
||||||
namespace fold {
|
namespace fold {
|
||||||
|
|
||||||
std::string calleeName(const clang::CallExpr* ce) {
|
std::string calleeName(const clang::CallExpr* ce) {
|
||||||
|
|
@ -61,6 +63,14 @@ std::string qualifiedFunctionName(const clang::FunctionDecl* fd) {
|
||||||
return name;
|
return name;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
std::string summaryStoreKey(const clang::FunctionDecl* fd) {
|
||||||
|
return fd == nullptr ? std::string{} : semanticFunctionId(fd);
|
||||||
|
}
|
||||||
|
|
||||||
|
std::string summaryStoreKeyForCall(const clang::CallExpr* ce) {
|
||||||
|
return ce == nullptr ? std::string{} : summaryStoreKey(ce->getDirectCallee());
|
||||||
|
}
|
||||||
|
|
||||||
bool isCLibraryFunction(const clang::FunctionDecl* fd) {
|
bool isCLibraryFunction(const clang::FunctionDecl* fd) {
|
||||||
if (fd == nullptr || clang::isa<clang::CXXMethodDecl>(fd))
|
if (fd == nullptr || clang::isa<clang::CXXMethodDecl>(fd))
|
||||||
return false;
|
return false;
|
||||||
|
|
|
||||||
|
|
@ -21,6 +21,7 @@
|
||||||
#include "fold/detect/incremental_cache.hpp"
|
#include "fold/detect/incremental_cache.hpp"
|
||||||
#include "fold/detect/summary_store.hpp"
|
#include "fold/detect/summary_store.hpp"
|
||||||
#include "fold/symbolic/cfg_helpers.hpp"
|
#include "fold/symbolic/cfg_helpers.hpp"
|
||||||
|
#include "fold/symbolic/encoder_callee_names.hpp"
|
||||||
#include "fold/symbolic/safety_observer.hpp"
|
#include "fold/symbolic/safety_observer.hpp"
|
||||||
|
|
||||||
namespace fold {
|
namespace fold {
|
||||||
|
|
@ -165,7 +166,7 @@ class SummaryMetadataCollector : public clang::RecursiveASTVisitor<SummaryMetada
|
||||||
const auto* callee = call ? call->getDirectCallee() : nullptr;
|
const auto* callee = call ? call->getDirectCallee() : nullptr;
|
||||||
if (!callee)
|
if (!callee)
|
||||||
return true;
|
return true;
|
||||||
std::string qname = qualifiedFunctionName(callee);
|
std::string qname = summaryStoreKey(callee);
|
||||||
if (!qname.empty()) {
|
if (!qname.empty()) {
|
||||||
callees.insert(qname);
|
callees.insert(qname);
|
||||||
callee_decls.emplace(std::move(qname), callee);
|
callee_decls.emplace(std::move(qname), callee);
|
||||||
|
|
@ -292,7 +293,7 @@ class SummaryPreconditionRecorder : public SafetyObserver {
|
||||||
const z3::expr& path, const ir::SourceLoc& /*loc*/,
|
const z3::expr& path, const ir::SourceLoc& /*loc*/,
|
||||||
const SymbolicEnv* /*env*/ = nullptr) override {
|
const SymbolicEnv* /*env*/ = nullptr) override {
|
||||||
const auto* call = call_ir.asClangCallExpr();
|
const auto* call = call_ir.asClangCallExpr();
|
||||||
const std::string callee_name = qualifiedCalleeName(call);
|
const std::string callee_name = summaryStoreKeyForCall(call);
|
||||||
if (callee_name.empty())
|
if (callee_name.empty())
|
||||||
return;
|
return;
|
||||||
const auto* summary = detect::SummaryStore::instance().lookup(callee_name);
|
const auto* summary = detect::SummaryStore::instance().lookup(callee_name);
|
||||||
|
|
@ -521,7 +522,7 @@ class SummaryWalker {
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
if (const auto* ce = dyn_cast<CallExpr>(stmt)) {
|
if (const auto* ce = dyn_cast<CallExpr>(stmt)) {
|
||||||
const std::string callee_name = qualifiedCalleeName(ce);
|
const std::string callee_name = summaryStoreKeyForCall(ce);
|
||||||
if (!callee_name.empty()) {
|
if (!callee_name.empty()) {
|
||||||
auto& store = detect::SummaryStore::instance();
|
auto& store = detect::SummaryStore::instance();
|
||||||
const auto* summary = store.lookup(callee_name);
|
const auto* summary = store.lookup(callee_name);
|
||||||
|
|
@ -569,7 +570,7 @@ class SummaryWalker {
|
||||||
|
|
||||||
private:
|
private:
|
||||||
std::optional<z3::expr> summarizeCallReturn(const clang::CallExpr* ce, const z3::expr& path) {
|
std::optional<z3::expr> summarizeCallReturn(const clang::CallExpr* ce, const z3::expr& path) {
|
||||||
const std::string callee_name = qualifiedCalleeName(ce);
|
const std::string callee_name = summaryStoreKeyForCall(ce);
|
||||||
if (callee_name.empty())
|
if (callee_name.empty())
|
||||||
return std::nullopt;
|
return std::nullopt;
|
||||||
|
|
||||||
|
|
@ -656,7 +657,7 @@ std::optional<FunctionSummarized> summarizeImpl(const clang::FunctionDecl* fd,
|
||||||
params.push_back(p);
|
params.push_back(p);
|
||||||
|
|
||||||
std::vector<ExitState> exits;
|
std::vector<ExitState> exits;
|
||||||
SummaryWalker walker(ctx, env, exits, params, qualifiedFunctionName(fd));
|
SummaryWalker walker(ctx, env, exits, params, summaryStoreKey(fd));
|
||||||
z3::expr path = env.ctx().bool_val(true);
|
z3::expr path = env.ctx().bool_val(true);
|
||||||
bool ok = walker.walk(fd->getBody(), path);
|
bool ok = walker.walk(fd->getBody(), path);
|
||||||
SoundnessTag tag = env.tag();
|
SoundnessTag tag = env.tag();
|
||||||
|
|
@ -695,7 +696,7 @@ std::optional<symbolic::SymbolicSummary> computeSummaryForDefinition(const clang
|
||||||
if (!def)
|
if (!def)
|
||||||
return std::nullopt;
|
return std::nullopt;
|
||||||
|
|
||||||
std::string qualified_name = qualifiedFunctionName(def);
|
std::string qualified_name = summaryStoreKey(def);
|
||||||
if (qualified_name.empty())
|
if (qualified_name.empty())
|
||||||
return std::nullopt;
|
return std::nullopt;
|
||||||
|
|
||||||
|
|
@ -807,7 +808,7 @@ summarizeLeafFunctionSymbolically(const clang::FunctionDecl* fd, const clang::AS
|
||||||
SummaryPreconditionRecorder preconditions(ctx, env, raw_inputs, input_names);
|
SummaryPreconditionRecorder preconditions(ctx, env, raw_inputs, input_names);
|
||||||
env.setObserver(&preconditions);
|
env.setObserver(&preconditions);
|
||||||
std::vector<ExitState> exits;
|
std::vector<ExitState> exits;
|
||||||
SummaryWalker walker(ctx, env, exits, params, qualifiedFunctionName(fd), &preconditions);
|
SummaryWalker walker(ctx, env, exits, params, summaryStoreKey(fd), &preconditions);
|
||||||
z3::expr path = env.ctx().bool_val(true);
|
z3::expr path = env.ctx().bool_val(true);
|
||||||
bool ok = walker.walk(fd->getBody(), path);
|
bool ok = walker.walk(fd->getBody(), path);
|
||||||
|
|
||||||
|
|
@ -899,7 +900,7 @@ summarizeLeafFunctionSymbolically(const clang::FunctionDecl* fd, const clang::AS
|
||||||
|
|
||||||
SummaryMetadataCollector metadata;
|
SummaryMetadataCollector metadata;
|
||||||
metadata.TraverseStmt(fd->getBody());
|
metadata.TraverseStmt(fd->getBody());
|
||||||
const std::string caller_name = qualifiedFunctionName(fd);
|
const std::string caller_name = summaryStoreKey(fd);
|
||||||
summary.reads_globals = std::move(metadata.reads_globals);
|
summary.reads_globals = std::move(metadata.reads_globals);
|
||||||
summary.writes_globals = std::move(metadata.writes_globals);
|
summary.writes_globals = std::move(metadata.writes_globals);
|
||||||
bool params_unchanged = can_emit_value_summary;
|
bool params_unchanged = can_emit_value_summary;
|
||||||
|
|
@ -943,7 +944,7 @@ const symbolic::SymbolicSummary* ensureSymbolicSummary(const clang::FunctionDecl
|
||||||
if (!fd)
|
if (!fd)
|
||||||
return nullptr;
|
return nullptr;
|
||||||
|
|
||||||
std::string qualified_name = qualifiedFunctionName(fd);
|
std::string qualified_name = summaryStoreKey(fd);
|
||||||
if (qualified_name.empty())
|
if (qualified_name.empty())
|
||||||
return nullptr;
|
return nullptr;
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -1,4 +1,7 @@
|
||||||
|
#include <clang/AST/RecursiveASTVisitor.h>
|
||||||
|
#include <clang/Frontend/ASTUnit.h>
|
||||||
#include <clang/Tooling/CompilationDatabase.h>
|
#include <clang/Tooling/CompilationDatabase.h>
|
||||||
|
#include <clang/Tooling/Tooling.h>
|
||||||
|
|
||||||
#include <algorithm>
|
#include <algorithm>
|
||||||
#include <catch2/catch_test_macros.hpp>
|
#include <catch2/catch_test_macros.hpp>
|
||||||
|
|
@ -21,6 +24,7 @@
|
||||||
#include "fold/detect/summary_store.hpp"
|
#include "fold/detect/summary_store.hpp"
|
||||||
#include "fold/frontend/comp_db.hpp"
|
#include "fold/frontend/comp_db.hpp"
|
||||||
#include "fold/ir/call_graph.hpp"
|
#include "fold/ir/call_graph.hpp"
|
||||||
|
#include "fold/symbolic/encoder_callee_names.hpp"
|
||||||
|
|
||||||
namespace {
|
namespace {
|
||||||
|
|
||||||
|
|
@ -37,6 +41,28 @@ struct TempDir {
|
||||||
~TempDir() { std::filesystem::remove_all(path); }
|
~TempDir() { std::filesystem::remove_all(path); }
|
||||||
};
|
};
|
||||||
|
|
||||||
|
// SummaryStore key (USR-based) of the function `name` defined in `path`,
|
||||||
|
// computed from the same file the scan parsed.
|
||||||
|
std::string summaryKeyIn(const std::filesystem::path& path, const std::string& name) {
|
||||||
|
std::ifstream in(path);
|
||||||
|
const std::string code((std::istreambuf_iterator<char>(in)), std::istreambuf_iterator<char>());
|
||||||
|
auto unit = clang::tooling::buildASTFromCodeWithArgs(code, {"-std=c++20"}, path.string());
|
||||||
|
if (!unit)
|
||||||
|
return {};
|
||||||
|
struct Finder : clang::RecursiveASTVisitor<Finder> {
|
||||||
|
std::string name;
|
||||||
|
const clang::FunctionDecl* found = nullptr;
|
||||||
|
bool VisitFunctionDecl(clang::FunctionDecl* fd) {
|
||||||
|
if (!found && fd->getNameAsString() == name)
|
||||||
|
found = fd;
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
} finder;
|
||||||
|
finder.name = name;
|
||||||
|
finder.TraverseDecl(unit->getASTContext().getTranslationUnitDecl());
|
||||||
|
return fold::summaryStoreKey(finder.found);
|
||||||
|
}
|
||||||
|
|
||||||
void writeFile(const std::filesystem::path& path, const std::string& content) {
|
void writeFile(const std::filesystem::path& path, const std::string& content) {
|
||||||
std::ofstream(path) << content;
|
std::ofstream(path) << content;
|
||||||
}
|
}
|
||||||
|
|
@ -478,8 +504,10 @@ TEST_CASE("symbolic summary priming reaches recursive SCC fixpoint", "[scan][sum
|
||||||
auto result = fold::cli::runDetectorsOnShard(*db, opts, {caller.string(), callee.string()},
|
auto result = fold::cli::runDetectorsOnShard(*db, opts, {caller.string(), callee.string()},
|
||||||
/*include_clone_clustering=*/false);
|
/*include_clone_clustering=*/false);
|
||||||
|
|
||||||
const auto* a_summary = fold::detect::SummaryStore::instance().lookup("a");
|
const auto* a_summary =
|
||||||
const auto* b_summary = fold::detect::SummaryStore::instance().lookup("b");
|
fold::detect::SummaryStore::instance().lookup(summaryKeyIn(caller, "a"));
|
||||||
|
const auto* b_summary =
|
||||||
|
fold::detect::SummaryStore::instance().lookup(summaryKeyIn(callee, "b"));
|
||||||
REQUIRE(b_summary != nullptr);
|
REQUIRE(b_summary != nullptr);
|
||||||
REQUIRE(a_summary != nullptr);
|
REQUIRE(a_summary != nullptr);
|
||||||
REQUIRE(!a_summary->param_pre.empty());
|
REQUIRE(!a_summary->param_pre.empty());
|
||||||
|
|
@ -520,10 +548,10 @@ TEST_CASE("symbolic summaries distinguish internal linkage functions by file",
|
||||||
(void)fold::cli::runDetectorsOnShard(*db, opts, {unsafe.string(), safe.string()},
|
(void)fold::cli::runDetectorsOnShard(*db, opts, {unsafe.string(), safe.string()},
|
||||||
/*include_clone_clustering=*/false);
|
/*include_clone_clustering=*/false);
|
||||||
|
|
||||||
const auto* unsafe_summary = fold::detect::SummaryStore::instance().lookup(
|
const auto* unsafe_summary =
|
||||||
std::string("require_nonzero@") + unsafe.string());
|
fold::detect::SummaryStore::instance().lookup(summaryKeyIn(unsafe, "require_nonzero"));
|
||||||
const auto* safe_summary = fold::detect::SummaryStore::instance().lookup(
|
const auto* safe_summary =
|
||||||
std::string("require_nonzero@") + safe.string());
|
fold::detect::SummaryStore::instance().lookup(summaryKeyIn(safe, "require_nonzero"));
|
||||||
|
|
||||||
REQUIRE(unsafe_summary != nullptr);
|
REQUIRE(unsafe_summary != nullptr);
|
||||||
REQUIRE(safe_summary != nullptr);
|
REQUIRE(safe_summary != nullptr);
|
||||||
|
|
|
||||||
|
|
@ -222,6 +222,15 @@ test_scan_coverage = executable(
|
||||||
)
|
)
|
||||||
test('scan_coverage', test_scan_coverage)
|
test('scan_coverage', test_scan_coverage)
|
||||||
|
|
||||||
|
test_summary_key_identity = executable(
|
||||||
|
'test_summary_key_identity',
|
||||||
|
'test_summary_key_identity.cpp',
|
||||||
|
dependencies: [fold_core_dep, catch2_dep],
|
||||||
|
install: false,
|
||||||
|
build_by_default: false,
|
||||||
|
)
|
||||||
|
test('summary_key_identity', test_summary_key_identity)
|
||||||
|
|
||||||
test_batch_safety_observer = executable(
|
test_batch_safety_observer = executable(
|
||||||
'test_batch_safety_observer',
|
'test_batch_safety_observer',
|
||||||
'test_batch_safety_observer.cpp',
|
'test_batch_safety_observer.cpp',
|
||||||
|
|
|
||||||
141
tests/unit/test_summary_key_identity.cpp
Normal file
141
tests/unit/test_summary_key_identity.cpp
Normal file
|
|
@ -0,0 +1,141 @@
|
||||||
|
#include <clang/AST/RecursiveASTVisitor.h>
|
||||||
|
#include <clang/Frontend/ASTUnit.h>
|
||||||
|
#include <clang/Tooling/Tooling.h>
|
||||||
|
|
||||||
|
#include <catch2/catch_test_macros.hpp>
|
||||||
|
#include <memory>
|
||||||
|
#include <string>
|
||||||
|
#include <vector>
|
||||||
|
|
||||||
|
#include "fold/detect/null_deref.hpp"
|
||||||
|
#include "fold/detect/summary_store.hpp"
|
||||||
|
#include "fold/report/finding.hpp"
|
||||||
|
#include "fold/symbolic/budget.hpp"
|
||||||
|
#include "fold/symbolic/encoder_callee_names.hpp"
|
||||||
|
#include "fold/symbolic/summary.hpp"
|
||||||
|
#include "fold/symbolic/summary_encoder.hpp"
|
||||||
|
|
||||||
|
// Function summaries were stored under the qualified name, which overloads
|
||||||
|
// share: `take(int*, int)` (dereferences p) and `take(int*, long)` (checks p)
|
||||||
|
// both mapped to "take", so whichever was summarized first answered for both.
|
||||||
|
// A caller of the null-checking overload then inherited a nonnull
|
||||||
|
// precondition it does not have, and vice versa. Summaries are now keyed by
|
||||||
|
// the declaration's USR.
|
||||||
|
|
||||||
|
namespace {
|
||||||
|
|
||||||
|
std::unique_ptr<clang::ASTUnit> parse(const std::string& code) {
|
||||||
|
return clang::tooling::buildASTFromCodeWithArgs(code, {"-std=c++20"}, "input.cpp");
|
||||||
|
}
|
||||||
|
|
||||||
|
std::vector<const clang::FunctionDecl*> definitionsNamed(clang::ASTUnit& unit,
|
||||||
|
const std::string& name) {
|
||||||
|
struct Finder : clang::RecursiveASTVisitor<Finder> {
|
||||||
|
std::string name;
|
||||||
|
std::vector<const clang::FunctionDecl*> found;
|
||||||
|
bool VisitFunctionDecl(clang::FunctionDecl* fd) {
|
||||||
|
if (fd->getNameAsString() == name && fd->isThisDeclarationADefinition())
|
||||||
|
found.push_back(fd);
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
} finder;
|
||||||
|
finder.name = name;
|
||||||
|
finder.TraverseDecl(unit.getASTContext().getTranslationUnitDecl());
|
||||||
|
return finder.found;
|
||||||
|
}
|
||||||
|
|
||||||
|
bool requiresNonNull(const fold::symbolic::SymbolicSummary* summary, unsigned index) {
|
||||||
|
if (summary == nullptr)
|
||||||
|
return false;
|
||||||
|
const auto it = summary->param_pre.find(index);
|
||||||
|
if (it == summary->param_pre.end())
|
||||||
|
return false;
|
||||||
|
for (const auto& term : it->second)
|
||||||
|
if (term.kind == "nonnull")
|
||||||
|
return true;
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
} // namespace
|
||||||
|
|
||||||
|
TEST_CASE("overloads get separate summaries", "[summary-key]") {
|
||||||
|
fold::detect::SummaryStore::instance().clear();
|
||||||
|
auto unit = parse("int take(int* p, int) { return *p; }\n"
|
||||||
|
"int take(int* p, long) { return p ? *p : 0; }\n");
|
||||||
|
REQUIRE(unit);
|
||||||
|
const auto defs = definitionsNamed(*unit, "take");
|
||||||
|
REQUIRE(defs.size() == 2);
|
||||||
|
const auto& ctx = unit->getASTContext();
|
||||||
|
|
||||||
|
SECTION("dereferencing overload first") {
|
||||||
|
CHECK(requiresNonNull(fold::ensureSymbolicSummary(defs[0], ctx), 0));
|
||||||
|
CHECK_FALSE(requiresNonNull(fold::ensureSymbolicSummary(defs[1], ctx), 0));
|
||||||
|
}
|
||||||
|
SECTION("null-checking overload first") {
|
||||||
|
CHECK_FALSE(requiresNonNull(fold::ensureSymbolicSummary(defs[1], ctx), 0));
|
||||||
|
CHECK(requiresNonNull(fold::ensureSymbolicSummary(defs[0], ctx), 0));
|
||||||
|
}
|
||||||
|
fold::detect::SummaryStore::instance().clear();
|
||||||
|
}
|
||||||
|
|
||||||
|
TEST_CASE("summary keys: identity across redeclarations, not across names", "[summary-key]") {
|
||||||
|
auto unit = parse("int f(int* p);\n"
|
||||||
|
"int f(int* p) { return *p; }\n"
|
||||||
|
"namespace a { int g(int* p) { return *p; } }\n"
|
||||||
|
"namespace b { int g(int* p) { return p ? *p : 0; } }\n");
|
||||||
|
REQUIRE(unit);
|
||||||
|
struct Finder : clang::RecursiveASTVisitor<Finder> {
|
||||||
|
std::vector<const clang::FunctionDecl*> f_decls;
|
||||||
|
std::vector<const clang::FunctionDecl*> g_defs;
|
||||||
|
bool VisitFunctionDecl(clang::FunctionDecl* fd) {
|
||||||
|
if (fd->getNameAsString() == "f")
|
||||||
|
f_decls.push_back(fd);
|
||||||
|
if (fd->getNameAsString() == "g")
|
||||||
|
g_defs.push_back(fd);
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
} finder;
|
||||||
|
finder.TraverseDecl(unit->getASTContext().getTranslationUnitDecl());
|
||||||
|
REQUIRE(finder.f_decls.size() == 2);
|
||||||
|
REQUIRE(finder.g_defs.size() == 2);
|
||||||
|
CHECK(fold::summaryStoreKey(finder.f_decls[0]) == fold::summaryStoreKey(finder.f_decls[1]));
|
||||||
|
CHECK(fold::summaryStoreKey(finder.g_defs[0]) != fold::summaryStoreKey(finder.g_defs[1]));
|
||||||
|
CHECK_FALSE(fold::summaryStoreKey(finder.f_decls[0]).empty());
|
||||||
|
CHECK(fold::summaryStoreKey(nullptr).empty());
|
||||||
|
}
|
||||||
|
|
||||||
|
TEST_CASE("null-deref: a cross-TU call uses its own overload's summary",
|
||||||
|
"[summary-key][null-deref]") {
|
||||||
|
// TU 1 defines both overloads and summarizes them (the dereferencing one
|
||||||
|
// last). TU 2 sees only declarations, so its call resolves through the
|
||||||
|
// store alone, and passes null to the overload that checks.
|
||||||
|
const std::string defs = "int take(int* p, long) { return p ? *p : 0; }\n"
|
||||||
|
"int take(int* p, int) { return *p; }\n";
|
||||||
|
const std::string caller = "int take(int* p, long);\n"
|
||||||
|
"int take(int* p, int);\n"
|
||||||
|
"int caller() { int* q = nullptr; return take(q, 1L); }\n"
|
||||||
|
"int control() { int* q = nullptr; return take(q, 1); }\n";
|
||||||
|
fold::detect::SummaryStore::instance().clear();
|
||||||
|
auto defs_unit = parse(defs);
|
||||||
|
REQUIRE(defs_unit);
|
||||||
|
for (const auto* fd : definitionsNamed(*defs_unit, "take"))
|
||||||
|
(void)fold::ensureSymbolicSummary(fd, defs_unit->getASTContext());
|
||||||
|
|
||||||
|
auto caller_unit = parse(caller);
|
||||||
|
REQUIRE(caller_unit);
|
||||||
|
fold::SolverBudget b;
|
||||||
|
fold::SolverStats s;
|
||||||
|
const auto findings = fold::detectNullDerefInContext(caller_unit->getASTContext(), b, s);
|
||||||
|
fold::detect::SummaryStore::instance().clear();
|
||||||
|
std::size_t in_caller = 0;
|
||||||
|
std::size_t in_control = 0;
|
||||||
|
for (const auto& f : findings) {
|
||||||
|
if (!f.locations.empty() && f.locations.front().line == 3)
|
||||||
|
++in_caller;
|
||||||
|
if (!f.locations.empty() && f.locations.front().line == 4)
|
||||||
|
++in_control;
|
||||||
|
}
|
||||||
|
CHECK(in_caller == 0);
|
||||||
|
// Control: the dereferencing overload's precondition still applies.
|
||||||
|
CHECK(in_control == 1);
|
||||||
|
}
|
||||||
|
|
@ -11,6 +11,7 @@
|
||||||
#include "fold/detect/null_deref.hpp"
|
#include "fold/detect/null_deref.hpp"
|
||||||
#include "fold/detect/summary_store.hpp"
|
#include "fold/detect/summary_store.hpp"
|
||||||
#include "fold/symbolic/budget.hpp"
|
#include "fold/symbolic/budget.hpp"
|
||||||
|
#include "fold/symbolic/encoder_callee_names.hpp"
|
||||||
#include "fold/symbolic/summary_encoder.hpp"
|
#include "fold/symbolic/summary_encoder.hpp"
|
||||||
|
|
||||||
namespace {
|
namespace {
|
||||||
|
|
@ -37,7 +38,7 @@ void primeLeafSummary(clang::ASTUnit& unit, const std::string& name) {
|
||||||
REQUIRE(fd != nullptr);
|
REQUIRE(fd != nullptr);
|
||||||
auto summary = fold::summarizeLeafFunctionSymbolically(fd, ctx);
|
auto summary = fold::summarizeLeafFunctionSymbolically(fd, ctx);
|
||||||
REQUIRE(summary);
|
REQUIRE(summary);
|
||||||
fold::detect::SummaryStore::instance().store(name, *summary);
|
fold::detect::SummaryStore::instance().store(fold::summaryStoreKey(fd), *summary);
|
||||||
}
|
}
|
||||||
|
|
||||||
} // namespace
|
} // namespace
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue