[flang-commits] [flang] af3c724 - [flang] Catch (and fix) attempts to create an invalid source range for a Scope

[Peter Klausler via flang-commits]

Author: Peter Klausler
Date: 2023-05-17T06:20:57-07:00
New Revision: af3c7241df9e274d9944f41e8c174b7580c656dd

URL: https://github.com/llvm/llvm-project/commit/af3c7241df9e274d9944f41e8c174b7580c656dd
DIFF: https://github.com/llvm/llvm-project/commit/af3c7241df9e274d9944f41e8c174b7580c656dd.diff

LOG: [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

Added: 
    

Modified: 
    flang/include/flang/Semantics/scope.h
    flang/lib/Semantics/scope.cpp

Removed: 
    


################################################################################
diff  --git a/flang/include/flang/Semantics/scope.h b/flang/include/flang/Semantics/scope.h
index 0c5beb6492c6d..48109c9de0412 100644
--- a/flang/include/flang/Semantics/scope.h
+++ b/flang/include/flang/Semantics/scope.h
@@ -247,7 +247,7 @@ class Scope {
 
   // 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 @@ class Scope {
   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 bcffbd331280c..7570714732e1a 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
+    }
   }
 }