martong updated this revision to Diff 257317.
martong marked an inline comment as done.
martong added a comment.
- Rebase to master
- Use BinaryOperator::negateComparisonOp
- getBufferDynamicSize -> getDynamicSizeWithOffset
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77066/new/
https://reviews.llvm.org/D77066
Files:
clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h
clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
clang/lib/StaticAnalyzer/Core/DynamicSize.cpp
clang/test/Analysis/std-c-library-functions-arg-constraints.c
Index: clang/test/Analysis/std-c-library-functions-arg-constraints.c
===================================================================
--- clang/test/Analysis/std-c-library-functions-arg-constraints.c
+++ clang/test/Analysis/std-c-library-functions-arg-constraints.c
@@ -122,3 +122,30 @@
// bugpath-warning{{Function argument constraint is not satisfied}} \
// bugpath-note{{Function argument constraint is not satisfied}}
}
+
+int __buf_size_arg_constraint(const void *, size_t);
+void test_buf_size_concrete() {
+ char buf[3]; // bugpath-note{{'buf' initialized here}}
+ __buf_size_arg_constraint(buf, 4); // \
+ // report-warning{{Function argument constraint is not satisfied}} \
+ // bugpath-warning{{Function argument constraint is not satisfied}} \
+ // bugpath-note{{Function argument constraint is not satisfied}}
+}
+void test_buf_size_symbolic(int s) {
+ char buf[3];
+ __buf_size_arg_constraint(buf, s);
+ clang_analyzer_eval(s <= 3); // \
+ // report-warning{{TRUE}} \
+ // bugpath-warning{{TRUE}} \
+ // bugpath-note{{TRUE}} \
+ // bugpath-note{{'s' is <= 3}}
+}
+void test_buf_size_symbolic_and_offset(int s) {
+ char buf[3];
+ __buf_size_arg_constraint(buf + 1, s);
+ clang_analyzer_eval(s <= 2); // \
+ // report-warning{{TRUE}} \
+ // bugpath-warning{{TRUE}} \
+ // bugpath-note{{TRUE}} \
+ // bugpath-note{{'s' is <= 2}}
+}
Index: clang/lib/StaticAnalyzer/Core/DynamicSize.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/DynamicSize.cpp
+++ clang/lib/StaticAnalyzer/Core/DynamicSize.cpp
@@ -44,5 +44,28 @@
return DivisionV.castAs<DefinedOrUnknownSVal>();
}
+SVal getDynamicSizeWithOffset(ProgramStateRef State, const SVal &BufV,
+ SValBuilder &SvalBuilder) {
+ const MemRegion *MRegion = BufV.getAsRegion();
+ if (!MRegion)
+ return UnknownVal();
+ RegionOffset Offset = MRegion->getAsOffset();
+ if (Offset.hasSymbolicOffset())
+ return UnknownVal();
+ const MemRegion *BaseRegion = MRegion->getBaseRegion();
+ if (!BaseRegion)
+ return UnknownVal();
+
+ NonLoc OffsetInBytes = SvalBuilder.makeArrayIndex(
+ Offset.getOffset() /
+ MRegion->getMemRegionManager().getContext().getCharWidth());
+ DefinedOrUnknownSVal ExtentInBytes =
+ getDynamicSize(State, BaseRegion, SvalBuilder);
+
+ return SvalBuilder.evalBinOp(State, BinaryOperator::Opcode::BO_Sub,
+ ExtentInBytes, OffsetInBytes,
+ SvalBuilder.getArrayIndexType());
+}
+
} // namespace ento
} // namespace clang
Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -56,6 +56,7 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h"
using namespace clang;
using namespace clang::ento;
@@ -108,7 +109,8 @@
/// Apply the effects of the constraint on the given program state. If null
/// is returned then the constraint is not feasible.
virtual ProgramStateRef apply(ProgramStateRef State, const CallEvent &Call,
- const Summary &Summary) const = 0;
+ const Summary &Summary,
+ CheckerContext &C) const = 0;
virtual ValueConstraintPtr negate() const {
llvm_unreachable("Not implemented");
};
@@ -143,7 +145,8 @@
const Summary &Summary) const;
public:
ProgramStateRef apply(ProgramStateRef State, const CallEvent &Call,
- const Summary &Summary) const override {
+ const Summary &Summary,
+ CheckerContext &C) const override {
switch (Kind) {
case OutOfRange:
return applyAsOutOfRange(State, Call, Summary);
@@ -178,7 +181,8 @@
ArgNo getOtherArgNo() const { return OtherArgN; }
BinaryOperator::Opcode getOpcode() const { return Opcode; }
ProgramStateRef apply(ProgramStateRef State, const CallEvent &Call,
- const Summary &Summary) const override;
+ const Summary &Summary,
+ CheckerContext &C) const override;
};
class NotNullConstraint : public ValueConstraint {
@@ -188,7 +192,8 @@
public:
ProgramStateRef apply(ProgramStateRef State, const CallEvent &Call,
- const Summary &Summary) const override {
+ const Summary &Summary,
+ CheckerContext &C) const override {
SVal V = getArgSVal(Call, getArgNo());
if (V.isUndef())
return State;
@@ -207,6 +212,52 @@
}
};
+ // Represents a buffer argument with an additional size argument.
+ // E.g. the first two arguments here:
+ // ctime_s(char *buffer, rsize_t bufsz, const time_t *time);
+ class BufferSizeConstraint : public ValueConstraint {
+ // The argument which holds the size of the buffer.
+ ArgNo SizeArgN;
+ // The operator we use in apply. This is negated in negate().
+ BinaryOperator::Opcode Op = BO_LE;
+
+ public:
+ BufferSizeConstraint(ArgNo BufArgN, ArgNo SizeArgN)
+ : ValueConstraint(BufArgN), SizeArgN(SizeArgN) {}
+
+ ProgramStateRef apply(ProgramStateRef State, const CallEvent &Call,
+ const Summary &Summary,
+ CheckerContext &C) const override {
+ // The buffer argument.
+ SVal BufV = getArgSVal(Call, getArgNo());
+ // The size argument.
+ SVal SizeV = getArgSVal(Call, SizeArgN);
+ // The dynamic size of the buffer argument, got from the analyzer engine.
+ SVal BufDynSize =
+ getDynamicSizeWithOffset(State, BufV, C.getSValBuilder());
+
+ SValBuilder &SvalBuilder = C.getSValBuilder();
+ SVal Feasible = SvalBuilder.evalBinOp(State, Op, SizeV, BufDynSize,
+ SvalBuilder.getContext().BoolTy);
+ if (auto F = Feasible.getAs<DefinedOrUnknownSVal>())
+ return State->assume(*F, true);
+
+ // We can get here only if the size argument or the dynamic size is
+ // undefined. But the dynamic size should never be undefined, only
+ // unknown. So, here, the size of the argument is undefined, i.e. we
+ // cannot apply the constraint. Actually, other checkers like
+ // CallAndMessage should catch this situation earlier, because we call a
+ // function with an uninitialized argument.
+ return nullptr;
+ }
+
+ ValueConstraintPtr negate() const override {
+ BufferSizeConstraint Tmp(*this);
+ Tmp.Op = BinaryOperator::negateComparisonOp(Op);
+ return std::make_shared<BufferSizeConstraint>(Tmp);
+ }
+ };
+
/// The complete list of constraints that defines a single branch.
typedef std::vector<ValueConstraintPtr> ConstraintSet;
@@ -428,8 +479,8 @@
}
ProgramStateRef StdLibraryFunctionsChecker::ComparisonConstraint::apply(
- ProgramStateRef State, const CallEvent &Call,
- const Summary &Summary) const {
+ ProgramStateRef State, const CallEvent &Call, const Summary &Summary,
+ CheckerContext &C) const {
ProgramStateManager &Mgr = State->getStateManager();
SValBuilder &SVB = Mgr.getSValBuilder();
@@ -460,8 +511,8 @@
ProgramStateRef NewState = State;
for (const ValueConstraintPtr& VC : Summary.ArgConstraints) {
- ProgramStateRef SuccessSt = VC->apply(NewState, Call, Summary);
- ProgramStateRef FailureSt = VC->negate()->apply(NewState, Call, Summary);
+ ProgramStateRef SuccessSt = VC->apply(NewState, Call, Summary, C);
+ ProgramStateRef FailureSt = VC->negate()->apply(NewState, Call, Summary, C);
// The argument constraint is not satisfied.
if (FailureSt && !SuccessSt) {
if (ExplodedNode *N = C.generateErrorNode(NewState))
@@ -494,7 +545,7 @@
for (const auto &VRS : Summary.CaseConstraints) {
ProgramStateRef NewState = State;
for (const auto &VR: VRS) {
- NewState = VR->apply(NewState, Call, Summary);
+ NewState = VR->apply(NewState, Call, Summary, C);
if (!NewState)
break;
}
@@ -684,6 +735,9 @@
IntRangeVector Ranges) {
return std::make_shared<RangeConstraint>(ArgN, Kind, Ranges);
};
+ auto BufferSize = [](ArgNo BufArgN, ArgNo SizeArgN) {
+ return std::make_shared<BufferSizeConstraint>(BufArgN, SizeArgN);
+ };
struct {
auto operator()(RangeKind Kind, IntRangeVector Ranges) {
return std::make_shared<RangeConstraint>(Ret, Kind, Ranges);
@@ -963,7 +1017,12 @@
{"__variadic", Summaries{Summary(ArgTypes{VoidPtrTy, ConstCharPtrTy},
RetType{IntTy}, EvalCallAsPure)
.ArgConstraint(NotNull(ArgNo(0)))
- .ArgConstraint(NotNull(ArgNo(1)))}}};
+ .ArgConstraint(NotNull(ArgNo(1)))}},
+ {"__buf_size_arg_constraint",
+ Summaries{Summary(ArgTypes{ConstVoidPtrTy, SizeTy}, RetType{IntTy},
+ EvalCallAsPure)
+ .ArgConstraint(BufferSize(0, 1))}},
+ };
for (auto &E : TestFunctionSummaryMap) {
auto InsertRes =
FunctionSummaryMap.insert({std::string(E.getKey()), E.getValue()});
Index: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp
+++ clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp
@@ -41,25 +41,7 @@
SVal PlacementNewChecker::getExtentSizeOfPlace(const Expr *Place,
ProgramStateRef State,
CheckerContext &C) const {
- const MemRegion *MRegion = C.getSVal(Place).getAsRegion();
- if (!MRegion)
- return UnknownVal();
- RegionOffset Offset = MRegion->getAsOffset();
- if (Offset.hasSymbolicOffset())
- return UnknownVal();
- const MemRegion *BaseRegion = MRegion->getBaseRegion();
- if (!BaseRegion)
- return UnknownVal();
-
- SValBuilder &SvalBuilder = C.getSValBuilder();
- NonLoc OffsetInBytes = SvalBuilder.makeArrayIndex(
- Offset.getOffset() / C.getASTContext().getCharWidth());
- DefinedOrUnknownSVal ExtentInBytes =
- getDynamicSize(State, BaseRegion, SvalBuilder);
-
- return SvalBuilder.evalBinOp(State, BinaryOperator::Opcode::BO_Sub,
- ExtentInBytes, OffsetInBytes,
- SvalBuilder.getArrayIndexType());
+ return getDynamicSizeWithOffset(State, C.getSVal(Place), C.getSValBuilder());
}
SVal PlacementNewChecker::getExtentSizeOfNewTarget(const CXXNewExpr *NE,
Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h
===================================================================
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h
@@ -32,6 +32,22 @@
SValBuilder &SVB,
QualType ElementTy);
+/// Get the dynamic size for a symbolic value that represents a buffer. If
+/// there is an offsetting to the underlying buffer we consider that too.
+/// Returns with an SVal that represents the size, this is Unknown if the
+/// engine cannot deduce the size.
+/// E.g.
+/// char buf[3];
+/// (buf); // size is 3
+/// (buf + 1); // size is 2
+/// (buf + 3); // size is 0
+/// (buf + 4); // size is -1
+///
+/// char *bufptr;
+/// (bufptr) // size is unknown
+SVal getDynamicSizeWithOffset(ProgramStateRef State, const SVal &BufV,
+ SValBuilder &SvalBuilder);
+
} // namespace ento
} // namespace clang
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
