Bug#925624: [Debichem-devel] Bug#925624: Bug#925624: abinit: ftbfs with GCC-9

[Michael Banck <mbanck@debian.org>]

Hi,

On Sat, Sep 07, 2019 at 10:50:34PM +0200, Michael Banck wrote:
> > Program received signal SIGSEGV: Segmentation fault - invalid memory reference.
> > 
> > Backtrace for this error:
> > #0  0x7f063149eb40 in ???
> > #1  0x7f063149dd75 in ???
> > #2  0x7f063115893f in ???
> > #3  0x5612b98bb243 in forces_
> > 	at /<<PKGBUILDDIR>>/src/67_common/forces.F90:462
> 
> So at that line we have:
> 
> | if (usefock==1 .and. associated(fock).and.fock%fock_common%optfor) then
> 
> and if I step through that function with gdb, usefock is 0 (while fock
> is pointing to 0x0).
> 
> If I comment out the above if block (plus two more pretty much identical
> ones in m_forstr.F90 and m_stress.F90) most of the tests run fine (I
> guess the other ones really do need fock).
> 
> I am a bit baffled why gfortran compiles this in such a way (at -g
> without any -O as well) that we get a segfault here. I now tried with
> FCFLAGS='-g -O2' and then the test suite runs fine, so reassigning to
> gfortran-9 for now, do you have an idea what's going on?
> 
> (I filed this one upstream as well:
> https://github.com/abinit/abinit/issues/25)

So it appears that fock%fock_common%optstr cannot be guaranteed not to
be evaluated after associated(fock) is false as the Fortran language
apparently does not define short-circuit beahviour, see
https://www.scivision.dev/fortran-short-circuit-logic/

That explains why it works fine at -O2 (and would as well at -O1 in
gfortran), but crashed in -O0. I wonder why it built fine at -O0 in
stable though, it might be that the gfortran behaviour has been changed
recently.

I guess the fix would be to have two ifs here, like:

| if (usefock==1 .and. associated(fock)) then
|   if (fock%fock_common%optfor) then
|     grtn(:,:)=grtn(:,:)+fock%fock_common%forces(:,:)
|   end if
| end if

I've uploaded abinit with FCFLAGS=-O2 now, should we close this bug and
leave it to the abinit maintainers to fix it properly, or do you want to
reassign it back?

Thanks to juliank for pointing me to the potential short-circuit logic
issue.


Michael