Re: Threadsanitizer Detection on GM
Hongxu Chen <[email protected]> Wed, 13 Feb 2019 00:12:49 +0800
| Newsgroups | gmane.comp.video.graphicsmagick.bugs |
|---|---|
| Message-ID | <CAJPBKOHrbU6-T7H-EdXo9a75TXJ7=GjzmvZ2Q4H6oB4QXtaGEQ@mail.gmail.com> |
--===============3747194112870550527==
Content-Type: multipart/alternative; boundary="000000000000f6c12d0581b4b2c2"
--000000000000f6c12d0581b4b2c2
Content-Type: text/plain; charset="UTF-8"
Hi Bob,
I think TSan warns not because of the atomic increment for row_count,
but the interleaving between the write of row_count (line 123) and the read
of it (line 124). It's possible to have the following interleavings
where row_count is different.
T1: row_count++;
T2: row_count++;
T1: if (QuantumTick(row_count,image->rows))
T2: if (QuantumTick(row_count,image->rows))
and
T1: row_count++;
T1: if (QuantumTick(row_count,image->rows))
T2: row_count++;
T2: if (QuantumTick(row_count,image->rows))
By observing QuantumTick's expansion at studio.h:
#define QuantumTick(i,span) \
((((i) % ((Max(101,span)-1)/100)) == 0) || \
((magick_int64_t) (i) == ((magick_int64_t) (span)-1)))
Therefore the differences of row_count can lead to different branchings
even if all the execution traces before entering line 123 are all the same.
Additionally, MagickMonitorFormatted also read row_count, which may finally
affect the "thread_status=MagickFail" assignment.
Best Regards,
Hongxu
On Wed, Feb 6, 2019 at 2:45 AM Bob Friesenhahn <[email protected]>
wrote:
> On Wed, 6 Feb 2019, Hongxu Chen wrote:
> > WARNING: ThreadSanitizer: data race (pid=29974)
> > Atomic write of size 8 at 0x7ffee84393b8 by thread T6:
> > #0 __tsan_atomic64_fetch_add <null> (gm+0x476040)
> > #1 .omp_outlined. /home/exp/work/gm/GM-tsan/magick/gradient.c:123:7
>
> The 'row_count' stack variable is declared to be omp shared and
> updated under the apparent protection of a 'pragma omp atomic' so I am
> curious if there is an actual problem here. Is there some code change
> (e.g. a 'pragma omp flush (row_count)') which makes the reported issue
> go away?
>
> > I'm trying some concurrent detection techniques, so I can try GM versions
> > prior to 1.3.31,
> > so thanks for this tip!
>
> It would be interesting to see the difference.
>
> Ultimately, well-tested patches which solve problems are most useful.
>
> Bob
> --
> Bob Friesenhahn
> [email protected], http://www.simplesystems.org/users/bfriesen/
> GraphicsMagick Maintainer, http://www.GraphicsMagick.org/
> Public Key, http://www.simplesystems.org/users/bfriesen/public-key.txt
>
>
> _______________________________________________
> Graphicsmagick-bugs mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/graphicsmagick-bugs
>
--000000000000f6c12d0581b4b2c2
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
<div dir=3D"ltr"><div dir=3D"ltr"><div dir=3D"ltr"><div dir=3D"ltr"><div di=
r=3D"ltr"><div dir=3D"ltr"><div dir=3D"ltr"><div dir=3D"ltr"><div dir=3D"lt=
r"><div dir=3D"ltr">Hi Bob,<div><br></div><div>=C2=A0 =C2=A0 I think TSan w=
arns not because of the atomic increment for row_count, but the interleavin=
g between the write of row_count (line 123) and the read of it (line 124). =
It's possible to have the following interleavings where=C2=A0row_count =
is different.</div><div><br></div><div>=C2=A0 =C2=A0 T1:=C2=A0row_count++;<=
/div><div>=C2=A0 =C2=A0 T2:=C2=A0row_count++;</div><div>=C2=A0 =C2=A0 T1:=
=C2=A0if (QuantumTick(row_count,image->rows))</div><div>=C2=A0 =C2=A0 T2=
:=C2=A0if (QuantumTick(row_count,image->rows))</div><div><br></div><div>=
and</div><div><br></div><div>=C2=A0 =C2=A0 T1:=C2=A0row_count++;</div><div>=
=C2=A0 =C2=A0 T1:=C2=A0if (QuantumTick(row_count,image->rows))<br></div>=
<div><div>=C2=A0 =C2=A0 T2:=C2=A0row_count++;</div></div><div>=C2=A0 =C2=A0=
T2:=C2=A0if (QuantumTick(row_count,image->rows))</div><div><br></div><d=
iv>By observing QuantumTick's expansion at studio.h:</div><div><div>#de=
fine QuantumTick(i,span) \=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0=C2=A0</div><div>=C2=A0 ((((i) % ((Max(101,span)-1)=
/100)) =3D=3D 0) || \=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0</div><div>=C2=A0 =C2=A0 ((magick_int=
64_t) (i) =3D=3D ((magick_int64_t) (span)-1)))=C2=A0</div></div><div><br></=
div><div>Therefore the differences of row_count can lead to different branc=
hings even if all the execution traces before entering line 123 are all the=
same.</div><div>Additionally,=C2=A0MagickMonitorFormatted also read=C2=A0r=
ow_count, which may finally affect the "thread_status=3DMagickFail&quo=
t; assignment.</div><div><br clear=3D"all"><div><div dir=3D"ltr" class=3D"g=
mail_signature"><div dir=3D"ltr"><div><div dir=3D"ltr"><font color=3D"#3131=
31" face=3D"Arial, sans-serif"><span style=3D"font-size:12px">Best Regards,=
</span></font><div><font color=3D"#313131" face=3D"Arial, sans-serif"><span=
style=3D"font-size:12px">Hongxu</span></font></div></div></div></div></div=
></div><br></div></div></div></div></div></div></div></div></div></div></di=
v><br><div class=3D"gmail_quote"><div dir=3D"ltr" class=3D"gmail_attr">On W=
ed, Feb 6, 2019 at 2:45 AM Bob Friesenhahn <<a href=3D"mailto:bfriesen@s=
imple.dallas.tx.us">[email protected]</a>> wrote:<br></div><b=
lockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-le=
ft:1px solid rgb(204,204,204);padding-left:1ex">On Wed, 6 Feb 2019, Hongxu =
Chen wrote:<br>
> WARNING: ThreadSanitizer: data race (pid=3D29974)<br>
>=C2=A0 Atomic write of size 8 at 0x7ffee84393b8 by thread T6:<br>
>=C2=A0 =C2=A0 #0 __tsan_atomic64_fetch_add <null> (gm+0x476040)<b=
r>
>=C2=A0 =C2=A0 #1 .omp_outlined. /home/exp/work/gm/GM-tsan/magick/gradie=
nt.c:123:7<br>
<br>
The 'row_count' stack variable is declared to be omp shared and <br=
>
updated under the apparent protection of a 'pragma omp atomic' so I=
am <br>
curious if there is an actual problem here.=C2=A0 Is there some code change=
<br>
(e.g. a 'pragma omp flush (row_count)') which makes the reported is=
sue <br>
go away?<br>
<br>
> I'm trying some concurrent detection techniques, so I can try GM v=
ersions<br>
> prior to 1.3.31,<br>
> so thanks for this tip!<br>
<br>
It would be interesting to see the difference.<br>
<br>
Ultimately, well-tested patches which solve problems are most useful.<br>
<br>
Bob<br>
-- <br>
Bob Friesenhahn<br>
<a href=3D"mailto:[email protected]" target=3D"_blank">bfriesen@=
simple.dallas.tx.us</a>, <a href=3D"http://www.simplesystems.org/users/bfri=
esen/" rel=3D"noreferrer" target=3D"_blank">http://www.simplesystems.org/us=
ers/bfriesen/</a><br>
GraphicsMagick Maintainer,=C2=A0 =C2=A0 <a href=3D"http://www.GraphicsMagic=
k.org/" rel=3D"noreferrer" target=3D"_blank">http://www.GraphicsMagick.org/=
</a><br>
Public Key,=C2=A0 =C2=A0 =C2=A0<a href=3D"http://www.simplesystems.org/user=
s/bfriesen/public-key.txt" rel=3D"noreferrer" target=3D"_blank">http://www.=
simplesystems.org/users/bfriesen/public-key.txt</a><br>
<br>
<br>
_______________________________________________<br>
Graphicsmagick-bugs mailing list<br>
<a href=3D"mailto:[email protected]" target=3D"_bla=
nk">[email protected]</a><br>
<a href=3D"https://lists.sourceforge.net/lists/listinfo/graphicsmagick-bugs=
" rel=3D"noreferrer" target=3D"_blank">https://lists.sourceforge.net/lists/=
listinfo/graphicsmagick-bugs</a><br>
</blockquote></div>
--000000000000f6c12d0581b4b2c2--
--===============3747194112870550527==
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline
--===============3747194112870550527==
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline
_______________________________________________
Graphicsmagick-bugs mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/graphicsmagick-bugs
--===============3747194112870550527==--