Potential segfault in gs2_main::set_initval_overrides_to_current_vals

[Nicolas Christen]

In gs2_main::set_initval_overrides_to_current_vals, it seems like a segmentation fault could happen when in_memory=.true. and force_maxwell_reinit=.false. ... The phi/apar/bpar arrays it wants to initialize are only allocated if force_maxwell_reinit=.true. in overrides::init_initial_values_overrides.

Could this perhaps be the segmentation fault Edmund was referring to in his warning message about force_maxwell_reinit ?

[David Dickinson]

The code in gs2_main looks like

    if(initval_ov%in_memory)then
      initval_ov%g=gnew
      if (.not. initval_ov%force_maxwell_reinit) then
        if(fphi.gt.0) then
          initval_ov%phi=phinew
        end if
        if(fapar.gt.0) then
          initval_ov%apar=aparnew
        end if
        if(fbpar.gt.0) then
          initval_ov%bpar=bparnew
        end if
      end if
else

so it seems it also only tries to set the override%phi etc. if override%force_maxwell_reinit is true so I think this should be ok (also if override%phi is allocatable then I think fortran would actually allow this asssignment essentially allocating on demand).

Is this the section of code you were concerned about?

On the topic of the warning message I believe this is saying that if you change from override%force_maxwell_reinit=false to override%force_maxwell_reinit=true then that chunk of code could cause a seg fault as you mention -- I think it would probably be ok if these arrays are allocatable @ZedThree probably has a better idea of if this is true or not. I'd be keen to try to avoid showing this warning in every simulation as it can be distracting for new users, for example.

[Nicolas Christen]

Hi David, thanks for the swift response.

Yes, this is the bit of code I am concerned about. It tries to initialize those variables if force_maxwell_reinit is FALSE. If you look at overrides::init_initial_values_overrides, they are only allocated if force_maxwell_reinit is TRUE.

As you say, this mistake in the code may be corrected with some sort of automatic allocation which I am not familiar with. But I think that this is what Edmund was referring to.

[David Dickinson]

I've finally managed to get around to looking at this in detail and I think you're correct -- essentially I think the logic is inverted in one place. I managed to get a segfault in the original code and a working system with inverted logic. The fix is in PR #105, which has just been merged so hopefully we can resolve this issue if you agree it has been addressed.

[Nicolas Christen]

Thanks David. Do I need to approve it by clicking somewhere ? Maybe the 'Resolve' button ?

[David Dickinson]

If you're happy with that this is fixed then clicking resolve should close the issue in a way that records that this was fixed (rather than some other outcome such as rejected).

[David Dickinson]

Fixed by PR #105