From f0bc7d24882ab84f6398a1ea614dd0bf6e2a1085 Mon Sep 17 00:00:00 2001 From: Denys Petrov Date: Tue, 9 Nov 2021 14:20:23 +0200 Subject: [PATCH] [analyzer] Fix region cast between the same types with different qualifiers. Summary: Specifically, this fixes the case when we get an access to array element through the pointer to element. This covers several FIXME's. in https://reviews.llvm.org/D111654. Example: const int arr[4][2]; const int *ptr = arr[1]; // Fixes this. The issue is that `arr[1]` is `int*` (&Element{Element{glob_arr5,1 S64b,int[2]},0 S64b,int}), and `ptr` is `const int*`. We don't take qualifiers into account. Consequently, we doesn't match the types as the same ones. Differential Revision: https://reviews.llvm.org/D113480 --- clang/lib/StaticAnalyzer/Core/Store.cpp | 31 ++++++++++++++++--------------- clang/test/Analysis/initialization.cpp | 21 +++++++-------------- 2 files changed, 23 insertions(+), 29 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Core/Store.cpp b/clang/lib/StaticAnalyzer/Core/Store.cpp index 97b8b9d3..3cc0cd2 100644 --- a/clang/lib/StaticAnalyzer/Core/Store.cpp +++ b/clang/lib/StaticAnalyzer/Core/Store.cpp @@ -96,18 +96,24 @@ Optional StoreManager::castRegion(const MemRegion *R, // already be handled. QualType PointeeTy = CastToTy->getPointeeType(); QualType CanonPointeeTy = Ctx.getCanonicalType(PointeeTy); + CanonPointeeTy = CanonPointeeTy.getLocalUnqualifiedType(); // Handle casts to void*. We just pass the region through. - if (CanonPointeeTy.getLocalUnqualifiedType() == Ctx.VoidTy) + if (CanonPointeeTy == Ctx.VoidTy) return R; - // Handle casts from compatible types. - if (R->isBoundable()) + const auto IsSameRegionType = [&Ctx](const MemRegion *R, QualType OtherTy) { if (const auto *TR = dyn_cast(R)) { QualType ObjTy = Ctx.getCanonicalType(TR->getValueType()); - if (CanonPointeeTy == ObjTy) - return R; + if (OtherTy == ObjTy.getLocalUnqualifiedType()) + return true; } + return false; + }; + + // Handle casts from compatible types. + if (R->isBoundable() && IsSameRegionType(R, CanonPointeeTy)) + return R; // Process region cast according to the kind of the region being cast. switch (R->getKind()) { @@ -174,16 +180,11 @@ Optional StoreManager::castRegion(const MemRegion *R, CharUnits off = rawOff.getOffset(); if (off.isZero()) { - // Edge case: we are at 0 bytes off the beginning of baseR. We - // check to see if type we are casting to is the same as the base - // region. If so, just return the base region. - if (const auto *TR = dyn_cast(baseR)) { - QualType ObjTy = Ctx.getCanonicalType(TR->getValueType()); - QualType CanonPointeeTy = Ctx.getCanonicalType(PointeeTy); - if (CanonPointeeTy == ObjTy) - return baseR; - } - + // Edge case: we are at 0 bytes off the beginning of baseR. We check to + // see if the type we are casting to is the same as the type of the base + // region. If so, just return the base region. + if (IsSameRegionType(baseR, CanonPointeeTy)) + return baseR; // Otherwise, create a new ElementRegion at offset 0. return MakeElementRegion(cast(baseR), PointeeTy); } diff --git a/clang/test/Analysis/initialization.cpp b/clang/test/Analysis/initialization.cpp index 0883678..e5b94ea 100644 --- a/clang/test/Analysis/initialization.cpp +++ b/clang/test/Analysis/initialization.cpp @@ -68,8 +68,7 @@ void glob_invalid_index3() { void glob_invalid_index4() { const int *ptr = glob_arr4[1]; int idx = -42; - // FIXME: Should warn {{garbage or undefined}}. - auto x = ptr[idx]; // no-warning + auto x = ptr[idx]; // expected-warning{{garbage or undefined}} } int const glob_arr5[4][2] = {{1}, 3, 4, 5}; @@ -86,16 +85,11 @@ void glob_array_index3() { void glob_ptr_index2() { int const *ptr = glob_arr5[1]; - // FIXME: Should be TRUE. - clang_analyzer_eval(ptr[0] == 3); // expected-warning{{UNKNOWN}} - // FIXME: Should be TRUE. - clang_analyzer_eval(ptr[1] == 4); // expected-warning{{UNKNOWN}} - // FIXME: Should be UNDEFINED. - clang_analyzer_eval(ptr[2] == 5); // expected-warning{{UNKNOWN}} - // FIXME: Should be UNDEFINED. - clang_analyzer_eval(ptr[3] == 0); // expected-warning{{UNKNOWN}} - // FIXME: Should be UNDEFINED. - clang_analyzer_eval(ptr[4] == 0); // expected-warning{{UNKNOWN}} + clang_analyzer_eval(ptr[0] == 3); // expected-warning{{TRUE}} + clang_analyzer_eval(ptr[1] == 4); // expected-warning{{TRUE}} + clang_analyzer_eval(ptr[2] == 5); // expected-warning{{UNDEFINED}} + clang_analyzer_eval(ptr[3] == 0); // expected-warning{{UNDEFINED}} + clang_analyzer_eval(ptr[4] == 0); // expected-warning{{UNDEFINED}} } void glob_invalid_index5() { @@ -106,8 +100,7 @@ void glob_invalid_index5() { void glob_invalid_index6() { int const *ptr = &glob_arr5[1][0]; int idx = 42; - // FIXME: Should warn {{garbage or undefined}}. - auto x = ptr[idx]; // no-warning + auto x = ptr[idx]; // expected-warning{{garbage or undefined}} } extern const int glob_arr_no_init[10]; -- 2.7.4