[flang][Semantics] Introduce `-Wpass-global-variable` warning #160324

foxtran · 2025-09-23T15:17:26Z

In this PR, I've implemented detection of passing global variables defined in common blocks or/and modules in a bit naive way.

The main reason of implementing this that we have a lot of bugs with ifx compiler due extensively usage of passing all possible variables from common blocks to subroutines.

Sometimes, Fortran programs actively utilises passing of global variables in a valid context, especially for stack memory allocators (like in GAMESS(US) or MRCC or Gaussian). For such cases, I've excluded those warning:

common block has only one variable of shape [ 1 ]
variable of shape [ 1 ] in module has attribute allocatable, pointer or volatile
variable in module has attribute parameter or private

To reduce warnings in other codebases, extra limitations are also added:

variable has derived type (it is quite easy to refactor but without it xtb has some amount of warnings from singletons)
variable is passed to intrinsic
variable is passed to pure function

See allowed cases in tests. However, some warning happens in already written tests.

github-actions · 2025-09-23T15:21:23Z

✅ With the latest revision this PR passed the C/C++ code formatter.

foxtran · 2025-09-23T15:29:03Z

flang/lib/Semantics/check-call.cpp

+            if (const ArraySpec *dims{base->GetShape()};
+                dims && dims->IsExplicitShape()) {
+              if (!((*dims)[0].lbound().GetExplicit() == (*dims)[0].ubound().GetExplicit())) {
+                  warn |= true;
+              }
+            }


@klausler, do you have an idea why base->GetShape() returns nullptr? It happens for variables defined in modules, but not for variables defined in common blocks.

llvm-project/flang/lib/Semantics/symbol.cpp

Line 453 in d79036c

