llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-static-analyzer-1 @llvm/pr-subscribers-clang Author: Endre Fülöp (gamesh411) <details> <summary>Changes</summary> …n RegionStore Extend getBindingForField() to handle the FieldRegion(ElementRegion(VarRegion)) hierarchy when accessing a field of a struct inside an array. Previously, only FieldRegion(VarRegion) was handled (i.e. fields of a single struct variable), so the initializer of struct array elements was never considered. The same trust conditions apply: const-qualified types are always trusted, and non-const globals are trusted when analyzing main(). For C++ structs with in-class (default member) initializers, we conservatively fall through to the symbolic path to avoid incorrectly returning zero. Actually resolving default member initializer values is left to a separate change. This resolves false positives from security.ArrayBound on sentinel-terminated struct arrays. --- Full diff: https://github.com/llvm/llvm-project/pull/189361.diff 3 Files Affected: - (modified) clang/lib/StaticAnalyzer/Core/RegionStore.cpp (+40) - (added) clang/test/Analysis/array-of-structs-initializer-not-visible-in-main.c (+47) - (added) clang/test/Analysis/array-of-structs-initializer-not-visible-in-main.cpp (+36) ``````````diff diff --git a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp index 6ec66298e8c45..eac87355f8401 100644 --- a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp +++ b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp @@ -2145,6 +2145,46 @@ SVal RegionStoreManager::getBindingForField(RegionBindingsConstRef B, } } + // In case of array of structs, the super-region is not directly a VarRegion, + // instead there is another layer of ElementRegion in between them, i.e.: + // FieldRegion(ElementRegion(VarRegion)). + if (const auto *ER = dyn_cast<ElementRegion>(superR)) { + if (const auto *VR = dyn_cast<VarRegion>(ER->getSuperRegion())) { + const VarDecl *VD = VR->getDecl(); + QualType ArrayTy = VD->getType(); + unsigned FieldIdx = FD->getFieldIndex(); + + if (ArrayTy.isConstQualified() || Ty.isConstQualified() || + (B.isMainAnalysis() && VD->hasGlobalStorage())) { + if (const Expr *Init = VD->getAnyInitializer()) + if (const auto *InitList = dyn_cast<InitListExpr>(Init)) + if (const auto CI = ER->getIndex().getAs<nonloc::ConcreteInt>()) { + uint64_t ElemIdx = CI->getValue()->getZExtValue(); + + const InitListExpr *StructILE = nullptr; + if (ElemIdx < InitList->getNumInits()) { + StructILE = dyn_cast<InitListExpr>(InitList->getInit(ElemIdx)); + } else if (!FD->hasInClassInitializer()) { + // This is zero initialization, because there is no explicit + // initializer for this index and there is no default member + // initializer in-class either. + return svalBuilder.makeZeroVal(Ty); + } + if (StructILE) { + if (FieldIdx < StructILE->getNumInits()) { + if (const Expr *FieldInit = StructILE->getInit(FieldIdx)) + if (std::optional<SVal> V = + svalBuilder.getConstantVal(FieldInit)) + return *V; + } else if (!FD->hasInClassInitializer()) { + return svalBuilder.makeZeroVal(Ty); + } + } + } + } + } + } + // Handle the case where we are accessing into a larger scalar object. // For example, this handles: // struct header { diff --git a/clang/test/Analysis/array-of-structs-initializer-not-visible-in-main.c b/clang/test/Analysis/array-of-structs-initializer-not-visible-in-main.c new file mode 100644 index 0000000000000..652a44d2815e9 --- /dev/null +++ b/clang/test/Analysis/array-of-structs-initializer-not-visible-in-main.c @@ -0,0 +1,47 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify %s + +// This test verifies that the analyzer can 'see' the initializer of an +// array of structs, covering const, non-const, and main() vs non-main cases. + +void clang_analyzer_warnIfReached(void); + +struct S { + int a; +}; + +// Non-const struct array: initializer should be visible in main(). +struct S struct_array[1] = { + {11}, +}; + +int main(int argc, char **argv) { + if (struct_array->a == 11) { + clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}} + } else { + clang_analyzer_warnIfReached(); // unreachable + } +} + +// Const struct array: initializer should be visible in any function. +const struct S struct_array_const[1] = { {44} }; + +void use_struct_array_const(void) { + if (struct_array_const->a == 44) { + clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}} + } else { + clang_analyzer_warnIfReached(); // unreachable + } +} + +// Non-const struct array in non-main: initializer must NOT be trusted. +struct S struct_array_nonconst[1] = { {55} }; + +void use_struct_array_nonconst(void) { + if (struct_array_nonconst->a == 55) { + clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}} + } else { + // This is intentionally reachable, because this is a non-const array which + // may have been changed before the call to this function. + clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}} + } +} diff --git a/clang/test/Analysis/array-of-structs-initializer-not-visible-in-main.cpp b/clang/test/Analysis/array-of-structs-initializer-not-visible-in-main.cpp new file mode 100644 index 0000000000000..6053a24a63f48 --- /dev/null +++ b/clang/test/Analysis/array-of-structs-initializer-not-visible-in-main.cpp @@ -0,0 +1,36 @@ +// RUN: %clang_analyze_cc1 -std=c++14 -analyzer-checker=core,debug.ExprInspection -verify %s + +// This test verifies that the analyzer does not incorrectly assume zero +// for fields with in-class (default member) initializers when accessing +// elements of a struct array. + +void clang_analyzer_warnIfReached(void); + +struct S { + int a = 3; +}; + +// Non-const array in main: the analyzer must not assume zero for 'a', +// because it has a default member initializer. +S sarr_nonconst[2] = {}; + +int main(int argc, char **argv) { + // FIXME: Should recognize that it is 3 (from the default member initializer). + if (sarr_nonconst[0].a == 3) { + clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}} + } else { + clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}} + } +} + +// Const array in non-main: the analyzer resolves the default member +// initializer correctly through the lazy binding path. +const S sarr_const[2] = {}; + +void use_const(void) { + if (sarr_const[0].a == 3) { + clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}} + } else { + clang_analyzer_warnIfReached(); // unreachable + } +} `````````` </details> https://github.com/llvm/llvm-project/pull/189361 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
