================
@@ -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 "";
+ }
----------------
NagyDonat wrote:
```suggestion
// Index is symbolic, but may have a descriptive name
else {
auto SI = ER->getIndex().getAs<nonloc::SymbolVal>();
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();
}
```
I realize it just now, but there are several failure cases that are not
handled/handled incorrectly within this code:
(1) `ER->getIndex().getAs<nonloc::SymbolVal>()` may return `nullptr` in the
unusual but perfectly legal case when the index is a symbolic value that is
e.g. a `nonloc::LocAsInteger` (a pointer converted to an integer type).
(2) `SymbolVal::getAsSymbol()` returns nonnull, but
`SI->getAsSymbol()->getOriginRegion();` may return `nullptr` when we have a
symbolic value that doesn't originate as the value of a memory region (e.g.
it's a conjured symbol, the integer returned by an opaque function).
(3) Finally `OR->getDescriptiveName(false)` may return an empty string when the
origin region doesn't have a descriptive name (e.g. it's a symbolic region
behind an opaque pointer, a nameless heap area returned by `malloc` etc.).
In each of these three situations the right thing to do is returning an empty
string, because `getDescriptiveName` already signals failure by returning `""`
when it cannot find a natural descriptive name. (And code that uses this
function checks for an empty return value.)
Disclaimer: I did not test the code that I'm suggesting here; there may be
typos in it. Also, it would be good to have testcases that cover the situations
(1)-(3) and verify that this method doesn't produce incorrect output under them.
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
