From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mout.gmx.net (mout.gmx.net [212.227.17.21]) by sourceware.org (Postfix) with ESMTPS id 89F9E385043B for ; Thu, 11 Mar 2021 11:43:39 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 89F9E385043B X-UI-Sender-Class: 01bb95c1-4bf8-414a-932a-4f6e2808ef9c Received: from [93.241.79.27] ([93.241.79.27]) by web-mail.gmx.net (3c-app-gmx-bs23.server.lan [172.19.170.75]) (via HTTP); Thu, 11 Mar 2021 12:43:35 +0100 MIME-Version: 1.0 Message-ID: From: Martin Stein To: Tobias Burnus Cc: fortran , Jerry DeLisle Subject: Re: [Patch] Fortran: Fix libgfortran I/O race with newunit_free [PR99529] Content-Type: text/plain; charset=UTF-8 Date: Thu, 11 Mar 2021 12:43:35 +0100 Importance: normal Sensitivity: Normal In-Reply-To: <9b6fa121-a5ea-4d2c-9924-fcdfe7d6d560@net-b.de> References: <9b6fa121-a5ea-4d2c-9924-fcdfe7d6d560@net-b.de> Content-Transfer-Encoding: quoted-printable X-UI-Message-Type: mail X-Priority: 3 X-Provags-ID: V03:K1:61LsefQ8RcE4occu5laX+PSPQBnesh2hqJIOGD+wQ/ADRLItaPQ3SwnZ46jw70pGIOnNH a3fm6RXOxcnT9ypvgBBJgy1+xpjOOdKkE4nxs2r1tuBZSq756zocRBzbmiKVcwlIIw9b2L6GuQqb /PFP8fZDhVLayj8nzaM759bxg2UdbM/tPhv8rHiRQj6lid11GCjtphlNgq21AHObVik7/Pr2JDzS cCM3xBxDuGwdHa/1MAjysd324ffwlzh16dTDyWgV7+ACkxrJ2Gwi0MsgHxIKsWQS+cXixuSkvkVt Q8= X-UI-Out-Filterresults: notjunk:1;V03:K0:XkkJm4faCA4=:dsoLH3RIcmubM3pgNJYtA+ u9OuKvLWFHJgy/3pQ6Crl/EhSpDH4DHyD5pdduRpwtrayHvUB8yc0151P/bsY/WskbRxBiD2/ bAZO34z8x+S+V++cavBNKUv6mvEoVYUW9Brdm2IGlHTaVc+urbvVk+jAuhvttOcW5frXxmoXP NfrWWcgAQgyHgt2jFqyOKlocLX938bYEG+HHYbGFGokdljs3OXgbEDbxYFBiMvP+5063mzD8A ZeBOf4FD+5jiZxhaiS5gqcPlLbt5Q8rLI/tXXVK2Prz1ZqAxRrGswGSICZkpq1bdB90UWdZZb oH2wjyyBHI4fxiAox9EMKOoAF0QcUIwYihUwChOKkyuLObG3XNNZAw4SyXanFSJ64DH+Y0jmP K7VEsESlzhinBlXPHiNqlfgzFMEqM4q1TCR7B2sXz38dQhkQOOe4SCclv6ibpPs/avcjS10yZ XDNVceDu2xU8VT8fNDc2r3wZiwYTs70mlqHHLmIwio8nMhHNt7EY6i98BXPO6CXqfG+10gnXG e8jY5ry/zJyLqnCXNQ3N4dM6rhuNLvNktJMtNjlfro1RA6XaMbAbLqVCPuZmAmqKdeoWI0sqo eO4TnVSXg7i7E= X-Spam-Status: No, score=-1.1 required=5.0 tests=BAYES_00, BODY_8BITS, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: fortran@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Fortran mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 11 Mar 2021 11:43:42 -0000 I have seen the lock inversion with helgrind as well=2E Otherwise I have not seen anything more in my real code=2E I will give the thread sanitizer another try=2E Problem with both are the huge number of false positives in conjunction with typical openmp code (e=2Eg=2E 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)=2E BTW, I will do some more tests, but it looks like the patch fixes the memory corruption issue=2E 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=2E=2E=2E Thanks! Martin =C2=A0 =C2=A0 =C2=A0 Gesendet:=C2=A0Donnerstag, 11=2E M=C3=A4rz 2021 um 11:38 Uhr Von:=C2=A0"Tobias Burnus" An:=C2=A0"gcc-patches" , "fortran" , "Jerry DeLisle" , mscfd@gmx=2Enet Betreff:=C2=A0Re: [Patch] Fortran: Fix libgfortran I/O race with newunit_f= ree [PR99529] Revised version =E2=80=93 the previous had a lock inversion, which could l= ead to a deadlock, found by -fsanitize=3Dthread=2E See transfer=2Ec for the chang= es=2E OK? Tobias On 11=2E03=2E21 10:42, Tobias Burnus wrote: > Hi all, > > as found by Martin (thanks!) there is a race for newunit_free=2E > While that call is within the unitlock for the calls in io/unit=2Ec, > the call in transfer=2Ec did not use locks=2E > > Additionally, > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 unit =3D get_gfc_unit (dtp->common=2Eunit= , do_create); > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 set_internal_unit (dtp, unit, kind); > gets first the unit (with proper locking when using the unit number > dtp->common=2Eunit) but then in set_internal_unit it re-sets the > unit number to the same number without locking=2E That causes > race warnings and if the assignment is not atomic it is a true race=2E > > 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)=2E > > Tobias >