Skip to content

Fieldregion descript name #112313

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Oct 25, 2024
Merged

Conversation

T-Gruber
Copy link
Contributor

The current implementation of MemRegion::getDescriptiveName fails for FieldRegions whose SuperRegion is an ElementRegion. As outlined below:

struct val_struct { int val; };
extern struct val_struct val_struct_array[3];

void func(){
  // FieldRegion with ElementRegion as SuperRegion.
  val_struct_array[0].val;
}

For this special case, the expression cannot be pretty printed and must therefore be obtained separately.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:static analyzer labels Oct 15, 2024
@llvmbot
Copy link
Member

llvmbot commented Oct 15, 2024

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-static-analyzer-1

Author: None (T-Gruber)

Changes

The current implementation of MemRegion::getDescriptiveName fails for FieldRegions whose SuperRegion is an ElementRegion. As outlined below:

struct val_struct { int val; };
extern struct val_struct val_struct_array[3];

void func(){
  // FieldRegion with ElementRegion as SuperRegion.
  val_struct_array[0].val;
}

For this special case, the expression cannot be pretty printed and must therefore be obtained separately.


Full diff: https://github.com/llvm/llvm-project/pull/112313.diff

2 Files Affected:

  • (modified) clang/lib/StaticAnalyzer/Core/MemRegion.cpp (+20-5)
  • (modified) clang/unittests/StaticAnalyzer/MemRegionDescriptiveNameTest.cpp (+14-1)
diff --git a/clang/lib/StaticAnalyzer/Core/MemRegion.cpp b/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
index 693791c3aee8b9..4144cff8607926 100644
--- a/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
+++ b/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
@@ -751,12 +751,27 @@ std::string MemRegion::getDescriptiveName(bool UseQuotes) const {
   }
 
   // Get variable name.
-  if (R && R->canPrintPrettyAsExpr()) {
-    R->printPrettyAsExpr(os);
-    if (UseQuotes)
-      return (llvm::Twine("'") + os.str() + ArrayIndices + "'").str();
-    else
+  if (R) {
+    // MemRegion can be pretty printed.
+    if (R->canPrintPrettyAsExpr()) {
+      R->printPrettyAsExpr(os);
+      if (UseQuotes)
+        return (llvm::Twine("'") + os.str() + ArrayIndices + "'").str();
       return (llvm::Twine(os.str()) + ArrayIndices).str();
+    }
+
+    // FieldRegion may have ElementRegion as SuperRegion.
+    if (const clang::ento::FieldRegion *FR =
+            R->getAs<clang::ento::FieldRegion>()) {
+      std::string Super = FR->getSuperRegion()->getDescriptiveName(false);
+      if (Super.empty())
+        return "";
+
+      if (UseQuotes)
+        return (llvm::Twine("'") + Super + "." + FR->getDecl()->getName() + "'")
+            .str();
+      return (llvm::Twine(Super) + "." + FR->getDecl()->getName()).str();
+    }
   }
 
   return VariableName;
diff --git a/clang/unittests/StaticAnalyzer/MemRegionDescriptiveNameTest.cpp b/clang/unittests/StaticAnalyzer/MemRegionDescriptiveNameTest.cpp
index b13e7123ee524d..966e5c0e9a6124 100644
--- a/clang/unittests/StaticAnalyzer/MemRegionDescriptiveNameTest.cpp
+++ b/clang/unittests/StaticAnalyzer/MemRegionDescriptiveNameTest.cpp
@@ -12,7 +12,6 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h"
 #include "gtest/gtest.h"
-#include <fstream>
 
 using namespace clang;
 using namespace ento;
@@ -143,4 +142,18 @@ void top() {
   EXPECT_EQ(Output, "DescriptiveNameChecker: array[x]\n");
 }
 
+TEST(MemRegionDescriptiveNameTest, FieldRegWithSuperElementReg) {
+  StringRef Code = R"cpp(
+void reportDescriptiveName(int *p);
+struct val_struct { int val; };
+extern struct val_struct val_struct_array[3];
+void top() {
+  reportDescriptiveName(&val_struct_array[0].val);
+})cpp";
+
+  std::string Output;
+  ASSERT_TRUE(runChecker(Code, Output));
+  EXPECT_EQ(Output, "DescriptiveNameChecker: val_struct_array[0].val\n");
+}
+
 } // namespace

Copy link
Contributor

@steakhal steakhal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@T-Gruber
Copy link
Contributor Author

LGTM

Hi @steakhal, should we then merge it? Or do you have any comments?

@steakhal steakhal merged commit 86d65ae into llvm:main Oct 25, 2024
8 checks passed
NoumanAmir657 pushed a commit to NoumanAmir657/llvm-project that referenced this pull request Nov 4, 2024
The current implementation of MemRegion::getDescriptiveName fails for
FieldRegions whose SuperRegion is an ElementRegion. As outlined below:
```Cpp
struct val_struct { int val; };
extern struct val_struct val_struct_array[3];

void func(){
  // FieldRegion with ElementRegion as SuperRegion.
  val_struct_array[0].val;
}
```

For this special case, the expression cannot be pretty printed and must
therefore be obtained separately.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:static analyzer clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants