From 86d479fd7c837e97be116ffb9e4c92812b87360f Mon Sep 17 00:00:00 2001 From: T-Gruber <100079402+T-Gruber@users.noreply.github.com> Date: Thu, 21 Mar 2024 18:27:53 +0100 Subject: [PATCH] Adapted MemRegion::getDescriptiveName to handle ElementRegions (#85104) Fixes https://github.com/llvm/llvm-project/issues/84463 Changes: - Adapted MemRegion::getDescriptiveName - Added unittest to check name for a given clang::ento::ElementRegion - Some format changes due to clang-format --------- Co-authored-by: Andreas Steinhausen Co-authored-by: Balazs Benics --- clang/lib/StaticAnalyzer/Core/MemRegion.cpp | 20 ++- clang/unittests/StaticAnalyzer/CMakeLists.txt | 1 + .../MemRegionDescriptiveNameTest.cpp | 145 ++++++++++++++++++ .../clang/unittests/StaticAnalyzer/BUILD.gn | 1 + 4 files changed, 161 insertions(+), 6 deletions(-) create mode 100644 clang/unittests/StaticAnalyzer/MemRegionDescriptiveNameTest.cpp diff --git a/clang/lib/StaticAnalyzer/Core/MemRegion.cpp b/clang/lib/StaticAnalyzer/Core/MemRegion.cpp index 16db6b249dc92b..a8e573f7982b12 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 775f0f8486b8f9..519be36fe0fa38 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 00000000000000..ba0c4d25e13b4d --- /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 01c2b6ced3366f..9230ac79a48ae3 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",