Goddard-Fortran-Ecosystem / yaFyaml

Yet Another Fortran YAML
Apache License 2.0
13 stars 8 forks source link

Bug with passing subsectioned string in the configuration "at" function #25

Open bena-nasa opened 3 years ago

bena-nasa commented 3 years ago

This is with yafmal version 0.4.1 and intel fortran 19.1.3.304 I have this simple program. I'm finding that when trying to retrieve a key I have to trim the string passed to the %at where as if I just take a slice of of the string without the trim it fails despite the trim slice and the slice without the trim seemingly the same, at least according to the result of the print here. Not clear why a trim is needed but seems like a bug in the compiler perhaps?

program yaml_test
use yaFyaml
implicit none
type(Configuration) :: subcfg, config
type(Parser) :: p
type(Filestream) :: fstream
integer :: status,idx
logical :: ll
character(:), allocatable :: key
p = parser('core')
fstream = FileStream("bc_merra2_extdata.yaml")
config = p%load(fstream)
call fstream%close()
key="rules%"//"BC_BIOMASS"
idx = index(key,"%")
! the next 3 lines all evaluate to true when run
write(*,*)"BC_BIOMASS"==key(idx+1:)
write(*,*)"BC_BIOMASS"==trim(key(idx+1:))
write(*,*)key(idx+1:)==trim(key(idx+1:))
subcfg = config%at(trim(key(:idx-1)),trim(key(idx+1:)),rc=status) ! works
subcfg = config%at(key(:idx-1),key(idx+1:),rc=status) ! segfaults
end program yaml_test

the input file:

data_sets:
  qfed_bc: {tmpl: "ExtData/PIESA/sfc/QFED/v2.4r6/Y%y4/M%m2/qfed2.emis_bc.005.%y4%m2%d2.nc4"}
  ante_bc: {tmpl: "ExtData/AeroCom/sfc/AeroCom.noship_BC_src.sfc.x360_y181_t44.19780703_12z_20210703_12z.nc"}
  ship_bc: {tmpl: "ExtData/MERRA2/sfc/edgar-v41.emis_bc.navigation.x360_y181_t47.19750703T12z_20210703T00z.nc4"}

rules:
  BC_BIOMASS: {file_key: qfed_bc, var: biomass, regrid: CONSERVE, upd_ref_time: "0" ,upd_freq: PT24H, upd_offset: PT12H}
  BC_BIOFUEL: {file_key: "/dev/null"}
  BC_ANTEBC1: {file_key: ante_bc, var: antebc1, regrid: CONSERVE, upd_ref_time: "0" ,upd_freq: PT24H, upd_offset: PT12H}
  BC_ANTEBC2: {file_key: "/dev/null"}
  BC_SHIP: {file_key: ship_bc, var: bc_ship, regrid: CONSERVE, upd_ref_time: "0" ,upd_freq: PT24H, upd_offset: PT12H}
  BC_AVIATION_LTO: {file_key: "/dev/null"}
  BC_AVIATION_CDS: {file_key: "/dev/null"}
  BC_AVIATION_CRS: {file_key: "/dev/null"}

and the traceback

 T
 T
 T
forrtl: severe (174): SIGSEGV, segmentation fault occurred
Image              PC                Routine            Line        Source
yaml_test.x        0000000000561C5A  Unknown               Unknown  Unknown
libpthread-2.22.s  00002AAAAAFDCC10  Unknown               Unknown  Unknown
yaml_test.x        0000000000467DAC  fy_configurationg         349  Configuration.F90
yaml_test.x        00000000004678AF  fy_configurationg         336  Configuration.F90
yaml_test.x        00000000004656BE  fy_configuration_         282  Configuration.F90
yaml_test.x        0000000000468A67  fy_configuration_         380  Configuration.F90
yaml_test.x        0000000000469C0D  fy_configuration_         428  Configuration.F90
yaml_test.x        0000000000407006  MAIN__                     27  yaml_test.F90
yaml_test.x        0000000000403152  Unknown               Unknown  Unknown
libc-2.22.so       00002AAAAB40D725  __libc_start_main     Unknown  Unknown
yaml_test.x        0000000000403069  Unknown               Unknown  Unknown

Note line 27 is the 2nd to last line of the program, the 2nd config%at call.

tclune commented 3 years ago

A couple of data points. (1) The code compiles and runs without error with NAG 7.0 (7037) with full error checking. and (2) the code crashes under OS X with intel 19.1.3 in pretty much the same ways as described above (which was presumably with intel on linux).

tclune commented 3 years ago

And it is not the trim per se that is helping. This variant also works

   a = key(:idx-1)
   b = key(idx+1:)
   subcfg = config%at(a, b, rc=status)
tclune commented 3 years ago

Well, it does not crash, but the status is coming back as a nonzero value (looks uninitialized).

tclune commented 3 years ago

OK - the last bit was due to a missing __RETURN__ macro in the topmost function invocation. I'll push a bugfix for that.

I'm fairly convinced that the remaining issue is a defect in the compiler. A standalone reproducer is important - but tedious to implement due to the gFTL component. My temptation is to not bother whittling it down and just shipping it off to Intel.

I think the corner case that is tripping them us is that we are passing a section of a string actual to an unlimited polymorphic dummy. Compilers in general have struggled with passing strings to unlimited polymorphic dummies - presumably due to the need to capture the len parameter. (Indeed, gfortran seems to have some terrible memory corruption still when doing things like this.)

Quite likely you can avoid the issue entirely with Intel by first putting the slice into a separate variable, or with the trim() as you've discovered. But since I happen to know what you want the final code to work with GFortran, you may need to actually wrap the arguments with String(<arg>) which is my workaround for gfortran.

tclune commented 3 years ago

Well, for once a small reproducer was easy to create. I'll submit to Intel shortly:

module bar
   implicit none

contains

   subroutine foo(arg1, arg2, arg3)
      class(*), optional, intent(in) :: arg1
      class(*), optional, intent(in) :: arg2
      class(*), optional, intent(in) :: arg3

      if (present(arg1)) call one(arg1)
      if (present(arg2)) call one(arg2)
      if (present(arg3)) call one(arg3)

   contains

      subroutine one(arg)
         class(*), intent(in) :: arg

         select type (arg)
         type is (character(*))
            print*,'yes'
         class default
            print*,'no'
         end select
      end subroutine one

   end subroutine foo

end module bar

program reproducer
   use bar
   implicit none
   character(:), allocatable :: string

   string = 'a%b'
   call foo(string(:1),string(2:))
!!$   call foo(trim(string(:1)),trim(string(2:)))
end program reproducer
tclune commented 3 years ago

Annoyingly - this still breaks with yaFyaml 1.0 and the latest intel compiler.

tclune commented 3 years ago

I am going to close this ticket. The problem is with the compiler and the workaround must be done in client code. Unfortunate, but will never be fixed (directly) within yaFyaml.