[Wien] Possible bugs from use of uninitialized variables

Pavel Ondračka pavel.ondracka at email.cz
Mon Oct 14 17:43:13 CEST 2013


Laurence Marks píše v Po 14. 10. 2013 v 08:46 -0500:
> I was travelling so did not have access to the source. You are correct
> and it is technically a bug which apparently neither -ftrapuv nor
> gfortran picked up. I have added
> 
>                 ROKMIX = 0.D0 ; ROKSC = 0.D0 ; ROKVL = 0.D0 ; ROKOLD = 0.D0
This is probably just a typo on your side, however ROKMIX = 0.D0 will
initialize just the real part... ROKMIX = (0.D0, 0.D0) is needed (at
lest on gfortran and xlf90) 
>                 KVECVL = 0 ; KVECSC = 0 ; KVCOLD = 0
> 
> after the allocation, and at some stage will add comparable to other
> allocations.
> 
> That said, it should have no effect on the performance of mixer, and
> if it does then there is a serious bug somewhere else. The test on
> line 1504 of your version is just for some output of how the PW's are
> changing for monitoring as Peter has noticed that converging these can
> sometimes be tricky. There are a few other similar traps just for
> cleaning things up.
I didn't saw any actual problems except the crash in f7splt, I was just
being extra cautious, since I imagine the AIX 5.3 build doesn't see much
testing.
> 
> Can you please send the lines for LimitDMIX8n.F in your version since
> it is probably different from mine and I do not see anything relevant
> at line 135.
LimitDMIX8n.F:135    PM1=min(1.D0,PM1)
attached is the stock LimitDMIX8n.F from WIEN2k_13.1 version

> N.B., concerning valgrind and mixer (for instance) not deallocating
> everything. There is a routine called "cleaner.f" which is in
> SRC_mixer for this purpose. However, with a multiply branching code it
> is very, very, very hard to trap everything and I have ended up hoping
> that the compiler does what it is supposed to. It is also very hard to
> protect for all possible systems and compilers particularly if I have
> no access to them. And.....mpi is much worse.
It wasn't actually leaking that much at all, just the small leak in
lapw1 (not counting the allocated memory still accessible on exit)

Basically even though my platform of interest is AIX 5.3, the testing
with valgrind was done on linux with gfortran, because valgrind isn't
working on AIX. As I've said before, my main motivation for this vagrind
exercise was just that I was really concerned that clearly invalid code
wasn't crashing at all neither with ifort (because it is initialized to
0 defaultly) nor with gfortran (it was probably optimized away, because
with -O0 it crash in the same way) and I was wondering if there are more
similar cases that are not severe enough to result in crash but rather
some unwanted behavior.

Also all the testing was done just with the TiC sample, so I probably
haven't tested that many codepaths at all...

Anyway thanks for looking into this and let me know if you need any more
info.

