[flang] Fix crash when handling benign USE conflict #121977
Merged
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
When the same name is used for distinct derived types in two modules, and at least one of those modules also defines a generic interface of the same name, name resolution crashes when both modules are USE'd into the same scope. The crash is due to some pointers into the symbol table becoming invalid when a symbol is replaced with a UseErrorDetails; set them to null. Also allow for extending a UseErrorDetails in place rather than emitting a spurious error message. Fixes llvm#121718.
llvmbot
added
flang
|
@llvm/pr-subscribers-flang-semantics Author: Peter Klausler (klausler) ChangesWhen the same name is used for distinct derived types in two modules, and at least one of those modules also defines a generic interface of the same name, name resolution crashes when both modules are USE'd into the same scope. The crash is due to some pointers into the symbol table becoming invalid when a symbol is replaced with a UseErrorDetails; set them to null. Also allow for extending a UseErrorDetails in place rather than emitting a spurious error message. Fixes #121718. Full diff: https://github.com/llvm/llvm-project/pull/121977.diff 2 Files Affected:
diff --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp
index 122c0a2ebb646a..724f1b28078356 100644
--- a/flang/lib/Semantics/resolve-names.cpp
+++ b/flang/lib/Semantics/resolve-names.cpp
@@ -3162,6 +3162,10 @@ ModuleVisitor::SymbolRename ModuleVisitor::AddUse(
// Convert it to a UseError with this additional location.
static bool ConvertToUseError(
Symbol &symbol, const SourceName &location, const Scope &module) {
+ if (auto *ued{symbol.detailsIf<UseErrorDetails>()}) {
+ ued->add_occurrence(location, module);
+ return true;
+ }
const auto *useDetails{symbol.detailsIf<UseDetails>()};
if (!useDetails) {
if (auto *genericDetails{symbol.detailsIf<GenericDetails>()}) {
@@ -3319,6 +3323,8 @@ void ModuleVisitor::DoAddUse(SourceName location, SourceName localName,
combinedDerivedType = CreateLocalUseError();
} else {
ConvertToUseError(*localSymbol, location, *useModuleScope_);
+ localDerivedType = nullptr;
+ localGeneric = nullptr;
combinedDerivedType = localSymbol;
}
}
diff --git a/flang/test/Semantics/bug121718.f90 b/flang/test/Semantics/bug121718.f90
new file mode 100644
index 00000000000000..e99391f227d72e
--- /dev/null
+++ b/flang/test/Semantics/bug121718.f90
@@ -0,0 +1,31 @@
+! RUN: %flang_fc1 2>&1 | FileCheck %s --allow-empty
+! CHECK-NOT: error
+! Regression test simplified from LLVM bug 121718.
+! Ensure no crash and no spurious error message.
+module m1
+ type foo
+ integer x
+ end type
+ contains
+ subroutine test
+ print *, foo(123)
+ end
+end
+module m2
+ interface foo
+ procedure f
+ end interface
+ type foo
+ real x
+ end type
+ contains
+ complex function f(x)
+ complex, intent(in) :: x
+ f = x
+ end
+end
+program main
+ use m1
+ use m2
+ call test
+end
|
vdonaldson
approved these changes
Jan 8, 2025
| @@ -3319,6 +3323,8 @@ void ModuleVisitor::DoAddUse(SourceName location, SourceName localName, | |||
| combinedDerivedType = CreateLocalUseError(); | |||
| } else { | |||
| ConvertToUseError(*localSymbol, location, *useModuleScope_); | |||
| localDerivedType = nullptr; | |||
| localGeneric = nullptr; | |||
There was a problem hiding this comment.
If we got to the else clause, then localGeneric should already be null
There was a problem hiding this comment.
Agreed, but this makes it explicit that the localXXXX pointers into the contents of localSymbol are being invalidated.
eugeneepshteyn
approved these changes
Jan 8, 2025
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/153/builds/19156 Here is the relevant piece of the build log for the reference |
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.
When the same name is used for distinct derived types in two modules, and at least one of those modules also defines a generic interface of the same name, name resolution crashes when both modules are USE'd into the same scope. The crash is due to some pointers into the symbol table becoming invalid when a symbol is replaced with a UseErrorDetails; set them to null. Also allow for extending a UseErrorDetails in place rather than emitting a spurious error message.
Fixes #121718.