const ArraySpec *Symbol::GetShape() const {

This could be found by searching for GetShape path:flang path:symbol within the repo. I use path:symbol because there were too many entries when I searched without it, and you were calling GetShape which seems to be a method of Symbols.

Based on the code, it returns nullptr when the symbol doesn't refer to a "object entity". I think this corresponds to "data entity" in the fortran '23 standard 5.4.3, which roughly corresponds to the notion of a value in general programming language theory (I think it specifically refers to the reference to a value). Not sure how familiar you are with fortran, but Fortran is weird in that the type system (and thus values) doesn't actually cover all "entities" that can be referenced in the syntax of the language. For instance functions can be referenced via symbols, but are not actually values (data entities) only their results are.

klausler · 2025-09-23T15:41:36Z

I don't understand what you're trying to do and I don't really have time to figure it out.

akuhlens · 2025-09-23T16:29:19Z

@foxtran You can direct your questions to me. I am not as knowledgable as @klausler.

I am a little worried about this warning in general. It is perfectly legit and indeed expected to pass global values to procedure calls. I think this would be a very noisy warning.

akuhlens · 2025-09-23T16:22:11Z

flang/lib/Support/Fortran-features.cpp

  warnUsage_.set(UsageWarning::HostAssociatedIntentOutInSpecExpr);
  warnUsage_.set(UsageWarning::NonVolatilePointerToVolatile);
  warnUsage_.set(UsageWarning::RealConstantWidening);
+  warnUsage_.set(UsageWarning::PassGlobalVariable);


This should definitely not be a warning enabled by default, and I probably wouldn't even want this one to fire with -Wall, which leads to questions about how this should be implemented.

I think it would be nice to enable this warning with -Wall. It helps to avoid global states. By default, I disabled this warning.

akuhlens · 2025-09-23T16:55:52Z

flang/lib/Semantics/check-call.cpp

+            if (const ArraySpec *dims{base->GetShape()};
+                dims && dims->IsExplicitShape()) {
+              if (!((*dims)[0].lbound().GetExplicit() == (*dims)[0].ubound().GetExplicit())) {
+                  warn |= true;
+              }
+            }


llvm-project/flang/lib/Semantics/symbol.cpp

Line 453 in d79036c

const ArraySpec *Symbol::GetShape() const {

This could be found by searching for GetShape path:flang path:symbol within the repo. I use path:symbol because there were too many entries when I searched without it, and you were calling GetShape which seems to be a method of Symbols.

Based on the code, it returns nullptr when the symbol doesn't refer to a "object entity". I think this corresponds to "data entity" in the fortran '23 standard 5.4.3, which roughly corresponds to the notion of a value in general programming language theory (I think it specifically refers to the reference to a value). Not sure how familiar you are with fortran, but Fortran is weird in that the type system (and thus values) doesn't actually cover all "entities" that can be referenced in the syntax of the language. For instance functions can be referenced via symbols, but are not actually values (data entities) only their results are.

foxtran · 2025-09-23T17:21:51Z

I think this would be a very noisy warning.

In fact, it depends on code. For MRCC, the most problematic code is full of these warnings (and not only this warning), while where I do not see this warning, we also do not have a bug with ifx. I see only one disadvantage related to output unit, which we keep in common block and use everywhere.

For GAMESS(US), the compilation log is full of incompatible-implicit-interfaces, known-bad-implicit-interface, and pass-global-variable which is dominant, unfortunately. However, for some of files, there is no pass-global-variable warnings :-)

foxtran · 2025-10-07T20:19:27Z

@akuhlens,

Based on the code, it returns nullptr when the symbol doesn't refer to a "object entity".

Yep, that was a problem. I had "use entity" which inside is an "object entity". But for "use entity", GetShape could not provide GetShape (while it can, actually). Now, it is fixed and my problem is gone.

foxtran · 2025-10-07T20:39:44Z

It is perfectly legit and indeed expected to pass global values to procedure calls.

For some cases, it may lead to undefined behaviour. For example, for the following pieces of code (it violates Fortran standard):

main.f90:

program main
  implicit none
  common /mem/ imem
  integer :: imem
  imem = 0
  call test(imem)
end program main

subroutine update_imem
  implicit none
  common /mem/ imem
  integer :: imem
  imem = 30
end subroutine update_imem

test.f90:

subroutine test(imem)
  integer :: imem
  print *, imem
  call update_imem()
  print *, imem
end subroutine test

No opts:

$ flang test.f90 main.f90 -O0 -o a.exe && ./a.exe
 0
 30

With -O3:

$ flang test.f90 main.f90 -O3 -o a.exe && ./a.exe
 0
 0

With this PR, one gets:

$ flang -Wpass-global-variable -fsyntax-only *.f90
./main.f90:6:13: warning: Passing global variable 'imem' from COMMON 'mem' as function argument [-Wpass-global-variable]
    call test(imem)
              ^^^^

I think this would be a very noisy warning.

Now, it is not. I have reduced some possible invalid cases that, unfortunately, used a lot, but in general they should not give a problem. And in some valid contexts it also does not give warnings. I have played with GAMESS(US), MRCC and xtb and it does not give a lot of warnings.

foxtran commented Sep 23, 2025

View reviewed changes

akuhlens requested changes Sep 23, 2025

View reviewed changes

foxtran force-pushed the warning-pass-global-variable branch from d056747 to 6192c00 Compare October 7, 2025 20:15

foxtran added 13 commits October 7, 2025 22:42

Implement -Wpass-global-variable flag for flang

a1f89bb

Add test for -Wpass-global-variable

6dbb895

Initial implementation of CheckPassGlobalVariable

6d0784c

Comment checks of single-element arrays

0b79c37

GetShape for variables loaded from modules

82582e6

Allow passing private

ec0dfc8

Allow passing derived types

256a11e

Allow passing to intrinsic

1f30465

Allow passing to pure

d0547e4

Allow passing to VALUE dummy argument

b0dc77f

Add second set of tests

f9280db

Disable default PassGlobalVariable

933b506

Do not raise PassGlobalVariable warning in some tests

39a63c8

foxtran force-pushed the warning-pass-global-variable branch from 6192c00 to 39a63c8 Compare October 7, 2025 20:42

foxtran added 2 commits October 8, 2025 09:30

Fast exit if actualFirstSymbol is not constructed

c7fc7bd

Avoid |= for warn; give the reason instead

38bc57c

foxtran marked this pull request as ready for review October 8, 2025 07:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[flang][Semantics] Introduce `-Wpass-global-variable` warning #160324

[flang][Semantics] Introduce `-Wpass-global-variable` warning #160324

foxtran commented Sep 23, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Sep 23, 2025 •

edited

Loading

Uh oh!

foxtran Sep 23, 2025

Uh oh!

akuhlens Sep 23, 2025

Uh oh!

klausler commented Sep 23, 2025

Uh oh!

akuhlens commented Sep 23, 2025

Uh oh!

akuhlens Sep 23, 2025

Uh oh!

foxtran Oct 7, 2025 •

edited

Loading

Uh oh!

akuhlens Sep 23, 2025

Uh oh!

foxtran commented Sep 23, 2025

Uh oh!

foxtran commented Oct 7, 2025 •

edited

Loading

Uh oh!

foxtran commented Oct 7, 2025 •

edited

Loading

Uh oh!

Uh oh!

[flang][Semantics] Introduce -Wpass-global-variable warning #160324

Are you sure you want to change the base?

[flang][Semantics] Introduce -Wpass-global-variable warning #160324

Conversation

foxtran commented Sep 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Sep 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

foxtran Sep 23, 2025

Choose a reason for hiding this comment

Uh oh!

akuhlens Sep 23, 2025

Choose a reason for hiding this comment

Uh oh!

klausler commented Sep 23, 2025

Uh oh!

akuhlens commented Sep 23, 2025

Uh oh!

akuhlens Sep 23, 2025

Choose a reason for hiding this comment

Uh oh!

foxtran Oct 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

akuhlens Sep 23, 2025

Choose a reason for hiding this comment

Uh oh!

foxtran commented Sep 23, 2025

Uh oh!

foxtran commented Oct 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

foxtran commented Oct 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

[flang][Semantics] Introduce `-Wpass-global-variable` warning #160324

[flang][Semantics] Introduce `-Wpass-global-variable` warning #160324

foxtran commented Sep 23, 2025 •

edited

Loading

github-actions bot commented Sep 23, 2025 •

edited

Loading

foxtran Oct 7, 2025 •

edited

Loading

foxtran commented Oct 7, 2025 •

edited

Loading

foxtran commented Oct 7, 2025 •

edited

Loading