> On Thu, Oct 10, 2013 at 10:47 AM, Pavel Ondračka
> <pavel.ondracka at email.cz> wrote:
> > Laurence Marks píše v Čt 10. 10. 2013 v 09:40 -0500:
> >> Sorry, but I agree with Peter & I am 99.9% certain that this is not a
> >> bug in the mixer. The way to test this is (with ifort) to use
> >> -ftrapuv ;
> > That is not true. According to ifort docs:
> > -ftrapuv
> > The option sets any uninitialized local variables that are allocated on
> > the _stack_ to a value that is typically interpreted as a very large
> > integer or an invalid address.
> >
> > However the ROKMIX is allocated on the _heap_
> >
> > Just try the test I've described earlier, set the imaginary part by hand
> > right after allocation and check the value right before the if check
> > either with debugger or just write it to stdout and you will see the
> > value doesn't change.
> >>  the arrays are not set in mixer.F but elsewhere within some
> >> complicated subroutines. This flag forces a fault if an undefined
> >> variable is used.
> >
> >
> >> ---------------------------
> >> Professor Laurence Marks
> >> Department of Materials Science and Engineering
> >> Northwestern University
> >> www.numis.northwestern.edu 1-847-491-3996
> >> "Research is to see what everybody else has seen, and to think what
> >> nobody else has thought"
> >> Albert Szent-Gyorgi
> >>
> >>
> >> On Oct 10, 2013 7:23 AM, "Pavel Ondračka" <pavel.ondracka at email.cz>
> >> wrote:
> >>         Peter Blaha píše v Čt 10. 10. 2013 v 12:22 +0200:
> >>         > I'm not very familiar with valgrid, but all those reports
> >>         seem not
> >>         > relevant to me.
> >>         > It seems to complain about all allocated arrays, which are
> >>         not set to
> >>         > zero globally. But this does NOT mean that one uses
> >>         uninitialized
> >>         > variables or assumes that the compiler sets them to zero.
> >>         Valgrind doesn't complain about uninitialized variables in
> >>         general. Just
> >>         when they are used in such way that they can alter the flow of
> >>         the
> >>         program or generally alter the externally-visible behavior.
> >>
> >>         I'll take the problem from mixer as an example, because
> >>         contrary to the
> >>         rest of the problems, here I'm 99% sure it is a real bug.
> >>
> >>         The complex array ROKMIX in mixer.F, is only partially
> >>         initialized. The
> >>         real part of the complex number is initialized somewhere
> >>         (don't know
> >>         exactly where, since I haven't tracked it that much), however
> >>         the
> >>         imaginary part is not. You can check this by setting the
> >>         imaginary part
> >>         to some specific value right after the allocation, and then
> >>         set a
> >>         breakpoint or print its value at line
> >>         mixer.F:1504  if(abs(ROKMIX(J,I)).gt.1D-12)then
> >>         and you will see, it has the same value as set before, so it
> >>         wasn't
> >>         written anywhere.
> >>         And now basically if real part of ROKMIX(J,I) is lower than
> >>         1D-12 we
> >>         would randomly fail or pass the if test, depending on what
> >>         stuff is
> >>         written in the corresponding uninitialized memory.
> >>
> >>         >
> >>         > PS: I don't have a variable "WS" in dergl.f
> >>         > WS=0.0D0 in dergl.f
> >>         My mistake, this should have been RW
> >>         >
> >>         > We can verify this also with ifort by setting -C in the
> >>         flags, and in
> >>         > fact doing so would result in an error in f7splt due to the
> >>         mistake you
> >>         > mentioned before.
> >>         >
> >>         > So the only problem was in f7splt.f and that should be fixed
> >>         as
> >>         > previously mentioned.
> >>         Well, this is not what valgrind does. Valgrind doesn't do
> >>         static code
> >>         analysis as compilers do during compilation, but it replaces
> >>         calls to
> >>         malloc and similar at runtime with his own versions and tracks
> >>         the used
> >>         memory at runtime.
> >>
> >>         To be more specific, I'll try to explain on my previous
> >>         example why I
> >>         think it can't be discovered by the ifort checking.
> >>         Although ifort probably can track, that imaginary part of
> >>         ROKMIX(J,I) is
> >>         uninitialized at that point, it has no way of knowing if
> >>         return value of
> >>         abs(ROKMIX(J,I)) actually depends on the uninitialized value
> >>         or not,
> >>         because abs function has not been linked at that point.
> >>
> >>         >
> >>         > (PS: I was wrong with my first reply that you need ISPLIT=15
> >>         (this
> >>         > relates to an older version). The present if statement is
> >>         intentional,
> >>         > but still, no result from f7splt will be used (except for
> >>         ISPLIT=15).
> >>         Thanks, this explains it.
> >>
> >>         I'm very grateful and I appreciate it very much, that you are
> >>         looking
> >>         into this, even though this is not a problem at your platform.
> >>         Thanks again.
> >>
> >>         _______________________________________________
> >>         Wien mailing list
> >>         Wien at zeus.theochem.tuwien.ac.at
> >>         http://zeus.theochem.tuwien.ac.at/mailman/listinfo/wien
> >>         SEARCH the MAILING-LIST at:
> >>          http://www.mail-archive.com/wien@zeus.theochem.tuwien.ac.at/index.html
> >> _______________________________________________
> >> Wien mailing list
> >> Wien at zeus.theochem.tuwien.ac.at
> >> http://zeus.theochem.tuwien.ac.at/mailman/listinfo/wien
> >> SEARCH the MAILING-LIST at:  http://www.mail-archive.com/wien@zeus.theochem.tuwien.ac.at/index.html
> >
> >
> > _______________________________________________
> > Wien mailing list
> > Wien at zeus.theochem.tuwien.ac.at
> > http://zeus.theochem.tuwien.ac.at/mailman/listinfo/wien
> > SEARCH the MAILING-LIST at:  http://www.mail-archive.com/wien@zeus.theochem.tuwien.ac.at/index.html
> 
> 
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: LimitDMIX8n.F
Type: text/x-fortran
Size: 11809 bytes
Desc: not available
URL: <http://zeus.theochem.tuwien.ac.at/pipermail/wien/attachments/20131014/ccc6cea9/attachment.f>


More information about the Wien mailing list