[clang] [llvm] Adapted MemRegion::getDescriptiveName to handle ElementRegions (PR #85104)

Balazs Benics via cfe-commits cfe-commits at lists.llvm.org
Thu Mar 14 06:56:22 PDT 2024


================
@@ -720,14 +720,20 @@ 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.
-    else {
-      std::string Idx = ER->getDescriptiveName(false);
-      if (!Idx.empty()) {
-        ArrayIndices = (llvm::Twine("[") + Idx + "]" + ArrayIndices).str();
+    // Index is a SymbolVal.
+    else if (auto SI = ER->getIndex().getAs<nonloc::SymbolVal>()) {
+      if (SymbolRef SR = SI->getAsSymbol()) {
+        if (const MemRegion *OR = SR->getOriginRegion()) {
+          std::string Idx = OR->getDescriptiveName(false);
+          ArrayIndices = (llvm::Twine("[") + Idx + "]" + ArrayIndices).str();
+        }
       }
     }
+    // Index is neither a ConcreteInt nor SymbolVal, give up and return.
+    else {
+      assert(false && "we should have a descriptive name");
+      return "";
+    }
----------------
steakhal wrote:

Let me elaborate what I think.
To set the scene, I'd like to point out that there was an unconditional infinite recursion in the previous code; and we in practice never seen crashes related to this so I think it's fair to say that currently we don't have any checkers exercising this path. This suggests to me that we could basically do anything here, that is better than a certain crash (aka. improve the previous handling).

So, at this point I'm okay to accept any code that at least sometimes work - given that we have a test for exercising that path.

And btw, if any of the sub-conditions fail, we hit the `else` branch and we assert in debug builds to get notified, and silently ignore the case (and may produce incomplete messages that people can still notice and report) in release builds.
Consequently, this shouldn't affect users in a blocking way.

https://github.com/llvm/llvm-project/pull/85104


More information about the cfe-commits mailing list