[PATCH][libgfortran] Fix uninitialized variable use in fallback_access

[Kyrill Tkachov]

Hi all,

I've been tracking down a bug in a Fortran program on a newlib target and it boils down to fallback_access doing something bad.
The unconditional calls to close cause havoc when open doesn't get called due to the short-circuiting in the if-statement above
because the fd is uninitialised. In my environment GCC ends up calling close on file descriptor 0, thus trying to close stdin.

This patch tightens up the calling so that close is called only when the corresponding open call succeeded.
With this my runtime failure disappears.

Bootstrapped and tested on aarch64-none-linux-gnu.
Though that doesn't exercise this call I hope it's an obviously correct change.

Ok for trunk and the branches?

Thanks,
Kyrill

2018-09-14 Kyrylo Tkachov <***@arm.com>

* io/unix.c (fallback_access): Avoid calling close on
uninitialized file descriptor.

[Paul Richard Thomas]

Hi Kyrill,

This indeed is an obvious change. OK for trunk and 7- and 8-branches.

Thanks for the patch

Paul

On 14 September 2018 at 09:06, Kyrill Tkachov

Post by Kyrill Tkachov
Hi all,
I've been tracking down a bug in a Fortran program on a newlib target and it
boils down to fallback_access doing something bad.
The unconditional calls to close cause havoc when open doesn't get called
due to the short-circuiting in the if-statement above
because the fd is uninitialised. In my environment GCC ends up calling close
on file descriptor 0, thus trying to close stdin.
This patch tightens up the calling so that close is called only when the
corresponding open call succeeded.
With this my runtime failure disappears.
Bootstrapped and tested on aarch64-none-linux-gnu.
Though that doesn't exercise this call I hope it's an obviously correct change.
Ok for trunk and the branches?
Thanks,
Kyrill
* io/unix.c (fallback_access): Avoid calling close on
uninitialized file descriptor.

[N.M. Maclaren]

Sorry about this, but searching in the usual places has found nothing
useful, and I am interested in gfortran's direction.

The kiddies do not like my OpenMP course, because it is all about what not
to do, but the problem is that even egregious data races do not show up in
test codes and there doesn't seem to be a usable testing tool. The best for
use with gfortran and gcc seems to be valgrind (helgrind or drd), but both
produce a hopeless number of false positives even in gcc 4.9.4 rebuilt with
no futexes and valgrind 3.13. Interestingly, gfortran is worse than gcc,
and my examples are functionally identical (I teach both languages at once).

This isn't just a teaching problem, as OpenMP experts will know. It's
horribly common for programs to fail, sometimes, after long running times,
on some systems - usually giving wrong answers with no diagnostic :-(

Has anyone any comments on this - except "you're stuffed" :-) I know the
difficulty involved, so am not expecting a good solution, nor thinking
about doing it, but is there even a partial solution to this for gfortran?


Regards,
Nick Maclaren.

[Paul Richard Thomas]

Hi Nick,

One remark that I have is that your gfortran is rather out of date. A
lot of bugs that show up with valgrind have been fixed since then. The
second thought is that you might contact Jakub Jelnik for questions
concerning OpenMP debugging.

Cheers

Paul

Post by N.M. Maclaren
Sorry about this, but searching in the usual places has found nothing
useful, and I am interested in gfortran's direction.
The kiddies do not like my OpenMP course, because it is all about what not
to do, but the problem is that even egregious data races do not show up in
test codes and there doesn't seem to be a usable testing tool. The best for
use with gfortran and gcc seems to be valgrind (helgrind or drd), but both
produce a hopeless number of false positives even in gcc 4.9.4 rebuilt with
no futexes and valgrind 3.13. Interestingly, gfortran is worse than gcc,
and my examples are functionally identical (I teach both languages at once).
This isn't just a teaching problem, as OpenMP experts will know. It's
horribly common for programs to fail, sometimes, after long running times,
on some systems - usually giving wrong answers with no diagnostic :-(
Has anyone any comments on this - except "you're stuffed" :-) I know the
difficulty involved, so am not expecting a good solution, nor thinking
about doing it, but is there even a partial solution to this for gfortran?
Regards,
Nick Maclaren.

[N.M. Maclaren]

Post by Paul Richard Thomas
One remark that I have is that your gfortran is rather out of date. A
lot of bugs that show up with valgrind have been fixed since then. The
second thought is that you might contact Jakub Jelnik for questions
concerning OpenMP debugging.

Thanks very much. I think that I am stuffed :-(

That's not the version I use, but one that was close to the version
referred to in the valgrind 3.13 release notes. I am trying to rebuild
4.3 (yes, really), which is the actual one referred to. I could certainly
rebuild the latest and greatest gfortran, but am not optimistic, if the
last version valgrind tested with was 4.3 :-(

I will try contacting him. I am specifically asking about tools for
automatic checking for race conditions - most of my course is about
writing OpenMP that actually works, and it goes into great length about
how hard it is to debug.

Regards,
Nick Maclaren.

[Thomas Koenig]

Post by N.M. Maclaren
I am specifically asking about tools for
automatic checking for race conditions - most of my course is about
writing OpenMP that actually works, and it goes into great length about
how hard it is to debug.

I have not debugged OpenMP myself, but for pthread - based code (the
async I/O stuff) valgrind has been invaluable. It helped Nicolas and
me to debug a one in 20000 occurrence of a deadlock...

Specifically, try to use

valgrind --tool=helgrind

and

valgrind --tool=drd

and watch the bugs come crawling out :-)

Regards

Thomas

[N.M. Maclaren]

Post by Paul Richard Thomas
One remark that I have is that your gfortran is rather out of date. A
lot of bugs that show up with valgrind have been fixed since then.

No better in 8.2. 4.3 won't build, and I don't have time to work out how
to bypass the problems. Unless I have made a stupid error, it's hopeless.

Regards,
Nick Maclaren.