[flang] Fix use-after-free cases found by valgrind

[klausler]

The expression traversal library needs to use interfaces into triplets (and substrings) that return pointers to nested expressions, rather than optional copies of them, since at least one semantic analysis collects a set of references to some subexpression representation class instances, and those references obviously can't point to local copies of objects.

Fixes #121999.

[llvmbot]

@llvm/pr-subscribers-flang-semantics

Author: Peter Klausler (klausler)

Changes

The expression traversal library needs to use interfaces into triplets (and substrings) that return pointers to nested expressions, rather than optional copies of them, since at least one semantic analysis collects a set of references to some subexpression representation class instances, and those references obviously can't point to local copies of objects.

Fixes #121999.

---

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

2 Files Affected:

• (modified) flang/include/flang/Evaluate/traverse.h (+2-2)
• (modified) flang/include/flang/Evaluate/variable.h (+6)

diff --git a/flang/include/flang/Evaluate/traverse.h b/flang/include/flang/Evaluate/traverse.h
index 90b93f6afd3515..dd38d64bff63f7 100644
--- a/flang/include/flang/Evaluate/traverse.h
+++ b/flang/include/flang/Evaluate/traverse.h
@@ -139,7 +139,7 @@ class Traverse {
     return visitor_(x.base());
   }
   Result operator()(const Triplet &x) const {
-    return Combine(x.lower(), x.upper(), x.stride());
+    return Combine(x.GetLower(), x.GetUpper(), x.GetStride());
   }
   Result operator()(const Subscript &x) const { return visitor_(x.u); }
   Result operator()(const ArrayRef &x) const {
@@ -151,7 +151,7 @@ class Traverse {
   }
   Result operator()(const DataRef &x) const { return visitor_(x.u); }
   Result operator()(const Substring &x) const {
-    return Combine(x.parent(), x.lower(), x.upper());
+    return Combine(x.parent(), x.GetLower(), x.GetUpper());
   }
   Result operator()(const ComplexPart &x) const {
     return visitor_(x.complex());
diff --git a/flang/include/flang/Evaluate/variable.h b/flang/include/flang/Evaluate/variable.h
index b454d37d93e57b..9b597d29813da1 100644
--- a/flang/include/flang/Evaluate/variable.h
+++ b/flang/include/flang/Evaluate/variable.h
@@ -331,8 +331,14 @@ class Substring {
   }
 
   Expr<SubscriptInteger> lower() const;
+  const Expr<SubscriptInteger> *GetLower() const {
+    return lower_.has_value() ? &lower_->value() : nullptr;
+  }
   Substring &set_lower(Expr<SubscriptInteger> &&);
   std::optional<Expr<SubscriptInteger>> upper() const;
+  const Expr<SubscriptInteger> *GetUpper() const {
+    return upper_.has_value() ? &upper_->value() : nullptr;
+  }
   Substring &set_upper(Expr<SubscriptInteger> &&);
   const Parent &parent() const { return parent_; }
   Parent &parent() { return parent_; }

[eugeneepshteyn]

Oh, so the old code was visiting a chain of automatic instances, instead of pointers?

[klausler]

The current code ends up visiting heap objects that had already been destroyed because they had been owned by local variables that had gone out of scope.