From af3c7241df9e274d9944f41e8c174b7580c656dd Mon Sep 17 00:00:00 2001
From: Peter Klausler <pklausler@nvidia.com>
Date: Thu, 11 May 2023 14:28:58 -0700
Subject: [flang] Catch (and fix) attempts to create an invalid source range
 for a Scope

When Scope::AddSourceRange() is called to extend the scope's source range to
include another snippet from a cooked character stream, add a check to ensure
that the new range is part of the same cooked character stream as the rest
of the scope.

And fix the bug that was causing such invalid source ranges to be created:
a submodule's Scope is a children of its parent's in the Scope tree, but
it may or may not be part of the same source file, and it is certainly
not enclosed in the parent's source range.  So don't propagate Scope
source range expansion from a submodule to its parent.

Differential Revision: https://reviews.llvm.org/D150714
---
 flang/include/flang/Semantics/scope.h |  3 ++-
 flang/lib/Semantics/scope.cpp         | 49 ++++++++++++++++++++++++++++++++---
 2 files changed, 47 insertions(+), 5 deletions(-)

(limited to 'flang')

diff --git a/flang/include/flang/Semantics/scope.h b/flang/include/flang/Semantics/scope.h
index 0c5beb6492c6..48109c9de041 100644
--- a/flang/include/flang/Semantics/scope.h
+++ b/flang/include/flang/Semantics/scope.h
@@ -247,7 +247,7 @@ public:
 
   // The range of the source of this and nested scopes.
   const parser::CharBlock &sourceRange() const { return sourceRange_; }
-  void AddSourceRange(const parser::CharBlock &);
+  void AddSourceRange(parser::CharBlock);
   // Find the smallest scope under this one that contains source
   const Scope *FindScope(parser::CharBlock) const;
   Scope *FindScope(parser::CharBlock);
@@ -276,6 +276,7 @@ private:
   std::size_t size_{0}; // size in bytes
   std::optional<std::size_t> alignment_; // required alignment in bytes
   parser::CharBlock sourceRange_;
+  const parser::CookedSource *cookedSource_{nullptr};
   Symbol *const symbol_; // if not null, symbol_->scope() == this
   std::list<Scope> children_;
   mapType symbols_;
diff --git a/flang/lib/Semantics/scope.cpp b/flang/lib/Semantics/scope.cpp
index bcffbd331280..7570714732e1 100644
--- a/flang/lib/Semantics/scope.cpp
+++ b/flang/lib/Semantics/scope.cpp
@@ -8,6 +8,7 @@
 
 #include "flang/Semantics/scope.h"
 #include "flang/Parser/characters.h"
+#include "flang/Semantics/semantics.h"
 #include "flang/Semantics/symbol.h"
 #include "flang/Semantics/type.h"
 #include "llvm/Support/raw_ostream.h"
@@ -309,7 +310,7 @@ const Scope *Scope::FindScope(parser::CharBlock source) const {
 
 Scope *Scope::FindScope(parser::CharBlock source) {
   bool isContained{sourceRange_.Contains(source)};
-  if (!isContained && !IsTopLevel() && !IsModuleFile()) {
+  if (!isContained && !IsTopLevel() && kind_ != Kind::Module) {
     return nullptr;
   }
   for (auto &child : children_) {
@@ -320,9 +321,49 @@ Scope *Scope::FindScope(parser::CharBlock source) {
   return isContained && !IsTopLevel() ? this : nullptr;
 }
 
-void Scope::AddSourceRange(const parser::CharBlock &source) {
-  for (auto *scope{this}; !scope->IsGlobal(); scope = &scope->parent()) {
-    scope->sourceRange_.ExtendToCover(source);
+void Scope::AddSourceRange(parser::CharBlock source) {
+  if (source.empty()) {
+    return;
+  }
+  const parser::AllCookedSources &allCookedSources{context_.allCookedSources()};
+  const parser::CookedSource *cooked{allCookedSources.Find(source)};
+  if (!cooked) {
+    CHECK(context_.IsTempName(source.ToString()));
+    return;
+  }
+  for (auto *scope{this}; !scope->IsTopLevel(); scope = &scope->parent()) {
+    CHECK(scope->sourceRange_.empty() == (scope->cookedSource_ == nullptr));
+    if (!scope->cookedSource_) {
+      scope->cookedSource_ = cooked;
+      scope->sourceRange_ = source;
+    } else if (scope->cookedSource_ == cooked) {
+      scope->sourceRange_.ExtendToCover(source);
+    } else {
+      // There's a bug that will be hard to fix; crash informatively
+      const parser::AllSources &allSources{allCookedSources.allSources()};
+      const auto describe{[&](parser::CharBlock src) {
+        if (auto range{allCookedSources.GetProvenanceRange(src)}) {
+          std::size_t offset;
+          if (const parser::SourceFile *
+              file{allSources.GetSourceFile(range->start(), &offset)}) {
+            return "'"s + file->path() + "' at " + std::to_string(offset) +
+                " for " + std::to_string(range->size());
+          } else {
+            return "(GetSourceFile failed)"s;
+          }
+        } else {
+          return "(GetProvenanceRange failed)"s;
+        }
+      }};
+      std::string scopeDesc{describe(scope->sourceRange_)};
+      std::string newDesc{describe(source)};
+      common::die("AddSourceRange would have combined ranges from distinct "
+                  "source files \"%s\" and \"%s\"",
+          scopeDesc.c_str(), newDesc.c_str());
+    }
+    if (scope->IsSubmodule()) {
+      break; // Submodules are child scopes but not contained ranges
+    }
   }
 }
 
-- 
cgit v1.2.1