Author: Gabor Marton Date: 2022-07-05T18:42:34+02:00 New Revision: 5d7fa481cf4d36e14345b1c3ebc951edda0b40b1
URL: https://github.com/llvm/llvm-project/commit/5d7fa481cf4d36e14345b1c3ebc951edda0b40b1 DIFF: https://github.com/llvm/llvm-project/commit/5d7fa481cf4d36e14345b1c3ebc951edda0b40b1.diff LOG: [analyzer] Do not emit redundant SymbolCasts In `RegionStore::getBinding` we call `evalCast` unconditionally to align the stored value's type to the one that is being queried. However, the stored type might be the same, so we may end up having redundant `SymbolCasts` emitted. The solution is to check whether the `to` and `from` type are the same in `makeNonLoc`. Note, we can't just do type equivalence check at the beginning of `evalCast` because when `evalCast` is called from `getBinding` then the original type (`OriginalTy`) is not set, so one operand is missing for the comparison. In `evalCastSubKind(nonloc::SymbolVal)` when the original type is not set, we get the `from` type via `SymbolVal::getType()`. Differential Revision: https://reviews.llvm.org/D128068 Added: clang/test/Analysis/symbolcast-floatingpoint.cpp Modified: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp Removed: ################################################################################ diff --git a/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp b/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp index 13fac37899cd2..7b08acdb569b0 100644 --- a/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp +++ b/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp @@ -113,6 +113,8 @@ nonloc::SymbolVal SValBuilder::makeNonLoc(const SymExpr *operand, QualType fromTy, QualType toTy) { assert(operand); assert(!Loc::isLocType(toTy)); + if (fromTy == toTy) + return operand; return nonloc::SymbolVal(SymMgr.getCastSymbol(operand, fromTy, toTy)); } diff --git a/clang/test/Analysis/symbolcast-floatingpoint.cpp b/clang/test/Analysis/symbolcast-floatingpoint.cpp new file mode 100644 index 0000000000000..8ea4c9b6c24c1 --- /dev/null +++ b/clang/test/Analysis/symbolcast-floatingpoint.cpp @@ -0,0 +1,24 @@ +// RUN: %clang_analyze_cc1 -x c++ -analyzer-checker=debug.ExprInspection \ +// RUN: -analyzer-config support-symbolic-integer-casts=false \ +// RUN: -verify %s + +// RUN: %clang_analyze_cc1 -x c++ -analyzer-checker=debug.ExprInspection \ +// RUN: -analyzer-config support-symbolic-integer-casts=true \ +// RUN: -verify %s + +template <typename T> +void clang_analyzer_dump(T); + +void test_no_redundant_floating_point_cast(int n) { + + double D = n / 30; + clang_analyzer_dump(D); // expected-warning{{(double) ((reg_$0<int n>) / 30)}} + + // There are two cast operations evaluated above: + // 1. (n / 30) is cast to a double during the store of `D`. + // 2. Then in the next line, in RegionStore::getBinding during the load of `D`. + // + // We should not see in the dump of the SVal any redundant casts like + // (double) ((double) $n / 30) + +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
