[LLVMdev] [cfe-dev] clang with pthread on mingw

[Owen Anderson]

David,

Your fix sounds fine to me.  Please apply.

--Owen

On Feb 14, 2010, at 6:30 AM, David Chisnall wrote:

> Hi Vincent,
> 
> This is an LLVM issue, rather than a clang issue, so llvmdev is the appropriate mailing list for this report.
> 
> Your fix looks semantically valid, but it would be cleaner to only create and destroy the pthread_rwlockattr_t if it is actually needed, rather than creating it, initialising it, not using it, and then destroying it on mingw (and, as the code currently stands, *BSD).  
> 
> The code in the #ifdef just above your change is a bit strange too.  Perhaps someone could explain this.  It is setting PTHREAD_PROCESS_PRIVATE, which POSIX says should be the default, but seems to only be doing it on some platforms.  Oddly, this is done by excluding *BSD (but not Darwin).  If Linux / Darwin / Solaris / some other platforms have a bug in their pthread implementation and default to the wrong value, then it would be cleaner to enumerate those platforms and only use the attributes there.
> 
> On any platform with a standards-compliant pthread implementation, you will get the same behaviour by passing NULL as the attribute argument (create with default attributes).  Creating the attributes object is only required on platforms where the default is incorrect.  
> 
> Checking the documentation, this means that it is not needed on FreeBSD, Darwin, OpenBSD, or Solaris, and probably not needed anywhere else (it doesn't seem to be documented in glibc, but there's always a chance that they followed the spec). 
> 
> Unless someone can nominate a platform that doesn't follow the spec and does default to shared rwlocks, I'd suggest removing all of the code related to attr in that function and just passing NULL as the attribute parameter in all cases.
> 
> I'm happy to make this change if no one objects.
> 
> David
> 
> On 14 Feb 2010, at 14:09, Vincent Richomme wrote:
> 
>> On Sun, 14 Feb 2010 14:26:38 +0100, Vincent Richomme
>> <forumer at smartmobili.com> wrote:
>>> Hi,
>>> 
>>> When testing clang on mingw platform I got some errors when pthread was
>>> enabled.
>>> Problem was in line 87 in RWMutex.cpp:
>>> 
>>> // Initialize the rwlock
>>>   errorcode = pthread_rwlock_init(rwlock, &attr);
>>>   assert(errorcode == 0);
>>> 
>>> 
>>> on mingw platform, pthread only support NULL attribues as shown below:
>>> 
>>> int
>>> pthread_rwlock_init (pthread_rwlock_t * rwlock,
>>> 		     const pthread_rwlockattr_t * attr)
>>> {
>>> int result;
>>> pthread_rwlock_t rwl = 0;
>>> 
>>> if (rwlock == NULL)
>>>   {
>>>     return EINVAL;
>>>   }
>>> 
>>> if (attr != NULL && *attr != NULL)
>>>   {
>>>     result = EINVAL;		/* Not supported */
>>>     goto DONE;
>>>   }
>>> 
>>> ...
>>> }
>>> 
>>> Could someone add a #ifdef to pass NULL when __MINGW32__ is defined.
>>> 
>> 
>> I have opened a bug report and provided a fix :
>> http://llvm.org/bugs/show_bug.cgi?id=6297
>> 
>> _______________________________________________
>> cfe-dev mailing list
>> cfe-dev at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
> 
> 
> -- Sent from my Apple II
> 
> 
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev

-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 2620 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20100214/7593485b/attachment.bin>