steakhal created this revision.
steakhal added reviewers: NoQ, vsavchenko, baloghadamsoftware, xazax.hun.
Herald added subscribers: cfe-commits, martong, Charusso, dkrupp, donat.nagy,
Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, whisperity.
Herald added a reviewer: Szelethus.
Herald added a project: clang.
steakhal requested review of this revision.
When the analyzer loads a value through a given type which was previously
stored with a different type, it will implicitly cast the associated value to
the type in which we are trying to access it.
However, sometimes this //implicit// cast done by the `CastRetrievedVal` in
`RegionStoreManager::getBinding` casted the value to a wrong type.
In this example, it cast to the `unsigned char` (which is the type of the
stored value of `**b`, stored at `#1`) instead of the static type of the access
(the type of `**b` is `char`at `#2`).
If the analyzer wouldn't overwrite the already given non-null type, it would
cast it to the correct one instead.
This patch simply modifies the code to overwrite the cast type to the
underlying type of the pointee's region only if the type were `null`.
This also resolves a FIXME in the test suite.
---
This test case crashes the analyzer:
// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s
// expected-no-diagnostics
void test(int *a, char ***b, float *c) {
*(unsigned char **)b = (unsigned char *)a; // #1
if (**b == 0) // no-crash, #2
;
*(unsigned char **)b = (unsigned char *)c;
if (**b == 0) // no-crash
;
}
Thank you @ASDenysPetrov for rising this issue at D77062
<https://reviews.llvm.org/D77062>.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D88477
Files:
clang/lib/StaticAnalyzer/Core/RegionStore.cpp
clang/test/Analysis/nonloc-as-loc-crash.c
clang/test/Analysis/svalbuilder-float-cast.c
Index: clang/test/Analysis/svalbuilder-float-cast.c
===================================================================
--- clang/test/Analysis/svalbuilder-float-cast.c
+++ clang/test/Analysis/svalbuilder-float-cast.c
@@ -4,13 +4,9 @@
void SymbolCast_of_float_type_aux(int *p) {
*p += 0;
- // FIXME: Ideally, all unknown values should be symbolicated.
- clang_analyzer_denote(*p, "$x"); // expected-warning{{Not a symbol}}
-
+ clang_analyzer_denote(*p, "$x");
*p += 1;
- // This should NOT be (float)$x + 1. Symbol $x was never casted to float.
- // FIXME: Ideally, this should be $x + 1.
- clang_analyzer_express(*p); // expected-warning{{Not a symbol}}
+ clang_analyzer_express(*p); // expected-warning{{$x + 1}}
}
void SymbolCast_of_float_type() {
Index: clang/test/Analysis/nonloc-as-loc-crash.c
===================================================================
--- /dev/null
+++ clang/test/Analysis/nonloc-as-loc-crash.c
@@ -0,0 +1,12 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s
+// expected-no-diagnostics
+
+void test(int *a, char ***b, float *c) {
+ *(unsigned char **)b = (unsigned char *)a;
+ if (**b == 0) // no-crash
+ ;
+
+ *(unsigned char **)b = (unsigned char *)c;
+ if (**b == 0) // no-crash
+ ;
+}
Index: clang/lib/StaticAnalyzer/Core/RegionStore.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ clang/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -1439,7 +1439,8 @@
assert(!T->isVoidType() && "Attempting to dereference a void pointer!");
MR = GetElementZeroRegion(cast<SubRegion>(MR), T);
} else {
- T = cast<TypedValueRegion>(MR)->getValueType();
+ if (T.isNull())
+ T = cast<TypedValueRegion>(MR)->getValueType();
}
// FIXME: Perhaps this method should just take a 'const MemRegion*' argument
Index: clang/test/Analysis/svalbuilder-float-cast.c
===================================================================
--- clang/test/Analysis/svalbuilder-float-cast.c
+++ clang/test/Analysis/svalbuilder-float-cast.c
@@ -4,13 +4,9 @@
void SymbolCast_of_float_type_aux(int *p) {
*p += 0;
- // FIXME: Ideally, all unknown values should be symbolicated.
- clang_analyzer_denote(*p, "$x"); // expected-warning{{Not a symbol}}
-
+ clang_analyzer_denote(*p, "$x");
*p += 1;
- // This should NOT be (float)$x + 1. Symbol $x was never casted to float.
- // FIXME: Ideally, this should be $x + 1.
- clang_analyzer_express(*p); // expected-warning{{Not a symbol}}
+ clang_analyzer_express(*p); // expected-warning{{$x + 1}}
}
void SymbolCast_of_float_type() {
Index: clang/test/Analysis/nonloc-as-loc-crash.c
===================================================================
--- /dev/null
+++ clang/test/Analysis/nonloc-as-loc-crash.c
@@ -0,0 +1,12 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s
+// expected-no-diagnostics
+
+void test(int *a, char ***b, float *c) {
+ *(unsigned char **)b = (unsigned char *)a;
+ if (**b == 0) // no-crash
+ ;
+
+ *(unsigned char **)b = (unsigned char *)c;
+ if (**b == 0) // no-crash
+ ;
+}
Index: clang/lib/StaticAnalyzer/Core/RegionStore.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ clang/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -1439,7 +1439,8 @@
assert(!T->isVoidType() && "Attempting to dereference a void pointer!");
MR = GetElementZeroRegion(cast<SubRegion>(MR), T);
} else {
- T = cast<TypedValueRegion>(MR)->getValueType();
+ if (T.isNull())
+ T = cast<TypedValueRegion>(MR)->getValueType();
}
// FIXME: Perhaps this method should just take a 'const MemRegion*' argument
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
