diff --git a/clang/lib/StaticAnalyzer/Core/MemRegion.cpp b/clang/lib/StaticAnalyzer/Core/MemRegion.cpp index 16db6b249dc92..a8e573f7982b1 100644 --- a/clang/lib/StaticAnalyzer/Core/MemRegion.cpp +++ b/clang/lib/StaticAnalyzer/Core/MemRegion.cpp @@ -720,13 +720,21 @@ std::string MemRegion::getDescriptiveName(bool UseQuotes) const { CI->getValue().toString(Idx); ArrayIndices = (llvm::Twine("[") + Idx.str() + "]" + ArrayIndices).str(); } - // If not a ConcreteInt, try to obtain the variable - // name by calling 'getDescriptiveName' recursively. + // Index is symbolic, but may have a descriptive name. else { - std::string Idx = ER->getDescriptiveName(false); - if (!Idx.empty()) { - ArrayIndices = (llvm::Twine("[") + Idx + "]" + ArrayIndices).str(); - } + auto SI = ER->getIndex().getAs(); + if (!SI) + return ""; + + const MemRegion *OR = SI->getAsSymbol()->getOriginRegion(); + if (!OR) + return ""; + + std::string Idx = OR->getDescriptiveName(false); + if (Idx.empty()) + return ""; + + ArrayIndices = (llvm::Twine("[") + Idx + "]" + ArrayIndices).str(); } R = ER->getSuperRegion(); } diff --git a/clang/unittests/StaticAnalyzer/CMakeLists.txt b/clang/unittests/StaticAnalyzer/CMakeLists.txt index 775f0f8486b8f..519be36fe0fa3 100644 --- a/clang/unittests/StaticAnalyzer/CMakeLists.txt +++ b/clang/unittests/StaticAnalyzer/CMakeLists.txt @@ -11,6 +11,7 @@ add_clang_unittest(StaticAnalysisTests CallEventTest.cpp ConflictingEvalCallsTest.cpp FalsePositiveRefutationBRVisitorTest.cpp + MemRegionDescriptiveNameTest.cpp NoStateChangeFuncVisitorTest.cpp ParamRegionTest.cpp RangeSetTest.cpp diff --git a/clang/unittests/StaticAnalyzer/MemRegionDescriptiveNameTest.cpp b/clang/unittests/StaticAnalyzer/MemRegionDescriptiveNameTest.cpp new file mode 100644 index 0000000000000..ba0c4d25e13b4 --- /dev/null +++ b/clang/unittests/StaticAnalyzer/MemRegionDescriptiveNameTest.cpp @@ -0,0 +1,145 @@ +//===- MemRegionDescriptiveNameTest.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 "CheckerRegistration.h" +#include "clang/StaticAnalyzer/Core/Checker.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h" +#include "gtest/gtest.h" +#include + +using namespace clang; +using namespace ento; + +namespace { + +class DescriptiveNameChecker : public Checker { +public: + void checkPreCall(const CallEvent &Call, CheckerContext &C) const { + if (!HandlerFn.matches(Call)) + return; + + const MemRegion *ArgReg = Call.getArgSVal(0).getAsRegion(); + assert(ArgReg && "expecting a location as the first argument"); + + auto DescriptiveName = ArgReg->getDescriptiveName(/*UseQuotes=*/false); + if (ExplodedNode *Node = C.generateNonFatalErrorNode(C.getState())) { + auto Report = + std::make_unique(Bug, DescriptiveName, Node); + C.emitReport(std::move(Report)); + } + } + +private: + const BugType Bug{this, "DescriptiveNameBug"}; + const CallDescription HandlerFn = {{"reportDescriptiveName"}, 1}; +}; + +void addDescriptiveNameChecker(AnalysisASTConsumer &AnalysisConsumer, + AnalyzerOptions &AnOpts) { + AnOpts.CheckersAndPackages = {{"DescriptiveNameChecker", true}}; + AnalysisConsumer.AddCheckerRegistrationFn([](CheckerRegistry &Registry) { + Registry.addChecker("DescriptiveNameChecker", + "Desc", "DocsURI"); + }); +} + +bool runChecker(StringRef Code, std::string &Output) { + return runCheckerOnCode(Code.str(), Output, + /*OnlyEmitWarnings=*/true); +} + +TEST(MemRegionDescriptiveNameTest, ConcreteIntElementRegionIndex) { + StringRef Code = R"cpp( +void reportDescriptiveName(int *p); +const unsigned int index = 1; +extern int array[3]; +void top() { + reportDescriptiveName(&array[index]); +})cpp"; + + std::string Output; + ASSERT_TRUE(runChecker(Code, Output)); + EXPECT_EQ(Output, "DescriptiveNameChecker: array[1]\n"); +} + +TEST(MemRegionDescriptiveNameTest, SymbolicElementRegionIndex) { + StringRef Code = R"cpp( +void reportDescriptiveName(int *p); +extern unsigned int index; +extern int array[3]; +void top() { + reportDescriptiveName(&array[index]); +})cpp"; + + std::string Output; + ASSERT_TRUE(runChecker(Code, Output)); + EXPECT_EQ(Output, "DescriptiveNameChecker: array[index]\n"); +} + +TEST(MemRegionDescriptiveNameTest, SymbolicElementRegionIndexSymbolValFails) { + StringRef Code = R"cpp( +void reportDescriptiveName(int *p); +extern int* ptr; +extern int array[3]; +void top() { + reportDescriptiveName(&array[(long)ptr]); +})cpp"; + + std::string Output; + ASSERT_TRUE(runChecker(Code, Output)); + EXPECT_EQ(Output, "DescriptiveNameChecker: \n"); +} + +TEST(MemRegionDescriptiveNameTest, SymbolicElementRegionIndexOrigRegionFails) { + StringRef Code = R"cpp( +void reportDescriptiveName(int *p); +extern int getInt(void); +extern int array[3]; +void top() { + reportDescriptiveName(&array[getInt()]); +})cpp"; + + std::string Output; + ASSERT_TRUE(runChecker(Code, Output)); + EXPECT_EQ(Output, "DescriptiveNameChecker: \n"); +} + +TEST(MemRegionDescriptiveNameTest, SymbolicElementRegionIndexDescrNameFails) { + StringRef Code = R"cpp( +void reportDescriptiveName(int *p); +extern int *ptr; +extern int array[3]; +void top() { + reportDescriptiveName(&array[*ptr]); +})cpp"; + + std::string Output; + ASSERT_TRUE(runChecker(Code, Output)); + EXPECT_EQ(Output, "DescriptiveNameChecker: \n"); +} + +TEST(MemRegionDescriptiveNameTest, + SymbolicElementRegionIndexIncorrectSymbolName) { + StringRef Code = R"cpp( +void reportDescriptiveName(int *p); +extern int x, y; +extern int array[3]; +void top() { + y = x; + reportDescriptiveName(&array[y]); +})cpp"; + + std::string Output; + ASSERT_TRUE(runChecker(Code, Output)); + // FIXME: Should return array[y], but returns array[x] (OriginRegion). + EXPECT_EQ(Output, "DescriptiveNameChecker: array[x]\n"); +} + +} // namespace diff --git a/llvm/utils/gn/secondary/clang/unittests/StaticAnalyzer/BUILD.gn b/llvm/utils/gn/secondary/clang/unittests/StaticAnalyzer/BUILD.gn index 01c2b6ced3366..9230ac79a48ae 100644 --- a/llvm/utils/gn/secondary/clang/unittests/StaticAnalyzer/BUILD.gn +++ b/llvm/utils/gn/secondary/clang/unittests/StaticAnalyzer/BUILD.gn @@ -19,6 +19,7 @@ unittest("StaticAnalysisTests") { "CallEventTest.cpp", "ConflictingEvalCallsTest.cpp", "FalsePositiveRefutationBRVisitorTest.cpp", + "MemRegionDescriptiveNameTest.cpp", "NoStateChangeFuncVisitorTest.cpp", "ParamRegionTest.cpp", "RangeSetTest.cpp",