Re: Threadsanitizer Detection on GM

Hongxu Chen <[email protected]> Wed, 13 Feb 2019 01:16:30 +0800
Newsgroups gmane.comp.video.graphicsmagick.bugs
Message-ID <CAJPBKOFXFiDk9xvMR9jkQuZcHaR0q3NU9jFT6b_eBzjCV+BR=w@mail.gmail.com>
--===============2261687066501630846==
Content-Type: multipart/alternative; boundary="000000000000c006020581b59678"

--000000000000c006020581b59678
Content-Type: text/plain; charset="UTF-8"

On Wed, Feb 13, 2019 at 1:05 AM Bob Friesenhahn <
[email protected]> wrote:

> On Wed, 13 Feb 2019, Hongxu Chen wrote:
>
> > 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.
>
> Does an 'omp flush' of row_count help?  Does Threadsanitizer
> understand 'omp atomic' and 'omp flush'?  These constructs are using
> CPU/hardware features to assure that all threads get the updated
> value rather than blocking while one thread updates it and then doing
> an implicit flush so that all threads see the change.
>

Yes, it understands. For example, it tells that " Atomic write" in the
report.

  Atomic write of size 8 at 0x7ffec6b0b118 by thread T1:



    #0 __tsan_atomic64_fetch_add <null> (gm+0x475e70)



    #1 .omp_outlined. /home/ubuntu/work/GM/GM-tsan/magick/gradient.c:123:7
(gm+0xae62db)


    #2 __kmp_invoke_microtask
/home/ubuntu/work/openmp/openmp-7/final/runtime/src/z_Linux_asm.S:1325
(libomp.so.5+0x8ed22)


>
> > 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.
>
> The value of row_count used by MagickMonitorFormatted() is passed by
> value so I don't see how it can read a different value of row_count
> than when it was originally prepared.
>

Yes, that is pass-by-value.
But the problem is that  there might also "(atomic) write row_count"
between these lines,
which causes the inconsistency.

atomic write row_count at line:123
<write>
read  row_count at line:124 (QuantumTick)
<write>
read  row_count at line:125 (MagickMonitorFormatted)

Therefore it's still possible that row_count at line:124 and line:125 are
different.


>
> Bob
>
>
>
> _______________________________________________
> Graphicsmagick-bugs mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/graphicsmagick-bugs
>

--000000000000c006020581b59678
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">On Wed, Feb 13, 2019 at 1:05 AM Bob Friesenhahn &lt;<a href=3D"ma=
ilto:[email protected]">[email protected]</a>&gt; wro=
te:<br></div><div class=3D"gmail_quote"><blockquote class=3D"gmail_quote" s=
tyle=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);pad=
ding-left:1ex">On Wed, 13 Feb 2019, Hongxu Chen wrote:<br>
<br>
&gt; Hi Bob,<br>
&gt;<br>
&gt;=C2=A0 =C2=A0 I think TSan warns not because of the atomic increment fo=
r row_count,<br>
&gt; but the interleaving between the write of row_count (line 123) and the=
 read<br>
&gt; of it (line 124). It&#39;s possible to have the following interleaving=
s<br>
&gt; where row_count is different.<br>
<br>
Does an &#39;omp flush&#39; of row_count help?=C2=A0 Does Threadsanitizer <=
br>
understand &#39;omp atomic&#39; and &#39;omp flush&#39;?=C2=A0 These constr=
ucts are using <br>
CPU/hardware features to assure that all threads get the updated <br>
value rather than blocking while one thread updates it and then doing <br>
an implicit flush so that all threads see the change.<br></blockquote><div>=
<br></div><div>Yes, it understands. For example, it tells that &quot;=C2=A0=
Atomic write&quot; in the report.</div><div><br></div><div><div>=C2=A0 Atom=
ic write of size 8 at 0x7ffec6b0b118 by thread T1:=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 #0 __tsan_atomic64_=
fetch_add &lt;null&gt; (gm+0x475e70)=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 #1 .omp_outlined. /h=
ome/ubuntu/work/GM/GM-tsan/magick/gradient.c:123:7 (gm+0xae62db)=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 #2 __kmp_invoke_microtask /home/ubuntu/work/=
openmp/openmp-7/final/runtime/src/z_Linux_asm.S:1325 (libomp.so.5+0x8ed22)=
=C2=A0</div></div><div>=C2=A0</div><blockquote class=3D"gmail_quote" style=
=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding=
-left:1ex">
<br>
&gt; Therefore the differences of row_count can lead to different branching=
s<br>
&gt; even if all the execution traces before entering line 123 are all the =
same.<br>
&gt; Additionally, MagickMonitorFormatted also read row_count, which may fi=
nally<br>
&gt; affect the &quot;thread_status=3DMagickFail&quot; assignment.<br>
<br>
The value of row_count used by MagickMonitorFormatted() is passed by <br>
value so I don&#39;t see how it can read a different value of row_count <br=
>
than when it was originally prepared.<br></blockquote><div><br></div><div>Y=
es, that is pass-by-value.</div><div>But the problem is that=C2=A0 there mi=
ght also &quot;(atomic) write row_count&quot; between these lines,</div><di=
v>which causes the inconsistency.</div><div><br></div><div>atomic write row=
_count at line:123</div><div>&lt;write&gt;</div><div>read=C2=A0 row_count a=
t line:124 (QuantumTick)</div><div>&lt;write&gt;</div><div>read=C2=A0 row_c=
ount at line:125 (MagickMonitorFormatted)<br></div><div><br></div><div>Ther=
efore it&#39;s still possible that row_count at line:124 and line:125 are d=
ifferent.</div><div>=C2=A0</div><blockquote class=3D"gmail_quote" style=3D"=
margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-lef=
t:1ex">
<br>
Bob<br>
<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></div></div></div></div>

--000000000000c006020581b59678--


--===============2261687066501630846==
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline


--===============2261687066501630846==
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

--===============2261687066501630846==--