balazske created this revision.
Herald added subscribers: steakhal, ASDenysPetrov, martong, Charusso,
gamesh411, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet,
baloghadamsoftware, xazax.hun, whisperity.
Herald added a reviewer: Szelethus.
balazske requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
This is an experimental fix for ArrayBoundV2 checker to handle "flexible arrays"
(incomplete array) better. If incomplete array is found to be indexed, the
used size (for out-of-bounds check) will be the dynamic size of base region,
with offset (of the incomplete array) taken into account.
In this was indexing of incomplete arrays is not reported as error,
if the array was allocated using a fixed size.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D99714
Files:
clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
clang/test/Analysis/array-bound-v2.c
Index: clang/test/Analysis/array-bound-v2.c
===================================================================
--- /dev/null
+++ clang/test/Analysis/array-bound-v2.c
@@ -0,0 +1,48 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc,alpha.security.ArrayBoundV2 %s
+
+//#include <stdlib.h>
+//#include "Inputs/system-header-simulator-cxx.h"
+
+typedef __typeof(sizeof(int)) size_t;
+void *malloc(size_t);
+
+struct FlexArr {
+ int n;
+ int s0[2];
+ int s[];
+};
+
+struct Outer {
+ int n;
+ struct FlexArr FA;
+};
+
+void test_flex() {
+ struct FlexArr *p = (struct FlexArr *)malloc(sizeof(struct FlexArr) + sizeof(int) * 10);
+ p->n = 10;
+ p->s[0] = 0;
+ p->s[9] = 9;
+ p->s[10] = 10; // expected-waring{{Out of bound memory access}}
+}
+
+void test_simple() {
+ struct FlexArr *p = (struct FlexArr *)malloc(sizeof(struct FlexArr) + sizeof(int) * 10);
+ p->n = 10;
+ p->s0[0] = 0;
+ p->s0[1] = 1;
+ p->s0[2] = 2; // expected-waring{{Out of bound memory access}}
+}
+
+void test_outer_flex() {
+ struct Outer *p = (struct Outer *)malloc(sizeof(struct Outer) + sizeof(int) * 10);
+ p->FA.s[0] = 0;
+ p->FA.s[9] = 9;
+ p->FA.s[10] = 10; // expected-waring{{Out of bound memory access}}
+}
+
+void test_outer_simple() {
+ struct Outer *p = (struct Outer *)malloc(sizeof(struct Outer) + sizeof(int) * 10);
+ p->FA.s0[0] = 0;
+ p->FA.s0[1] = 1;
+ p->FA.s0[2] = 2; // expected-waring{{Out of bound memory access}}
+}
Index: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
+++ clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
@@ -113,6 +113,30 @@
return std::pair<NonLoc, nonloc::ConcreteInt>(offset, extent);
}
+namespace {
+SVal getDynamicSizeWithOffset(ProgramStateRef State, const MemRegion *MRegion) {
+ SValBuilder &SvalBuilder = State->getStateManager().getSValBuilder();
+ 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
+
void ArrayBoundCheckerV2::checkLocation(SVal location, bool isLoad,
const Stmt* LoadS,
CheckerContext &checkerContext) const {
@@ -137,6 +161,11 @@
NonLoc rawOffsetVal = rawOffset.getByteOffset();
+ bool IsIncompleteArray = false;
+ if (const auto *ASExpr = dyn_cast_or_null<ArraySubscriptExpr>(LoadS))
+ IsIncompleteArray = isa<IncompleteArrayType>(
+ ASExpr->getBase()->IgnoreParenImpCasts()->getType().getCanonicalType());
+
// CHECK LOWER BOUND: Is byteOffset < extent begin?
// If so, we are doing a load/store
// before the first valid offset in the memory region.
@@ -180,6 +209,13 @@
// we are doing a load/store after the last valid offset.
const MemRegion *MR = rawOffset.getRegion();
DefinedOrUnknownSVal Size = getDynamicSize(state, MR, svalBuilder);
+
+ if (IsIncompleteArray) {
+ SVal DSWithOffset = getDynamicSizeWithOffset(state, MR);
+ if (auto Val = DSWithOffset.getAs<DefinedOrUnknownSVal>())
+ Size = *Val;
+ }
+
if (!Size.getAs<NonLoc>())
break;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
