[clang] 93b98eb - [analyzer] getBinding should auto-detect type only if it was not given
Balazs Benics via cfe-commits
cfe-commits at lists.llvm.org
Wed Nov 23 06:52:44 PST 2022
Author: Balazs Benics
Date: 2022-11-23T15:52:11+01:00
New Revision: 93b98eb399a14f2c626ed82eaaa70997f8323406
URL: https://github.com/llvm/llvm-project/commit/93b98eb399a14f2c626ed82eaaa70997f8323406
DIFF: https://github.com/llvm/llvm-project/commit/93b98eb399a14f2c626ed82eaaa70997f8323406.diff
LOG: [analyzer] getBinding should auto-detect type only if it was not given
Casting a pointer to a suitably large integral type by reinterpret-cast
should result in the same value as by using the `__builtin_bit_cast()`.
The compiler exploits this: https://godbolt.org/z/zMP3sG683
However, the analyzer does not bind the same symbolic value to these
expressions, resulting in weird situations, such as failing equality
checks and even results in crashes: https://godbolt.org/z/oeMP7cj8q
Previously, in the `RegionStoreManager::getBinding()` even if `T` was
non-null, we replaced it with `TVR->getValueType()` in case the `MR` was
`TypedValueRegion`.
It doesn't make much sense to auto-detect the type if the type is
already given. By not doing the auto-detection, we would just do the
right thing and perform the load by that type.
This means that we will cast the value to that type.
So, in this patch, I'm proposing to do auto-detection only if the type
was null.
Here is a snippet of code, annotated by the previous and new dump values.
`LocAsInteger` should wrap the `SymRegion`, since we want to load the
address as if it was an integer.
In none of the following cases should type auto-detection be triggered,
hence we should eventually reach an `evalCast()` to lazily cast the loaded
value into that type.
```lang=C++
void LValueToRValueBitCast_dumps(void *p, char (*array)[8]) {
clang_analyzer_dump(p); // remained: &SymRegion{reg_$0<void * p>}
clang_analyzer_dump(array); // remained: {{&SymRegion{reg_$1<char (*)[8] array>}
clang_analyzer_dump((unsigned long)p);
// remained: {{&SymRegion{reg_$0<void * p>} [as 64 bit integer]}}
clang_analyzer_dump(__builtin_bit_cast(unsigned long, p)); <--------- change #1
// previously: {{&SymRegion{reg_$0<void * p>}}}
// now: {{&SymRegion{reg_$0<void * p>} [as 64 bit integer]}}
clang_analyzer_dump((unsigned long)array); // remained: {{&SymRegion{reg_$1<char (*)[8] array>} [as 64 bit integer]}}
clang_analyzer_dump(__builtin_bit_cast(unsigned long, array)); <--------- change #2
// previously: {{&SymRegion{reg_$1<char (*)[8] array>}}}
// now: {{&SymRegion{reg_$1<char (*)[8] array>} [as 64 bit integer]}}
}
```
Reviewed By: xazax.hun
Differential Revision: https://reviews.llvm.org/D136603
Added:
Modified:
clang/lib/StaticAnalyzer/Core/RegionStore.cpp
clang/test/Analysis/ptr-arith.cpp
clang/test/Analysis/svalbuilder-float-cast.c
Removed:
################################################################################
diff --git a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
index 8af351f642469..3e638be787b51 100644
--- a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -1414,19 +1414,20 @@ SVal RegionStoreManager::getBinding(RegionBindingsConstRef B, Loc L, QualType T)
return UnknownVal();
}
- if (!isa<TypedValueRegion>(MR)) {
- if (T.isNull()) {
- if (const TypedRegion *TR = dyn_cast<TypedRegion>(MR))
- T = TR->getLocationType()->getPointeeType();
- else if (const SymbolicRegion *SR = dyn_cast<SymbolicRegion>(MR))
- T = SR->getPointeeStaticType();
- }
- assert(!T.isNull() && "Unable to auto-detect binding type!");
- assert(!T->isVoidType() && "Attempting to dereference a void pointer!");
+ // Auto-detect the binding type.
+ if (T.isNull()) {
+ if (const auto *TVR = dyn_cast<TypedValueRegion>(MR))
+ T = TVR->getValueType();
+ else if (const auto *TR = dyn_cast<TypedRegion>(MR))
+ T = TR->getLocationType()->getPointeeType();
+ else if (const auto *SR = dyn_cast<SymbolicRegion>(MR))
+ T = SR->getPointeeStaticType();
+ }
+ assert(!T.isNull() && "Unable to auto-detect binding type!");
+ assert(!T->isVoidType() && "Attempting to dereference a void pointer!");
+
+ if (!isa<TypedValueRegion>(MR))
MR = GetElementZeroRegion(cast<SubRegion>(MR), T);
- } else {
- T = cast<TypedValueRegion>(MR)->getValueType();
- }
// FIXME: Perhaps this method should just take a 'const MemRegion*' argument
// instead of 'Loc', and have the other Loc cases handled at a higher level.
diff --git a/clang/test/Analysis/ptr-arith.cpp b/clang/test/Analysis/ptr-arith.cpp
index e44939768960d..a1264a1f04839 100644
--- a/clang/test/Analysis/ptr-arith.cpp
+++ b/clang/test/Analysis/ptr-arith.cpp
@@ -1,4 +1,9 @@
-// RUN: %clang_analyze_cc1 -Wno-unused-value -std=c++14 -analyzer-checker=core,debug.ExprInspection,alpha.core.PointerArithm -verify %s
+// RUN: %clang_analyze_cc1 -Wno-unused-value -std=c++14 -verify %s -triple x86_64-pc-linux-gnu \
+// RUN: -analyzer-checker=core,debug.ExprInspection,alpha.core.PointerArithm
+
+// RUN: %clang_analyze_cc1 -Wno-unused-value -std=c++14 -verify %s -triple x86_64-pc-linux-gnu \
+// RUN: -analyzer-config support-symbolic-integer-casts=true \
+// RUN: -analyzer-checker=core,debug.ExprInspection,alpha.core.PointerArithm
template <typename T> void clang_analyzer_dump(T);
@@ -141,3 +146,22 @@ int parse(parse_t *p) {
return bits->b; // no-warning
}
} // namespace Bug_55934
+
+void LValueToRValueBitCast_dumps(void *p, char (*array)[8]) {
+ clang_analyzer_dump(p);
+ clang_analyzer_dump(array);
+ // expected-warning at -2 {{&SymRegion{reg_$0<void * p>}}}
+ // expected-warning at -2 {{&SymRegion{reg_$1<char (*)[8] array>}}}
+ clang_analyzer_dump((unsigned long)p);
+ clang_analyzer_dump(__builtin_bit_cast(unsigned long, p));
+ // expected-warning at -2 {{&SymRegion{reg_$0<void * p>} [as 64 bit integer]}}
+ // expected-warning at -2 {{&SymRegion{reg_$0<void * p>} [as 64 bit integer]}}
+ clang_analyzer_dump((unsigned long)array);
+ clang_analyzer_dump(__builtin_bit_cast(unsigned long, array));
+ // expected-warning at -2 {{&SymRegion{reg_$1<char (*)[8] array>} [as 64 bit integer]}}
+ // expected-warning at -2 {{&SymRegion{reg_$1<char (*)[8] array>} [as 64 bit integer]}}
+}
+
+unsigned long ptr_arithmetic(void *p) {
+ return __builtin_bit_cast(unsigned long, p) + 1; // no-crash
+}
diff --git a/clang/test/Analysis/svalbuilder-float-cast.c b/clang/test/Analysis/svalbuilder-float-cast.c
index 0ca9cecf35436..6b0c6f369f718 100644
--- a/clang/test/Analysis/svalbuilder-float-cast.c
+++ b/clang/test/Analysis/svalbuilder-float-cast.c
@@ -1,16 +1,25 @@
// RUN: %clang_analyze_cc1 -analyzer-checker debug.ExprInspection -Wno-deprecated-non-prototype -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker debug.ExprInspection -Wno-deprecated-non-prototype -verify %s \
+// RUN: -analyzer-config support-symbolic-integer-casts=true
+
void clang_analyzer_denote(int, const char *);
void clang_analyzer_express(int);
+void clang_analyzer_dump(int);
+void clang_analyzer_dump_ptr(int *);
void SymbolCast_of_float_type_aux(int *p) {
+ clang_analyzer_dump_ptr(p); // expected-warning {{&x}}
+ clang_analyzer_dump(*p); // expected-warning {{Unknown}}
+ // Storing to the memory region of 'float x' as 'int' will
+ // materialize a fresh conjured symbol to regain accuracy.
*p += 0;
- // FIXME: Ideally, all unknown values should be symbolicated.
- clang_analyzer_denote(*p, "$x"); // expected-warning{{Not a symbol}}
+ clang_analyzer_dump_ptr(p); // expected-warning {{&x}}
+ clang_analyzer_dump(*p); // expected-warning {{conj_$0{int}}
+ 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(void) {
More information about the cfe-commits
mailing list