diff --git a/clang/include/clang/Driver/CC1Options.td b/clang/include/clang/Driver/CC1Options.td index 28a31cacab4f..1eefc78b3a58 100644 --- a/clang/include/clang/Driver/CC1Options.td +++ b/clang/include/clang/Driver/CC1Options.td @@ -70,6 +70,8 @@ def analysis_WarnIdempotentOps : Flag<"-analyzer-check-idempotent-operations">, HelpText<"Warn about idempotent operations">; def analysis_AnalyzerStats : Flag<"-analyzer-stats">, HelpText<"Emit warnings with analyzer statistics">; +def analysis_WarnBufferOverflows : Flag<"-analyzer-check-buffer-overflows">, + HelpText<"Warn about buffer overflows">; def analyzer_store : Separate<"-analyzer-store">, HelpText<"Source Code Analysis - Abstract Memory Store Models">; diff --git a/clang/include/clang/Frontend/AnalyzerOptions.h b/clang/include/clang/Frontend/AnalyzerOptions.h index 89de26e307b5..8915cc59e4b5 100644 --- a/clang/include/clang/Frontend/AnalyzerOptions.h +++ b/clang/include/clang/Frontend/AnalyzerOptions.h @@ -68,6 +68,7 @@ public: unsigned AnalyzerStats : 1; unsigned EagerlyAssume : 1; unsigned IdempotentOps : 1; + unsigned BufferOverflows : 1; unsigned PurgeDead : 1; unsigned TrimGraph : 1; unsigned VisualizeEGDot : 1; @@ -89,6 +90,8 @@ public: AnalyzeNestedBlocks = 0; AnalyzerStats = 0; EagerlyAssume = 0; + IdempotentOps = 0; + BufferOverflows = 0; PurgeDead = 1; TrimGraph = 0; VisualizeEGDot = 0; diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp index 7dbc8f23203e..ba9151019722 100644 --- a/clang/lib/Frontend/CompilerInvocation.cpp +++ b/clang/lib/Frontend/CompilerInvocation.cpp @@ -117,7 +117,9 @@ static void AnalyzerOptsToArgs(const AnalyzerOptions &Opts, if (Opts.EnableExperimentalInternalChecks) Res.push_back("-analyzer-experimental-internal-checks"); if (Opts.IdempotentOps) - Res.push_back("-analyzer-check-idempotent-operations"); + Res.push_back("-analyzer-check-idempotent-operations"); + if (Opts.BufferOverflows) + Res.push_back("-analyzer-check-buffer-overflows"); } static void CodeGenOptsToArgs(const CodeGenOptions &Opts, @@ -857,6 +859,7 @@ static void ParseAnalyzerArgs(AnalyzerOptions &Opts, ArgList &Args, Opts.MaxLoop = Args.getLastArgIntValue(OPT_analyzer_max_loop, 4, Diags); Opts.InlineCall = Args.hasArg(OPT_analyzer_inline_call); Opts.IdempotentOps = Args.hasArg(OPT_analysis_WarnIdempotentOps); + Opts.BufferOverflows = Args.hasArg(OPT_analysis_WarnBufferOverflows); } static void ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args, InputKind IK, diff --git a/clang/lib/GR/Checkers/AnalysisConsumer.cpp b/clang/lib/GR/Checkers/AnalysisConsumer.cpp index 1ebe53012eca..28a260c3e364 100644 --- a/clang/lib/GR/Checkers/AnalysisConsumer.cpp +++ b/clang/lib/GR/Checkers/AnalysisConsumer.cpp @@ -354,6 +354,9 @@ static void ActionExprEngine(AnalysisConsumer &C, AnalysisManager& mgr, if (C.Opts.IdempotentOps || C.Opts.EnableExperimentalChecks || C.Opts.EnableExperimentalInternalChecks) RegisterIdempotentOperationChecker(Eng); + + if (C.Opts.BufferOverflows) + RegisterArrayBoundCheckerV2(Eng); // Enable AnalyzerStatsChecker if it was given as an argument if (C.Opts.AnalyzerStats) diff --git a/clang/lib/GR/Checkers/ArrayBoundCheckerV2.cpp b/clang/lib/GR/Checkers/ArrayBoundCheckerV2.cpp new file mode 100644 index 000000000000..9a672087bc5f --- /dev/null +++ b/clang/lib/GR/Checkers/ArrayBoundCheckerV2.cpp @@ -0,0 +1,277 @@ +//== ArrayBoundCheckerV2.cpp ------------------------------------*- C++ -*--==// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +// +// This file defines ArrayBoundCheckerV2, which is a path-sensitive check +// which looks for an out-of-bound array element access. +// +//===----------------------------------------------------------------------===// + +#include "ExprEngineInternalChecks.h" +#include "clang/GR/BugReporter/BugType.h" +#include "clang/GR/PathSensitive/CheckerVisitor.h" +#include "clang/GR/PathSensitive/ExprEngine.h" +#include "clang/AST/CharUnits.h" + +using namespace clang; +using namespace GR; + +namespace { +class ArrayBoundCheckerV2 : + public CheckerVisitor { + BuiltinBug *BT; + + enum OOB_Kind { OOB_Precedes, OOB_Excedes }; + + void reportOOB(CheckerContext &C, const GRState *errorState, + OOB_Kind kind); + +public: + ArrayBoundCheckerV2() : BT(0) {} + static void *getTag() { static int x = 0; return &x; } + void visitLocation(CheckerContext &C, const Stmt *S, SVal l); +}; + +// FIXME: Eventually replace RegionRawOffset with this class. +class RegionRawOffsetV2 { +private: + const SubRegion *baseRegion; + SVal byteOffset; + + RegionRawOffsetV2() + : baseRegion(0), byteOffset(UnknownVal()) {} + +public: + RegionRawOffsetV2(const SubRegion* base, SVal offset) + : baseRegion(base), byteOffset(offset) {} + + NonLoc getByteOffset() const { return cast(byteOffset); } + const SubRegion *getRegion() const { return baseRegion; } + + static RegionRawOffsetV2 computeOffset(const GRState *state, + SValBuilder &svalBuilder, + SVal location); + + void dump() const; + void dumpToStream(llvm::raw_ostream& os) const; +}; +} + +void GR::RegisterArrayBoundCheckerV2(ExprEngine &Eng) { + Eng.registerCheck(new ArrayBoundCheckerV2()); +} + +void ArrayBoundCheckerV2::visitLocation(CheckerContext &checkerContext, + const Stmt *S, + SVal location) { + + // NOTE: Instead of using GRState::assumeInBound(), we are prototyping + // some new logic here that reasons directly about memory region extents. + // Once that logic is more mature, we can bring it back to assumeInBound() + // for all clients to use. + // + // The algorithm we are using here for bounds checking is to see if the + // memory access is within the extent of the base region. Since we + // have some flexibility in defining the base region, we can achieve + // various levels of conservatism in our buffer overflow checking. + const GRState *state = checkerContext.getState(); + const GRState *originalState = state; + + SValBuilder &svalBuilder = checkerContext.getSValBuilder(); + const RegionRawOffsetV2 &rawOffset = + RegionRawOffsetV2::computeOffset(state, svalBuilder, location); + + if (!rawOffset.getRegion()) + return; + + // CHECK LOWER BOUND: Is byteOffset < 0? If so, we are doing a load/store + // before the first valid offset in the memory region. + + SVal lowerBound + = svalBuilder.evalBinOpNN(state, BO_LT, rawOffset.getByteOffset(), + svalBuilder.makeZeroArrayIndex(), + svalBuilder.getConditionType()); + + NonLoc *lowerBoundToCheck = dyn_cast(&lowerBound); + if (!lowerBoundToCheck) + return; + + const GRState *state_precedesLowerBound, *state_withinLowerBound; + llvm::tie(state_precedesLowerBound, state_withinLowerBound) = + state->assume(*lowerBoundToCheck); + + // Are we constrained enough to definitely precede the lower bound? + if (state_precedesLowerBound && !state_withinLowerBound) { + reportOOB(checkerContext, state_precedesLowerBound, OOB_Precedes); + return; + } + + // Otherwise, assume the constraint of the lower bound. + assert(state_withinLowerBound); + state = state_withinLowerBound; + + do { + // CHECK UPPER BOUND: Is byteOffset >= extent(baseRegion)? If so, + // we are doing a load/store after the last valid offset. + DefinedOrUnknownSVal extentVal = + rawOffset.getRegion()->getExtent(svalBuilder); + if (!isa(extentVal)) + break; + + SVal upperbound + = svalBuilder.evalBinOpNN(state, BO_GE, rawOffset.getByteOffset(), + cast(extentVal), + svalBuilder.getConditionType()); + + NonLoc *upperboundToCheck = dyn_cast(&upperbound); + if (!upperboundToCheck) + break; + + const GRState *state_exceedsUpperBound, *state_withinUpperBound; + llvm::tie(state_exceedsUpperBound, state_withinUpperBound) = + state->assume(*upperboundToCheck); + + // Are we constrained enough to definitely exceed the upper bound? + if (state_exceedsUpperBound && !state_withinUpperBound) { + reportOOB(checkerContext, state_exceedsUpperBound, OOB_Excedes); + return; + } + + assert(state_withinUpperBound); + state = state_withinUpperBound; + } + while (false); + + if (state != originalState) + checkerContext.generateNode(state); +} + +void ArrayBoundCheckerV2::reportOOB(CheckerContext &checkerContext, + const GRState *errorState, + OOB_Kind kind) { + + ExplodedNode *errorNode = checkerContext.generateSink(errorState); + if (!errorNode) + return; + + if (!BT) + BT = new BuiltinBug("Out-of-bound access"); + + // FIXME: This diagnostics are preliminary. We should get far better + // diagnostics for explaining buffer overruns. + + llvm::SmallString<256> buf; + llvm::raw_svector_ostream os(buf); + os << "Out of bound memory access " + << (kind == OOB_Precedes ? "(accessed memory precedes memory block)" + : "(access exceeds upper limit of memory block)"); + + checkerContext.EmitReport(new RangedBugReport(*BT, os.str(), errorNode)); +} + +void RegionRawOffsetV2::dump() const { + dumpToStream(llvm::errs()); +} + +void RegionRawOffsetV2::dumpToStream(llvm::raw_ostream& os) const { + os << "raw_offset_v2{" << getRegion() << ',' << getByteOffset() << '}'; +} + +// FIXME: Merge with the implementation of the same method in Store.cpp +static bool IsCompleteType(ASTContext &Ctx, QualType Ty) { + if (const RecordType *RT = Ty->getAs()) { + const RecordDecl *D = RT->getDecl(); + if (!D->getDefinition()) + return false; + } + + return true; +} + + +// Lazily computes a value to be used by 'computeOffset'. If 'val' +// is unknown or undefined, we lazily substitute '0'. Otherwise, +// return 'val'. +static inline SVal getValue(SVal val, SValBuilder &svalBuilder) { + return isa(val) ? svalBuilder.makeArrayIndex(0) : val; +} + +// Scale a base value by a scaling factor, and return the scaled +// value as an SVal. Used by 'computeOffset'. +static inline SVal scaleValue(const GRState *state, + NonLoc baseVal, CharUnits scaling, + SValBuilder &sb) { + return sb.evalBinOpNN(state, BO_Mul, baseVal, + sb.makeArrayIndex(scaling.getQuantity()), + sb.getArrayIndexType()); +} + +// Add an SVal to another, treating unknown and undefined values as +// summing to UnknownVal. Used by 'computeOffset'. +static SVal addValue(const GRState *state, SVal x, SVal y, + SValBuilder &svalBuilder) { + // We treat UnknownVals and UndefinedVals the same here because we + // only care about computing offsets. + if (x.isUnknownOrUndef() || y.isUnknownOrUndef()) + return UnknownVal(); + + return svalBuilder.evalBinOpNN(state, BO_Add, + cast(x), cast(y), + svalBuilder.getArrayIndexType()); +} + +/// Compute a raw byte offset from a base region. Used for array bounds +/// checking. +RegionRawOffsetV2 RegionRawOffsetV2::computeOffset(const GRState *state, + SValBuilder &svalBuilder, + SVal location) +{ + const MemRegion *region = location.getAsRegion(); + SVal offset = UndefinedVal(); + + while (region) { + switch (region->getKind()) { + default: { + if (const SubRegion *subReg = dyn_cast(region)) + if (!offset.isUnknownOrUndef()) + return RegionRawOffsetV2(subReg, offset); + return RegionRawOffsetV2(); + } + case MemRegion::ElementRegionKind: { + const ElementRegion *elemReg = cast(region); + SVal index = elemReg->getIndex(); + if (!isa(index)) + return RegionRawOffsetV2(); + QualType elemType = elemReg->getElementType(); + // If the element is an incomplete type, go no further. + ASTContext &astContext = svalBuilder.getContext(); + if (!IsCompleteType(astContext, elemType)) + return RegionRawOffsetV2(); + + // Update the offset. + offset = addValue(state, + getValue(offset, svalBuilder), + scaleValue(state, + cast(index), + astContext.getTypeSizeInChars(elemType), + svalBuilder), + svalBuilder); + + if (offset.isUnknownOrUndef()) + return RegionRawOffsetV2(); + + region = elemReg->getSuperRegion(); + continue; + } + } + } + return RegionRawOffsetV2(); +} + + + diff --git a/clang/lib/GR/Checkers/CMakeLists.txt b/clang/lib/GR/Checkers/CMakeLists.txt index 9800b2ae05ba..2f2c7a7b5c11 100644 --- a/clang/lib/GR/Checkers/CMakeLists.txt +++ b/clang/lib/GR/Checkers/CMakeLists.txt @@ -2,9 +2,11 @@ add_clang_library(clangGRCheckers AdjustedReturnValueChecker.cpp AnalysisConsumer.cpp ArrayBoundChecker.cpp + ArrayBoundCheckerV2.cpp AttrNonNullChecker.cpp BasicObjCFoundationChecks.cpp BuiltinFunctionChecker.cpp + CStringChecker.cpp CallAndMessageChecker.cpp CastSizeChecker.cpp CastToStructChecker.cpp @@ -14,7 +16,6 @@ add_clang_library(clangGRCheckers CheckSecuritySyntaxOnly.cpp CheckSizeofPointer.cpp ChrootChecker.cpp - CStringChecker.cpp DereferenceChecker.cpp DivZeroChecker.cpp ExprEngine.cpp diff --git a/clang/lib/GR/Checkers/ExprEngineInternalChecks.h b/clang/lib/GR/Checkers/ExprEngineInternalChecks.h index aa2f99b1a114..bfb22f5c9efb 100644 --- a/clang/lib/GR/Checkers/ExprEngineInternalChecks.h +++ b/clang/lib/GR/Checkers/ExprEngineInternalChecks.h @@ -24,6 +24,7 @@ class ExprEngine; // Foundational checks that handle basic semantics. void RegisterAdjustedReturnValueChecker(ExprEngine &Eng); void RegisterArrayBoundChecker(ExprEngine &Eng); +void RegisterArrayBoundCheckerV2(ExprEngine &Eng); void RegisterAttrNonNullChecker(ExprEngine &Eng); void RegisterBuiltinFunctionChecker(ExprEngine &Eng); void RegisterCallAndMessageChecker(ExprEngine &Eng); diff --git a/clang/test/Analysis/out-of-bounds.c b/clang/test/Analysis/out-of-bounds.c new file mode 100644 index 000000000000..79a2ca06051c --- /dev/null +++ b/clang/test/Analysis/out-of-bounds.c @@ -0,0 +1,132 @@ +// RUN: %clang_cc1 -analyze -analyzer-check-objc-mem -analyzer-check-buffer-overflows -verify + +// Tests doing an out-of-bounds access after the end of an array using: +// - constant integer index +// - constant integer size for buffer +void test1(int x) { + int buf[100]; + buf[100] = 1; // expected-warning{{Out of bound memory access}} +} + +void test1_ok(int x) { + int buf[100]; + buf[99] = 1; // no-warning +} + +// Tests doing an out-of-bounds access after the end of an array using: +// - indirect pointer to buffer +// - constant integer index +// - constant integer size for buffer +void test1_ptr(int x) { + int buf[100]; + int *p = buf; + p[101] = 1; // expected-warning{{Out of bound memory access}} +} + +void test1_ptr_ok(int x) { + int buf[100]; + int *p = buf; + p[99] = 1; // expected-warning{{Out of bound memory access}} +} + +// Tests doing an out-of-bounds access before the start of an array using: +// - indirect pointer to buffer, manipulated using simple pointer arithmetic +// - constant integer index +// - constant integer size for buffer +void test1_ptr_arith(int x) { + int buf[100]; + int *p = buf; + p = p + 100; + p[0] = 1; // expected-warning{{Out of bound memory access}} +} + +void test1_ptr_arith_ok(int x) { + int buf[100]; + int *p = buf; + p = p + 99; + p[0] = 1; // no-warning +} + +void test1_ptr_arith_bad(int x) { + int buf[100]; + int *p = buf; + p = p + 99; + p[1] = 1; // expected-warning{{Out of bound memory access}} +} + +void test1_ptr_arith_ok2(int x) { + int buf[100]; + int *p = buf; + p = p + 100; + p[-1] = 1; // no-warning +} + +// Tests doing an out-of-bounds access before the start of an array using: +// - constant integer index +// - constant integer size for buffer +void test2(int x) { + int buf[100]; + buf[-1] = 1; // expected-warning{{Out of bound memory access}} +} + +// Tests doing an out-of-bounds access before the start of an array using: +// - indirect pointer to buffer +// - constant integer index +// - constant integer size for buffer +void test2_ptr(int x) { + int buf[100]; + int *p = buf; + p[-1] = 1; // expected-warning{{Out of bound memory access}} +} + +// Tests doing an out-of-bounds access before the start of an array using: +// - indirect pointer to buffer, manipulated using simple pointer arithmetic +// - constant integer index +// - constant integer size for buffer +void test2_ptr_arith(int x) { + int buf[100]; + int *p = buf; + --p; + p[0] = 1; // expected-warning{{Out of bound memory access}} +} + +// Tests doing an out-of-bounds access before the start of a multi-dimensional +// array using: +// - constant integer indices +// - constant integer sizes for the array +void test2_multi(int x) { + int buf[100][100]; + buf[0][-1] = 1; // expected-warning{{Out of bound memory access}} +} + +// Tests doing an out-of-bounds access before the start of a multi-dimensional +// array using: +// - constant integer indices +// - constant integer sizes for the array +void test2_multi_b(int x) { + int buf[100][100]; + buf[-1][0] = 1; // expected-warning{{Out of bound memory access}} +} + +void test2_multi_ok(int x) { + int buf[100][100]; + buf[0][0] = 1; // no-warning +} + +// *** FIXME *** +// We don't get a warning here yet because our symbolic constraint solving +// doesn't handle: (symbol * constant) < constant +void test3(int x) { + int buf[100]; + if (x < 0) + buf[x] = 1; +} + +// *** FIXME *** +// We don't get a warning here yet because our symbolic constraint solving +// doesn't handle: (symbol * constant) < constant +void test4(int x) { + int buf[100]; + if (x > 99) + buf[x] = 1; +}