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 <<a href=3D"ma= ilto:[email protected]">[email protected]</a>> 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> > Hi Bob,<br> ><br> >=C2=A0 =C2=A0 I think TSan warns not because of the atomic increment fo= r row_count,<br> > but the interleaving between the write of row_count (line 123) and the= read<br> > of it (line 124). It's possible to have the following interleaving= s<br> > where row_count is different.<br> <br> Does an 'omp flush' of row_count help?=C2=A0 Does Threadsanitizer <= br> understand 'omp atomic' and 'omp flush'?=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 "=C2=A0= Atomic write" 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 <null> (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> > Therefore the differences of row_count can lead to different branching= s<br> > even if all the execution traces before entering line 123 are all the = same.<br> > Additionally, MagickMonitorFormatted also read row_count, which may fi= nally<br> > affect the "thread_status=3DMagickFail" assignment.<br> <br> The value of row_count used by MagickMonitorFormatted() is passed by <br> value so I don'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 "(atomic) write row_count" between these lines,</div><di= v>which causes the inconsistency.</div><div><br></div><div>atomic write row= _count at line:123</div><div><write></div><div>read=C2=A0 row_count a= t line:124 (QuantumTick)</div><div><write></div><div>read=C2=A0 row_c= ount at line:125 (MagickMonitorFormatted)<br></div><div><br></div><div>Ther= efore it'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==--