Skip to content

[Flang][OpenMP] Prevent ICE for certain constructs in unnamed programs #73938

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 22, 2024
Merged

[Flang][OpenMP] Prevent ICE for certain constructs in unnamed programs #73938

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion flang/lib/Semantics/resolve-directives.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
template <typename T>
static Fortran::semantics::Scope *GetScope(
Fortran::semantics::SemanticsContext &context, const T &x) {
std::optional<Fortran::parser::CharBlock> source{GetSource(x)};
std::optional<Fortran::parser::CharBlock> source{GetLastSource(x)};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this fix the bug? Wouldn't it still fail when presented with a list or vector or tuple of parse tree nodes with a single element that would fail without this change?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function is only called for MainProgram, FunctionSubprogram, SubroutineSubprogram and BlockData (see ResolveOmpTopLevelParts in the same file). It tries to get the scope associated to these nodes and crashes while doing so in certain situations when it is a nameless MainProgram. As far as I can tell, the issue is that, when the optional ProgramStmt is not present (nameless program) the call to GetSource will return the source object of the first sourced PFT node that appears in the program, which may not have a scope associated.

In my opinion, it is incorrect to return the scope associated to any inner constructs when querying the program itself. This change makes it so the END statement of the program/function/subroutine/block, which is always present, is picked up every time, preventing this problem and stopping the crash from happening.

Maybe it'd be good to possibly restrict the GetScope function so it fails to compile if we pass any unexpected PFT node type, or just add a comment to describe its intended use. I'll gladly do it if that helps, or if there are any other suggestions as to how to deal with this case.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this sound like a reasonable solution? Thanks!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ping @klausler, I'd appreciate it if you could let me know whether my previous explanation addresses your concerns. Thanks for your time!

return source ? &context.FindScope(*source) : nullptr;
}

Expand Down
7 changes: 7 additions & 0 deletions flang/test/Semantics/OpenMP/struct.f90
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
! RUN: %python %S/../test_errors.py %s %flang_fc1 -fopenmp
! Check OpenMP compatibility with the DEC STRUCTURE extension

structure /s/
end structure

end