[flang] Fix references to destroyed objects #111582
Open
+19
−9
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
flang
|
@llvm/pr-subscribers-flang-semantics Author: Peter Klausler (klausler) ChangesProgramTree instances are created as the value of a local variable in the Pre(const parser::ProgramUnit &) member function in name resolution. But references to these ProgramTree instances can persist in SubprogramNameDetails symbol table entries that might survive that function call's lifetime, and lead to trouble later when (e.g.) expression semantics needs to deal with a possible forward reference in a function reference in an expression being processed later in expression checking. So put those ProgramTree instances into a longer-lived linked list within the SemanticsContext. Might fix some weird crashes reported on big-endian targets (AIX & Solaris). Full diff: https://github.com/llvm/llvm-project/pull/111582.diff 8 Files Affected:
diff --git a/flang/lib/Semantics/program-tree.h b/flang/include/flang/Semantics/program-tree.h
similarity index 97%
rename from flang/lib/Semantics/program-tree.h
rename to flang/include/flang/Semantics/program-tree.h
index ab00261a964a13..1c89e6c175b964 100644
--- a/flang/lib/Semantics/program-tree.h
+++ b/flang/include/flang/Semantics/program-tree.h
@@ -9,8 +9,8 @@
#ifndef FORTRAN_SEMANTICS_PROGRAM_TREE_H_
#define FORTRAN_SEMANTICS_PROGRAM_TREE_H_
+#include "symbol.h"
#include "flang/Parser/parse-tree.h"
-#include "flang/Semantics/symbol.h"
#include <list>
#include <variant>
@@ -35,7 +35,7 @@ class ProgramTree {
std::list<common::Reference<const parser::GenericSpec>>;
// Build the ProgramTree rooted at one of these program units.
- static ProgramTree Build(const parser::ProgramUnit &, SemanticsContext &);
+ static ProgramTree &Build(const parser::ProgramUnit &, SemanticsContext &);
static std::optional<ProgramTree> Build(
const parser::MainProgram &, SemanticsContext &);
static std::optional<ProgramTree> Build(
diff --git a/flang/include/flang/Semantics/semantics.h b/flang/include/flang/Semantics/semantics.h
index 606afbe288c38d..c981d86fbd94cb 100644
--- a/flang/include/flang/Semantics/semantics.h
+++ b/flang/include/flang/Semantics/semantics.h
@@ -9,6 +9,8 @@
#ifndef FORTRAN_SEMANTICS_SEMANTICS_H_
#define FORTRAN_SEMANTICS_SEMANTICS_H_
+#include "module-dependences.h"
+#include "program-tree.h"
#include "scope.h"
#include "symbol.h"
#include "flang/Common/Fortran-features.h"
@@ -17,7 +19,6 @@
#include "flang/Evaluate/intrinsics.h"
#include "flang/Evaluate/target.h"
#include "flang/Parser/message.h"
-#include "flang/Semantics/module-dependences.h"
#include <iosfwd>
#include <set>
#include <string>
@@ -280,6 +281,9 @@ class SemanticsContext {
void DumpSymbols(llvm::raw_ostream &);
+ // Top-level ProgramTrees are owned by the SemanticsContext for persistence.
+ ProgramTree &SaveProgramTree(ProgramTree &&);
+
private:
struct ScopeIndexComparator {
bool operator()(parser::CharBlock, parser::CharBlock) const;
@@ -331,6 +335,7 @@ class SemanticsContext {
ModuleDependences moduleDependences_;
std::map<const Symbol *, SourceName> moduleFileOutputRenamings_;
UnorderedSymbolSet isDefined_;
+ std::list<ProgramTree> programTrees_;
};
class Semantics {
diff --git a/flang/lib/Semantics/program-tree.cpp b/flang/lib/Semantics/program-tree.cpp
index 250f5801b39e1a..86085e78803a23 100644
--- a/flang/lib/Semantics/program-tree.cpp
+++ b/flang/lib/Semantics/program-tree.cpp
@@ -6,7 +6,7 @@
//
//===----------------------------------------------------------------------===//
-#include "program-tree.h"
+#include "flang/Semantics/program-tree.h"
#include "flang/Common/idioms.h"
#include "flang/Parser/char-block.h"
#include "flang/Semantics/scope.h"
@@ -130,13 +130,13 @@ static ProgramTree BuildModuleTree(
return node;
}
-ProgramTree ProgramTree::Build(
+ProgramTree &ProgramTree::Build(
const parser::ProgramUnit &x, SemanticsContext &context) {
return common::visit(
- [&](const auto &y) {
+ [&](const auto &y) -> ProgramTree & {
auto node{Build(y.value(), context)};
CHECK(node.has_value());
- return std::move(*node);
+ return context.SaveProgramTree(std::move(*node));
},
x.u);
}
diff --git a/flang/lib/Semantics/resolve-names-utils.cpp b/flang/lib/Semantics/resolve-names-utils.cpp
index b8ce8d14a33faa..a838d49c06104d 100644
--- a/flang/lib/Semantics/resolve-names-utils.cpp
+++ b/flang/lib/Semantics/resolve-names-utils.cpp
@@ -31,8 +31,6 @@ using common::NumericOperator;
using common::RelationalOperator;
using IntrinsicOperator = parser::DefinedOperator::IntrinsicOperator;
-static constexpr const char *operatorPrefix{"operator("};
-
static GenericKind MapIntrinsicOperator(IntrinsicOperator);
Symbol *Resolve(const parser::Name &name, Symbol *symbol) {
@@ -69,37 +67,6 @@ bool IsIntrinsicOperator(
return false;
}
-template <typename E>
-std::forward_list<std::string> GetOperatorNames(
- const SemanticsContext &context, E opr) {
- std::forward_list<std::string> result;
- for (const char *name : context.languageFeatures().GetNames(opr)) {
- result.emplace_front(std::string{operatorPrefix} + name + ')');
- }
- return result;
-}
-
-std::forward_list<std::string> GetAllNames(
- const SemanticsContext &context, const SourceName &name) {
- std::string str{name.ToString()};
- if (!name.empty() && name.end()[-1] == ')' &&
- name.ToString().rfind(std::string{operatorPrefix}, 0) == 0) {
- for (int i{0}; i != common::LogicalOperator_enumSize; ++i) {
- auto names{GetOperatorNames(context, LogicalOperator{i})};
- if (llvm::is_contained(names, str)) {
- return names;
- }
- }
- for (int i{0}; i != common::RelationalOperator_enumSize; ++i) {
- auto names{GetOperatorNames(context, RelationalOperator{i})};
- if (llvm::is_contained(names, str)) {
- return names;
- }
- }
- }
- return {str};
-}
-
bool IsLogicalConstant(
const SemanticsContext &context, const SourceName &name) {
std::string str{name.ToString()};
diff --git a/flang/lib/Semantics/resolve-names-utils.h b/flang/lib/Semantics/resolve-names-utils.h
index 5b537d80e5f880..64784722ff4f84 100644
--- a/flang/lib/Semantics/resolve-names-utils.h
+++ b/flang/lib/Semantics/resolve-names-utils.h
@@ -51,11 +51,6 @@ parser::MessageFixedText WithSeverity(
bool IsIntrinsicOperator(const SemanticsContext &, const SourceName &);
bool IsLogicalConstant(const SemanticsContext &, const SourceName &);
-// Some intrinsic operators have more than one name (e.g. `operator(.eq.)` and
-// `operator(==)`). GetAllNames() returns them all, including symbolName.
-std::forward_list<std::string> GetAllNames(
- const SemanticsContext &, const SourceName &);
-
template <typename T>
MaybeIntExpr EvaluateIntExpr(SemanticsContext &context, const T &expr) {
if (MaybeExpr maybeExpr{
diff --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp
index e5e03f644f1b00..f1ce0b415ebe9c 100644
--- a/flang/lib/Semantics/resolve-names.cpp
+++ b/flang/lib/Semantics/resolve-names.cpp
@@ -10,7 +10,6 @@
#include "definable.h"
#include "mod-file.h"
#include "pointer-assignment.h"
-#include "program-tree.h"
#include "resolve-directives.h"
#include "resolve-names-utils.h"
#include "rewrite-parse-tree.h"
@@ -32,6 +31,7 @@
#include "flang/Parser/tools.h"
#include "flang/Semantics/attr.h"
#include "flang/Semantics/expression.h"
+#include "flang/Semantics/program-tree.h"
#include "flang/Semantics/scope.h"
#include "flang/Semantics/semantics.h"
#include "flang/Semantics/symbol.h"
@@ -2490,6 +2490,7 @@ Symbol &ScopeHandler::CopySymbol(const SourceName &name, const Symbol &symbol) {
}
// Look for name only in scope, not in enclosing scopes.
+
Symbol *ScopeHandler::FindInScope(
const Scope &scope, const parser::Name &name) {
return Resolve(name, FindInScope(scope, name.source));
@@ -9120,7 +9121,7 @@ bool ResolveNamesVisitor::Pre(const parser::ProgramUnit &x) {
ResolveAccParts(context(), x, &topScope_);
return false;
}
- auto root{ProgramTree::Build(x, context())};
+ ProgramTree &root{ProgramTree::Build(x, context())};
SetScope(topScope_);
ResolveSpecificationParts(root);
FinishSpecificationParts(root);
diff --git a/flang/lib/Semantics/semantics.cpp b/flang/lib/Semantics/semantics.cpp
index 637088ff0171c0..58dc1f218b56f4 100644
--- a/flang/lib/Semantics/semantics.cpp
+++ b/flang/lib/Semantics/semantics.cpp
@@ -663,6 +663,10 @@ void SemanticsContext::DumpSymbols(llvm::raw_ostream &os) {
DoDumpSymbols(os, globalScope());
}
+ProgramTree &SemanticsContext::SaveProgramTree(ProgramTree &&tree) {
+ return programTrees_.emplace_back(std::move(tree));
+}
+
void Semantics::DumpSymbols(llvm::raw_ostream &os) { context_.DumpSymbols(os); }
void Semantics::DumpSymbolsSources(llvm::raw_ostream &os) const {
diff --git a/flang/lib/Semantics/tools.cpp b/flang/lib/Semantics/tools.cpp
index 4d2a0a607abe89..379d5d0eb3eef0 100644
--- a/flang/lib/Semantics/tools.cpp
+++ b/flang/lib/Semantics/tools.cpp
@@ -1654,6 +1654,37 @@ bool HasDefinedIo(common::DefinedIo which, const DerivedTypeSpec &derived,
return parentType && HasDefinedIo(which, *parentType, scope);
}
+template <typename E>
+std::forward_list<std::string> GetOperatorNames(
+ const SemanticsContext &context, E opr) {
+ std::forward_list<std::string> result;
+ for (const char *name : context.languageFeatures().GetNames(opr)) {
+ result.emplace_front("operator("s + name + ')');
+ }
+ return result;
+}
+
+std::forward_list<std::string> GetAllNames(
+ const SemanticsContext &context, const SourceName &name) {
+ std::string str{name.ToString()};
+ if (!name.empty() && name.end()[-1] == ')' &&
+ name.ToString().rfind("operator(", 0) == 0) {
+ for (int i{0}; i != common::LogicalOperator_enumSize; ++i) {
+ auto names{GetOperatorNames(context, common::LogicalOperator{i})};
+ if (llvm::is_contained(names, str)) {
+ return names;
+ }
+ }
+ for (int i{0}; i != common::RelationalOperator_enumSize; ++i) {
+ auto names{GetOperatorNames(context, common::RelationalOperator{i})};
+ if (llvm::is_contained(names, str)) {
+ return names;
+ }
+ }
+ }
+ return {str};
+}
+
void WarnOnDeferredLengthCharacterScalar(SemanticsContext &context,
const SomeExpr *expr, parser::CharBlock at, const char *what) {
if (context.languageFeatures().ShouldWarn(
|
ProgramTree instances are created as the value of a local variable in the Pre(const parser::ProgramUnit &) member function in name resolution. But references to these ProgramTree instances can persist in SubprogramNameDetails symbol table entries that might survive that function call's lifetime, and lead to trouble later when (e.g.) expression semantics needs to deal with a possible forward reference in a function reference in an expression being processed later in expression checking. So put those ProgramTree instances into a longer-lived linked list within the SemanticsContext. Might fix some weird crashes reported on big-endian targets (AIX & Solaris).
klausler
force-pushed
the
fix-program-tree
branch
from
df6219c
to
28d5082
Compare
|
This fixes a crash I was seeing when running |
|
It fixes the ICE in |
That's a big relief. Let's wait to hear from @rorth about Solaris. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
ProgramTree instances are created as the value of a local variable in the Pre(const parser::ProgramUnit &) member function in name resolution. But references to these ProgramTree instances can persist in SubprogramNameDetails symbol table entries that might survive that function call's lifetime, and lead to trouble later when (e.g.) expression semantics needs to deal with a possible forward reference in a function reference in an expression being processed later in expression checking.
So put those ProgramTree instances into a longer-lived linked list within the SemanticsContext.
Might fix some weird crashes reported on big-endian targets (AIX & Solaris).