Bug #119529 for threads-shared: shared.xs: warning generated when compiling with clang

Tue Dec 27 22:09:01 2016 jkeenan [...] cpan.org - Ticket created

Subject:

shared.xs: warning generated when compiling with clang

Jerry, I have set up VMs which I use mainly for smoke-testing Perl 5 blead on FreeBSD versions 10.3 and 11. When I do a smoke test run on FreeBSD 11, the compiler is Clang 3.8.0. In this build I get a number of compiler warnings, one of which is from threads-shared: ##### shared.xs:659:13: warning: calling function 'pthread_cond_timedwait' requires holding mutex 'mut' exclusively [-Wthread-safety-analysis] ##### See, e.g., http://perl5.test-smoke.org/report/52709 More complete version of the warning: ##### Making header files for XS... shared.xs:659:13: warning: calling function 'pthread_cond_timedwait' requires holding mutex 'mut' exclusively [-Wthread-safety-analysis] switch (pthread_cond_timedwait(cond, mut, &ts)) { ^ 1 warning generated. ##### threads-shared has $VERSION 1.52. Do you think this can be cleared up? Thank you very much. Jim Keenan

Tue Dec 27 22:09:23 2016 jkeenan [...] cpan.org - Broken in 1.52 added

Wed Dec 28 08:57:47 2016 jdhedden [...] cpan.org - Correspondence added

On 2016-12-27 22:09:01, JKEENAN wrote: Show quoted text

> shared.xs:659:13: warning: calling function 'pthread_cond_timedwait' > requires holding mutex 'mut' exclusively [-Wthread-safety-analysis]

Jim, This warning can be ignored. The mutex in question comes in locked, and so the call to pthread_cond_timedwait is performed correctly. Would you please try the attached patch to see if it suppresses the warning?

Subject:

shared_xs.patch

diff --git a/dist/threads-shared/shared.xs b/dist/threads-shared/shared.xs index a019732..e5799ae 100644 --- a/dist/threads-shared/shared.xs +++ b/dist/threads-shared/shared.xs @@ -656,7 +656,13 @@ Perl_sharedsv_cond_timedwait(perl_cond *cond, perl_mutex *mut, double abs) abs -= (NV)ts.tv_sec; ts.tv_nsec = (long)(abs * 1000000000.0); +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wthread-safety-analysis" + switch (pthread_cond_timedwait(cond, mut, &ts)) { + +#pragma clang diagnostic pop + case 0: got_it = 1; break; case ETIMEDOUT: break; #ifdef OEMVS

Wed Dec 28 08:57:48 2016 The RT System itself - Status changed from 'new' to 'open'

Wed Dec 28 08:57:53 2016 jdhedden [...] cpan.org - Taken

Wed Dec 28 09:10:00 2016 jdhedden [...] cpan.org - Correspondence added

On 2016-12-28 08:57:47, JDHEDDEN wrote: Show quoted text

> Would you please try the attached patch to see if it suppresses the > warning?

I found a similar clang suppression in threads.xs, and have adapted it to shared.xs. Please try the attached patch instead. Thanks.

Subject:

shared_xs.patch

diff --git a/dist/threads-shared/shared.xs b/dist/threads-shared/shared.xs index a019732..2f3dd91 100644 --- a/dist/threads-shared/shared.xs +++ b/dist/threads-shared/shared.xs @@ -656,7 +656,15 @@ Perl_sharedsv_cond_timedwait(perl_cond *cond, perl_mutex *mut, double abs) abs -= (NV)ts.tv_sec; ts.tv_nsec = (long)(abs * 1000000000.0); + CLANG_DIAG_IGNORE(-Wthread-safety); + /* warning: calling function 'pthread_cond_timedwait' requires holding mutex 'mut' exclusively [-Wthread-safety-analysis] */ + switch (pthread_cond_timedwait(cond, mut, &ts)) { + +#if defined(__clang__) || defined(__clang) +CLANG_DIAG_RESTORE; +#endif + case 0: got_it = 1; break; case ETIMEDOUT: break; #ifdef OEMVS

Wed Dec 28 13:09:30 2016 jkeenan [...] cpan.org - Correspondence added

On Wed Dec 28 09:10:00 2016, JDHEDDEN wrote: Show quoted text

> On 2016-12-28 08:57:47, JDHEDDEN wrote:

> > Would you please try the attached patch to see if it suppresses the > > warning?

> > I found a similar clang suppression in threads.xs, and have adapted it > to shared.xs. Please try the attached patch instead. Thanks.

I took your 2nd patch and created this branch in core distro: smoke-me/jkeenan/cpan-119529-threads-shared Looks good so far on FreeBSD-11. See: http://perl5.test-smoke.org/report/52716 As a precaution, I'm smoking this branch with clang on linux. Will provide update later. Thank you very much. Jim Keenan

Wed Dec 28 19:58:27 2016 jkeenan [...] cpan.org - Correspondence added

On Wed Dec 28 13:09:30 2016, JKEENAN wrote: Show quoted text

> On Wed Dec 28 09:10:00 2016, JDHEDDEN wrote:

> > On 2016-12-28 08:57:47, JDHEDDEN wrote:

> > > Would you please try the attached patch to see if it suppresses the > > > warning?

> > > > I found a similar clang suppression in threads.xs, and have adapted > > it > > to shared.xs. Please try the attached patch instead. Thanks.

> > I took your 2nd patch and created this branch in core distro: > > smoke-me/jkeenan/cpan-119529-threads-shared > > Looks good so far on FreeBSD-11. See: http://perl5.test- > smoke.org/report/52716 > > As a precaution, I'm smoking this branch with clang on linux. Will > provide update later. > > Thank you very much. > Jim Keenan

We're good on Linux as well: http://perl5.test-smoke.org/report/52720 So, either you could issue a new CPAN release and then inform p5p of the new version's availability, or I could merge my branch into blead and let you pick it up from there. Let me know which works for you. Thanks for your speedy response. Jim Keenan

Thu Dec 29 10:01:20 2016 jdhedden [...] cpan.org - Correspondence added

On 2016-12-28 19:58:27, JKEENAN wrote: Show quoted text

> So, either you could issue a new CPAN release and then inform p5p of > the new version's availability, or I could merge my branch into blead > and let you pick it up from there.

Merge to blead, let me know when it's in (directly; not via this ticket which I am closing), and I will pick up from there. Thanks.

Thu Dec 29 10:01:26 2016 jdhedden [...] cpan.org - Status changed from 'open' to 'resolved'