Re: [Patch] Fortran: Fix libgfortran I/O race with newunit_free [PR99529]

[Martin Stein <mscfd@gmx.net>]

I have seen the lock inversion with helgrind as well. Otherwise I
have not seen anything more in my real code. I will give the
thread sanitizer another try. Problem with both are the huge
number of false positives in conjunction with typical openmp code
(e.g. master thread allocates a big vector or matrix outside
openmp region, and then it is accessed within openmp loop by all
threads without locks of course -> possible data race warning).

BTW, I will do some more tests, but it looks like the patch fixes
the memory corruption issue. This morning I tried with a wrong
build setup, as I used patched gfortran but linked with unpatched
gcc, which probably used the unpatched libgfortran...

Thanks!
Martin
 
 
 

Gesendet: Donnerstag, 11. März 2021 um 11:38 Uhr
Von: "Tobias Burnus" <burnus@net-b.de>
An: "gcc-patches" <gcc-patches@gcc.gnu.org>, "fortran" <fortran@gcc.gnu.org>, "Jerry DeLisle" <jvdelisle@charter.net>, mscfd@gmx.net
Betreff: Re: [Patch] Fortran: Fix libgfortran I/O race with newunit_free [PR99529]
Revised version – the previous had a lock inversion, which could lead to
a deadlock, found by -fsanitize=thread. See transfer.c for the changes.

OK?

Tobias

On 11.03.21 10:42, Tobias Burnus wrote:
> Hi all,
>
> as found by Martin (thanks!) there is a race for newunit_free.
> While that call is within the unitlock for the calls in io/unit.c,
> the call in transfer.c did not use locks.
>
> Additionally,
>       unit = get_gfc_unit (dtp->common.unit, do_create);
>       set_internal_unit (dtp, unit, kind);
> gets first the unit (with proper locking when using the unit number
> dtp->common.unit) but then in set_internal_unit it re-sets the
> unit number to the same number without locking. That causes
> race warnings and if the assignment is not atomic it is a true race.
>
> OK for mainline? What about GCC 10?
>
> As Martin notes in the email thread and in the PR there are more
> race warnings (and likely true race issues).
>
> Tobias
>