================ @@ -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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits