flang-compiler / flang

Flang is a Fortran language front-end designed for integration with LLVM.
Other
799 stars 134 forks source link

BigDFT: surprising interactions within the code: interface nested into subroutine causes imported symbol appear undefined #1013

Closed pawosm-arm closed 2 years ago

pawosm-arm commented 3 years ago

This is similar to #367

In the following example:

module base
  implicit none
  integer, public :: a = 10
end module

module intermediate
  use base, only: a
  implicit none
  private
end module

program x
  use base
  implicit none

  interface
    subroutine sub2
      use intermediate
      implicit none
    end subroutine sub2
  end interface

  call sub

contains

subroutine sub
  use intermediate
  implicit none
  print *, a
end subroutine

end program

The symbol a imported by use base in program x became inaccessible to subroutine sub after the interface block was added. Namely, use intermediate line in the subroutine sub2 declared in this interface block causes a problem, removal of this particular line makes this problem go away. The intermediate module contains private clause which limits the visibility of the a variable defined in the base module, yet the base module is also imported directly by the use base line in the program x, hence the imported a variable should be visible to each of the contained subroutines. Note that the use intermediate line itself does not cause a problem, e.g. in the contained subroutine sub, the a variable is still visible after that line. The only context in which this suspicious interaction occurs is in the interface block which for some reason spills whatever happens in it outside of its context. Similar problem was somehow fixed some time ago (see issue #367), yet the code behind handling interfaces in flang1 seems to be so messy it still causes recurring problems.

michalpasztamobica commented 3 years ago

The root cause seems to be very far away from the symptoms in case of this bug.

Variable a is indeed not visible in line 29 - refsym() is not able to find it. That's because the a symbol from base module is marked hidden in adjust_symbol_accessibility(). It is marked hidden, because the use_tree is corrupt when trying to find an alias pointer for a - it contains the intermediate module twice, while it should contain base module from line 13 and intermediate module from line 28.

The issue seems to be in this line:

  if (!use_tree) {
    ...
  } else {
    ul->next = use_tree;
  }

If there was a use_tree - set it as next of the current use in the list. The current use is intermediate from line 28. What is the current use_tree? If line 18 is commented, the last use is use base from line 13. But if line 18 is uncommented - then this is the last use and it is use intermediate, so effectively we will have 2 intermediate modules added in the list.

So the root cause is that the algorithm simply takes the most recent use as ul->next. The use from line 18 is not even available in line 28.

michalpasztamobica commented 3 years ago

I have found an even more of a root case, I believe, which in fact leads to the use_tree not being loaded correctly. It's not just about the fact that it gets cleared. There are in total 4 different data structures that hold lists of "used" modules. I now come to think that the issue is with the usedb.

On every line with use modulename an open_module() is called which adds the module modulename to the usedb - a kind of database that holds the "used" modules.

Then when all modules must have already been added (for example - we hit an implicit none) an algorithm is started which fills in the use_tree - a connected graph holding the relations between really available modules (considering the modules being public/private etc.).

Finally - based on that graph modules are read in from .cmod files and all symbols which they contain and which should be available for a given scope are added.

The problem is about when the usedb and use_tree are cleared.

use_tree is cleared in apply_use_stmts() when: if (!gbl.currmod && gbl.internal <= 1) { init_use_tree(); }

usedb is unconditionally cleared right after it is put to use in apply_use_stmts(), so basically after uses were added to scope. The bug is - we should keep track of the scope for every added "used" module, rather than clearing the whole usedb.

A nasty workaround to fix the issue is:

--- a/tools/flang1/flang1exe/module.c
+++ b/tools/flang1/flang1exe/module.c
@@ -467,7 +467,7 @@ apply_use_stmts(void)
   }

   gbl.lineno = save_lineno;
-  if (usedb.base) {
+  if (usedb.base && gbl.lineno != 14 && gbl.lineno != 19) {
     /* usedb gets wiped clean in here */
   }

Compare that against commented code (definitions of modules same as in original reproducer):

12 program x
13   use base      ! adds base to usedb
14   implicit none ! adds base to use_tree; use_tree not cleared, but usedb cleared
15 
16   interface
17     subroutine sub2
18       use intermediate ! adds intermediate to usedb
19       implicit none    ! adds intermediate to use_tree; use_tree and usedb both cleared
20     end subroutine sub2
21   end interface
22 
23   call sub
24 
25 contains
26 
27 subroutine sub
28   use intermediate ! adds intermediate, but base is private in there
29   implicit none    ! base was added to usedb in line 13, but then cleared in lines 14 and 19
30   print *, a
31 end subroutine
32 
33 end program

By skipping the usedb reset when adding subsequent uses, I managed to get the code to build and run fine. The proper solution would be, in my opinion, to add proper scope tracking to usedb, so it doesn't clear all the entries at the end of use processing - just the ones whose scope has ended. For example - the base from line 13, should stay available all the way until line 33.