================ @@ -0,0 +1,331 @@ +//===- NullTerminatedChecker.cpp - Check null_terminated params -*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +// +// This defines NullTerminatedChecker, which checks for arguments treated as +// buffers that are expected to be null-terminated (ends with a zero-valued +// element). A constant-size array is considered null-terminated if any of its +// elements may be zero on the current path. +// +// Parameters are marked as expecting null-terminated buffers using: +// __attribute__((annotate("null_terminated"))) +// +//===----------------------------------------------------------------------===// + +#include "clang/AST/Attr.h" +#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" +#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" +#include "clang/StaticAnalyzer/Core/Checker.h" +#include "clang/StaticAnalyzer/Core/CheckerManager.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" +#include "llvm/ADT/SmallBitVector.h" + +using namespace clang; +using namespace ento; + +namespace { +class NullTerminatedChecker : public Checker<check::PreCall> { +public: + // TODO: region-store-max-binding-fanout defaults to 128, meaning a single + // bind only covers that many elements. The 1024 option here is only truly + // respected when the array is built by separate bind operations, e.g., + // the case of straight-line writes: + // + // int a[500]; + // a[0] = val; + // a[1] = val; + // ... + // a[499] = val; + int MaxArraySize = 1024; + + void checkPreCall(const CallEvent &Call, CheckerContext &C) const; + +private: + const BugType BT{this, "Array not null-terminated", "API"}; + + /// Return true if the parameter has annotate("null_terminated"). + static bool isNullTerminatedParam(const ParmVarDecl *Param); + + /// Return true if any element in [0, \p ArraySize) can be zero. + bool mayContainZeroElement(ProgramStateRef State, SValBuilder &SVB, + QualType EltTy, uint64_t ArraySize, + const TypedValueRegion *Arr) const; +}; + +/// Return true if we can't prove \p Val is non-zero on the current path. +bool mayBeZero(ProgramStateRef State, SValBuilder &SVB, QualType Ty, SVal Val) { + // Unknown or undefined: can't reason either way. + auto DV = Val.getAs<DefinedSVal>(); + if (!DV) + return true; + + // Try the fast lookup first (much cheaper than assuming condition for + // concrete values). + ConditionTruthVal IsZero = State->isNull(*DV); + if (IsZero.isConstrainedFalse()) + return false; + if (IsZero.isConstrainedTrue()) + return true; + + // For an atomic symbol, the solver won't do any better than the preceeding + // check, so we cannot prove anything further (hence it may be zero). + SymbolRef Sym = DV->getAsSymbol(/*IncludeBaseRegion=*/true); + if (Sym && isa<SymbolData>(Sym)) + return true; + + // Worst case: try to solve symbolic expression. + SVal EqZero = SVB.evalEQ(State, *DV, SVB.makeZeroVal(Ty)); + auto EqZeroDV = EqZero.getAs<DefinedSVal>(); + if (!EqZeroDV) + return true; + return static_cast<bool>(State->assume(*EqZeroDV, true)); ---------------- NagyDonat wrote:
I understand why your code is justified for this concrete situation, but I'm worried about the general trend that the analyzer has more and more functions that try to calculate "Can this value be null?" in various ways (e.g. `isNull`, `assume` and its variants, now this checker-local function, IIRC some other checkers also have their own analogous functions, ...). ([Obligatory XKCD.](https://xkcd.com/927/)) This is mostly not your fault; the root of the problem is that the analyzer engine doesn't offer a single clear tool that can be used by the checkers. (In fact, the "menu" of `isNull`-like tools is so complicated that even I'm not familiar with the differences between them.) Eventually it would be nice to clean up this area (including the convoluted, indirect and wasteful logic of the `assume` function family), get rid of the suboptimal choices and implement a few effective tools in the engine that support the needs of all checkers. However, this cleanup is a big task and is clearly out of scope for this PR, so if you think that you need to define your own tool for this task, then I can accept this. https://github.com/llvm/llvm-project/pull/188128 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
