r358722 - [analyzer] Make default bindings to variables actually work.
Artem Dergachev via cfe-commits
cfe-commits at lists.llvm.org
Thu Apr 18 16:35:56 PDT 2019
Author: dergachev
Date: Thu Apr 18 16:35:56 2019
New Revision: 358722
URL: http://llvm.org/viewvc/llvm-project?rev=358722&view=rev
Log:
[analyzer] Make default bindings to variables actually work.
Default RegionStore bindings represent values that can be obtained by loading
from anywhere within the region, not just the specific offset within the region
that they are said to be bound to. For example, default-binding a character \0
to an int (eg., via memset()) means that the whole int is 0, not just
that its lower byte is 0.
Even though memset and bzero were modeled this way, it didn't work correctly
when applied to simple variables. Eg., in
int x;
memset(x, 0, sizeof(x));
we did produce a default binding, but were unable to read it later, and 'x'
was perceived as an uninitialized variable even after memset.
At the same time, if we replace 'x' with a variable of a structure or array
type, accessing fields or elements of such variable was working correctly,
which was enough for most cases. So this was only a problem for variables of
simple integer/enumeration/floating-point/pointer types.
Fix loading default bindings from RegionStore for regions of simple variables.
Add a unit test to document the API contract as well.
Differential Revision: https://reviews.llvm.org/D60742
Added:
cfe/trunk/unittests/StaticAnalyzer/StoreTest.cpp
Modified:
cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
cfe/trunk/test/Analysis/string.c
cfe/trunk/unittests/StaticAnalyzer/CMakeLists.txt
Modified: cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp?rev=358722&r1=358721&r2=358722&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp Thu Apr 18 16:35:56 2019
@@ -1927,7 +1927,10 @@ SVal RegionStoreManager::getBindingForVa
const VarRegion *R) {
// Check if the region has a binding.
- if (const Optional<SVal> &V = B.getDirectBinding(R))
+ if (Optional<SVal> V = B.getDirectBinding(R))
+ return *V;
+
+ if (Optional<SVal> V = B.getDefaultBinding(R))
return *V;
// Lazily derive a value for the VarRegion.
Modified: cfe/trunk/test/Analysis/string.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/string.c?rev=358722&r1=358721&r2=358722&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/string.c (original)
+++ cfe/trunk/test/Analysis/string.c Thu Apr 18 16:35:56 2019
@@ -1554,3 +1554,9 @@ void memset28() {
// This should be true.
clang_analyzer_eval(x == 0x101); // expected-warning{{UNKNOWN}}
}
+
+void memset29_plain_int_zero() {
+ short x;
+ memset(&x, 0, sizeof(short));
+ clang_analyzer_eval(x == 0); // expected-warning{{TRUE}}
+}
Modified: cfe/trunk/unittests/StaticAnalyzer/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/StaticAnalyzer/CMakeLists.txt?rev=358722&r1=358721&r2=358722&view=diff
==============================================================================
--- cfe/trunk/unittests/StaticAnalyzer/CMakeLists.txt (original)
+++ cfe/trunk/unittests/StaticAnalyzer/CMakeLists.txt Thu Apr 18 16:35:56 2019
@@ -4,6 +4,7 @@ set(LLVM_LINK_COMPONENTS
add_clang_unittest(StaticAnalysisTests
AnalyzerOptionsTest.cpp
+ StoreTest.cpp
RegisterCustomCheckersTest.cpp
SymbolReaperTest.cpp
)
Added: cfe/trunk/unittests/StaticAnalyzer/StoreTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/StaticAnalyzer/StoreTest.cpp?rev=358722&view=auto
==============================================================================
--- cfe/trunk/unittests/StaticAnalyzer/StoreTest.cpp (added)
+++ cfe/trunk/unittests/StaticAnalyzer/StoreTest.cpp Thu Apr 18 16:35:56 2019
@@ -0,0 +1,105 @@
+//===- unittests/StaticAnalyzer/StoreTest.cpp -----------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "Reusables.h"
+
+#include "clang/Tooling/Tooling.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace ento {
+namespace {
+
+// Test that we can put a value into an int-type variable and load it
+// back from that variable. Test what happens if default bindings are used.
+class VariableBindConsumer : public ExprEngineConsumer {
+ void performTest(const Decl *D) {
+ StoreManager &StMgr = Eng.getStoreManager();
+ SValBuilder &SVB = Eng.getSValBuilder();
+ MemRegionManager &MRMgr = StMgr.getRegionManager();
+ const ASTContext &ACtx = Eng.getContext();
+
+ const auto *VDX0 = findDeclByName<VarDecl>(D, "x0");
+ const auto *VDY0 = findDeclByName<VarDecl>(D, "y0");
+ const auto *VDZ0 = findDeclByName<VarDecl>(D, "z0");
+ const auto *VDX1 = findDeclByName<VarDecl>(D, "x1");
+ const auto *VDY1 = findDeclByName<VarDecl>(D, "y1");
+ assert(VDX0 && VDY0 && VDZ0 && VDX1 && VDY1);
+
+ const StackFrameContext *SFC =
+ Eng.getAnalysisDeclContextManager().getStackFrame(D);
+
+ Loc LX0 = loc::MemRegionVal(MRMgr.getVarRegion(VDX0, SFC));
+ Loc LY0 = loc::MemRegionVal(MRMgr.getVarRegion(VDY0, SFC));
+ Loc LZ0 = loc::MemRegionVal(MRMgr.getVarRegion(VDZ0, SFC));
+ Loc LX1 = loc::MemRegionVal(MRMgr.getVarRegion(VDX1, SFC));
+ Loc LY1 = loc::MemRegionVal(MRMgr.getVarRegion(VDY1, SFC));
+
+ Store StInit = StMgr.getInitialStore(SFC).getStore();
+ SVal Zero = SVB.makeZeroVal(ACtx.IntTy);
+ SVal One = SVB.makeIntVal(1, ACtx.IntTy);
+ SVal NarrowZero = SVB.makeZeroVal(ACtx.CharTy);
+
+ // Bind(Zero)
+ Store StX0 =
+ StMgr.Bind(StInit, LX0, Zero).getStore();
+ ASSERT_EQ(Zero, StMgr.getBinding(StX0, LX0, ACtx.IntTy));
+
+ // BindDefaultInitial(Zero)
+ Store StY0 =
+ StMgr.BindDefaultInitial(StInit, LY0.getAsRegion(), Zero).getStore();
+ ASSERT_EQ(Zero, StMgr.getBinding(StY0, LY0, ACtx.IntTy));
+ ASSERT_EQ(Zero, *StMgr.getDefaultBinding(StY0, LY0.getAsRegion()));
+
+ // BindDefaultZero()
+ Store StZ0 =
+ StMgr.BindDefaultZero(StInit, LZ0.getAsRegion()).getStore();
+ // BindDefaultZero wipes the region with '0 S8b', not with out Zero.
+ // Direct load, however, does give us back the object of the type
+ // that we specify for loading.
+ ASSERT_EQ(Zero, StMgr.getBinding(StZ0, LZ0, ACtx.IntTy));
+ ASSERT_EQ(NarrowZero, *StMgr.getDefaultBinding(StZ0, LZ0.getAsRegion()));
+
+ // Bind(One)
+ Store StX1 =
+ StMgr.Bind(StInit, LX1, One).getStore();
+ ASSERT_EQ(One, StMgr.getBinding(StX1, LX1, ACtx.IntTy));
+
+ // BindDefaultInitial(One)
+ Store StY1 =
+ StMgr.BindDefaultInitial(StInit, LY1.getAsRegion(), One).getStore();
+ ASSERT_EQ(One, StMgr.getBinding(StY1, LY1, ACtx.IntTy));
+ ASSERT_EQ(One, *StMgr.getDefaultBinding(StY1, LY1.getAsRegion()));
+ }
+
+public:
+ VariableBindConsumer(CompilerInstance &C) : ExprEngineConsumer(C) {}
+
+ bool HandleTopLevelDecl(DeclGroupRef DG) override {
+ for (const auto *D : DG)
+ performTest(D);
+ return true;
+ }
+};
+
+class VariableBindAction : public ASTFrontendAction {
+public:
+ std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &Compiler,
+ StringRef File) override {
+ return llvm::make_unique<VariableBindConsumer>(Compiler);
+ }
+};
+
+TEST(Store, VariableBind) {
+ EXPECT_TRUE(tooling::runToolOnCode(
+ new VariableBindAction, "void foo() { int x0, y0, z0, x1, y1; }"));
+}
+
+} // namespace
+} // namespace ento
+} // namespace clang
More information about the cfe-commits
mailing list