diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp index eacacc4e1d611..ae878ecbcc34a 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp @@ -606,6 +606,27 @@ void ExprEngine::handleConstructor(const Expr *E, unsigned Idx = 0; if (CE->getType()->isArrayType() || AILE) { + + auto isZeroSizeArray = [&] { + uint64_t Size = 1; + + if (const auto *CAT = dyn_cast(CE->getType())) + Size = getContext().getConstantArrayElementCount(CAT); + else if (AILE) + Size = getContext().getArrayInitLoopExprElementCount(AILE); + + return Size == 0; + }; + + // No element construction will happen in a 0 size array. + if (isZeroSizeArray()) { + StmtNodeBuilder Bldr(Pred, destNodes, *currBldrCtx); + static SimpleProgramPointTag T{"ExprEngine", + "Skipping 0 size array construction"}; + Bldr.generateNode(CE, Pred, State, &T); + return; + } + Idx = getIndexOfElementToConstruct(State, CE, LCtx).value_or(0u); State = setIndexOfElementToConstruct(State, CE, LCtx, Idx + 1); } @@ -1165,6 +1186,16 @@ void ExprEngine::VisitLambdaExpr(const LambdaExpr *LE, ExplodedNode *Pred, assert(InitExpr && "Capture missing initialization expression"); + // Capturing a 0 length array is a no-op, so we ignore it to get a more + // accurate analysis. If it's not ignored, it would set the default + // binding of the lambda to 'Unknown', which can lead to falsely detecting + // 'Uninitialized' values as 'Unknown' and not reporting a warning. + const auto FTy = FieldForCapture->getType(); + if (FTy->isConstantArrayType() && + getContext().getConstantArrayElementCount( + getContext().getAsConstantArrayType(FTy)) == 0) + continue; + // With C++17 copy elision the InitExpr can be anything, so instead of // pattern matching all cases, we simple check if the current field is // under construction or not, regardless what it's InitExpr is. diff --git a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp index eca012b6015fa..4670ae0fb248e 100644 --- a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp +++ b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp @@ -2594,6 +2594,12 @@ RegionStoreManager::tryBindSmallStruct(RegionBindingsConstRef B, return None; QualType Ty = FD->getType(); + + // Zero length arrays are basically no-ops, so we also ignore them here. + if (Ty->isConstantArrayType() && + Ctx.getConstantArrayElementCount(Ctx.getAsConstantArrayType(Ty)) == 0) + continue; + if (!(Ty->isScalarType() || Ty->isReferenceType())) return None; diff --git a/clang/test/Analysis/array-init-loop.cpp b/clang/test/Analysis/array-init-loop.cpp index ca7c832e8ab13..4ab4489fc882f 100644 --- a/clang/test/Analysis/array-init-loop.cpp +++ b/clang/test/Analysis/array-init-loop.cpp @@ -306,3 +306,27 @@ void structured_binding_multi() { clang_analyzer_eval(b[0].i == 3); // expected-warning{{TRUE}} clang_analyzer_eval(b[1].i == 4); // expected-warning{{TRUE}} } + +// This snippet used to crash +namespace crash { + +struct S +{ + int x; + S() { x = 1; } +}; + +void no_crash() { + S arr[0]; + int n = 1; + + auto l = [arr, n] { return n; }; + + int x = l(); + clang_analyzer_eval(x == 1); // expected-warning{{TRUE}} + + // FIXME: This should be 'Undefined'. + clang_analyzer_eval(arr[0].x); // expected-warning{{UNKNOWN}} +} + +} // namespace crash diff --git a/clang/test/Analysis/dtor-array.cpp b/clang/test/Analysis/dtor-array.cpp index da1c80e261948..ab0a939232173 100644 --- a/clang/test/Analysis/dtor-array.cpp +++ b/clang/test/Analysis/dtor-array.cpp @@ -258,8 +258,7 @@ void zeroLength(){ auto *arr4 = new InlineDtor[2][2][0]; delete[] arr4; - // FIXME: Should be TRUE once the constructors are handled properly. - clang_analyzer_eval(InlineDtor::dtorCalled == 0); // expected-warning {{TRUE}} expected-warning {{FALSE}} + clang_analyzer_eval(InlineDtor::dtorCalled == 0); // expected-warning {{TRUE}} } diff --git a/clang/test/Analysis/flexible-array-member.cpp b/clang/test/Analysis/flexible-array-member.cpp new file mode 100644 index 0000000000000..10ca64bc43805 --- /dev/null +++ b/clang/test/Analysis/flexible-array-member.cpp @@ -0,0 +1,58 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -std=c++11 -verify %s +// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -std=c++17 -verify %s + +#include "Inputs/system-header-simulator-cxx.h" + +void clang_analyzer_eval(bool); + +struct S +{ + static int c; + static int d; + int x; + S() { x = c++; } + ~S() { d++; } +}; + +int S::c = 0; +int S::d = 0; + +struct Flex +{ + int length; + S contents[0]; +}; + +void flexibleArrayMember() +{ + S::c = 0; + S::d = 0; + + const int size = 4; + + Flex *arr = + (Flex *)::operator new(__builtin_offsetof(Flex, contents) + sizeof(S) * size); + + clang_analyzer_eval(S::c == 0); // expected-warning{{TRUE}} + + new (&arr->contents[0]) S; + new (&arr->contents[1]) S; + new (&arr->contents[2]) S; + new (&arr->contents[3]) S; + + clang_analyzer_eval(S::c == size); // expected-warning{{TRUE}} + + clang_analyzer_eval(arr->contents[0].x == 0); // expected-warning{{TRUE}} + clang_analyzer_eval(arr->contents[1].x == 1); // expected-warning{{TRUE}} + clang_analyzer_eval(arr->contents[2].x == 2); // expected-warning{{TRUE}} + clang_analyzer_eval(arr->contents[3].x == 3); // expected-warning{{TRUE}} + + arr->contents[0].~S(); + arr->contents[1].~S(); + arr->contents[2].~S(); + arr->contents[3].~S(); + + ::operator delete(arr); + + clang_analyzer_eval(S::d == size); // expected-warning{{TRUE}} +} diff --git a/clang/test/Analysis/zero-size-non-pod-array.cpp b/clang/test/Analysis/zero-size-non-pod-array.cpp new file mode 100644 index 0000000000000..a2117bc80eac8 --- /dev/null +++ b/clang/test/Analysis/zero-size-non-pod-array.cpp @@ -0,0 +1,189 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -std=c++11 -verify %s +// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -std=c++17 -verify %s + +void clang_analyzer_eval(bool); + +struct S{ + static int CtorInvocationCount; + static int DtorInvocationCount; + + S(){CtorInvocationCount++;} + ~S(){DtorInvocationCount++;} +}; + +int S::CtorInvocationCount = 0; +int S::DtorInvocationCount = 0; + +void zeroSizeArrayStack() { + S::CtorInvocationCount = 0; + + S arr[0]; + + clang_analyzer_eval(S::CtorInvocationCount == 0); //expected-warning{{TRUE}} +} + +void zeroSizeMultidimensionalArrayStack() { + S::CtorInvocationCount = 0; + S::DtorInvocationCount = 0; + + { + S arr[2][0]; + S arr2[0][2]; + + S arr3[0][2][2]; + S arr4[2][2][0]; + S arr5[2][0][2]; + } + + clang_analyzer_eval(S::CtorInvocationCount == 0); //expected-warning{{TRUE}} + clang_analyzer_eval(S::DtorInvocationCount == 0); //expected-warning{{TRUE}} +} + +void zeroSizeArrayStackInLambda() { + S::CtorInvocationCount = 0; + S::DtorInvocationCount = 0; + + []{ + S arr[0]; + }(); + + clang_analyzer_eval(S::CtorInvocationCount == 0); //expected-warning{{TRUE}} + clang_analyzer_eval(S::DtorInvocationCount == 0); //expected-warning{{TRUE}} +} + +void zeroSizeArrayHeap() { + S::CtorInvocationCount = 0; + S::DtorInvocationCount = 0; + + auto *arr = new S[0]; + delete[] arr; + + clang_analyzer_eval(S::CtorInvocationCount == 0); //expected-warning{{TRUE}} + clang_analyzer_eval(S::DtorInvocationCount == 0); //expected-warning{{TRUE}} +} + +void zeroSizeMultidimensionalArrayHeap() { + S::CtorInvocationCount = 0; + S::DtorInvocationCount = 0; + + auto *arr = new S[2][0]; + delete[] arr; + + auto *arr2 = new S[0][2]; + delete[] arr2; + + auto *arr3 = new S[0][2][2]; + delete[] arr3; + + auto *arr4 = new S[2][2][0]; + delete[] arr4; + + auto *arr5 = new S[2][0][2]; + delete[] arr5; + + clang_analyzer_eval(S::CtorInvocationCount == 0); //expected-warning{{TRUE}} + clang_analyzer_eval(S::DtorInvocationCount == 0); //expected-warning{{TRUE}} +} + +#if __cplusplus >= 201703L + +void zeroSizeArrayBinding() { + S::CtorInvocationCount = 0; + + S arr[0]; + + // Note: This is an error in gcc but a warning in clang. + // In MSVC the declaration of 'S arr[0]' is already an error + // and it doesn't recognize this syntax as a structured binding. + auto [] = arr; //expected-warning{{ISO C++17 does not allow a decomposition group to be empty}} + + clang_analyzer_eval(S::CtorInvocationCount == 0); //expected-warning{{TRUE}} +} + +#endif + +void zeroSizeArrayLambdaCapture() { + S::CtorInvocationCount = 0; + S::DtorInvocationCount = 0; + + S arr[0]; + + auto l = [arr]{}; + [arr]{}(); + + //FIXME: These should be TRUE. We should avoid calling the destructor + // of the temporary that is materialized as the lambda. + clang_analyzer_eval(S::CtorInvocationCount == 0); //expected-warning{{TRUE}} expected-warning{{FALSE}} + clang_analyzer_eval(S::DtorInvocationCount == 0); //expected-warning{{TRUE}} expected-warning{{FALSE}} +} + +// FIXME: Report a warning if the standard is at least C++17. +#if __cplusplus < 201703L +void zeroSizeArrayLambdaCaptureUndefined1() { + S arr[0]; + int n; + + auto l = [arr, n]{ + int x = n; //expected-warning{{Assigned value is garbage or undefined}} + (void) x; + }; + + l(); +} +#endif + +void zeroSizeArrayLambdaCaptureUndefined2() { + S arr[0]; + int n; + + [arr, n]{ + int x = n; //expected-warning{{Assigned value is garbage or undefined}} + (void) x; + }(); +} + +struct Wrapper{ + S arr[0]; +}; + +void zeroSizeArrayMember() { + S::CtorInvocationCount = 0; + S::DtorInvocationCount = 0; + + { + Wrapper W; + } + + clang_analyzer_eval(S::CtorInvocationCount == 0); //expected-warning{{TRUE}} + clang_analyzer_eval(S::DtorInvocationCount == 0); //expected-warning{{TRUE}} +} + +void zeroSizeArrayMemberCopyMove() { + S::CtorInvocationCount = 0; + S::DtorInvocationCount = 0; + + { + Wrapper W; + Wrapper W2 = W; + Wrapper W3 = (Wrapper&&) W2; + } + + clang_analyzer_eval(S::CtorInvocationCount == 0); //expected-warning{{TRUE}} + clang_analyzer_eval(S::DtorInvocationCount == 0); //expected-warning{{TRUE}} +} + +struct MultiWrapper{ + S arr[2][0]; +}; + +void zeroSizeMultidimensionalArrayMember() { + S::CtorInvocationCount = 0; + S::DtorInvocationCount = 0; + + { + MultiWrapper MW; + } + + clang_analyzer_eval(S::CtorInvocationCount == 0); //expected-warning{{TRUE}} + clang_analyzer_eval(S::DtorInvocationCount == 0); //expected-warning{{TRUE}} +}