[llvm-commits] [llvm] r118461 - /llvm/trunk/lib/VMCore/Attributes.cpp

[Benjamin Kramer]

On 09.11.2010, at 01:27, Owen Anderson wrote:

> Author: resistor
> Date: Mon Nov  8 18:27:03 2010
> New Revision: 118461
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=118461&view=rev
> Log:
> Fix PR8441, a race condition in the static attributes list.  While the reference counting was itself threadsafe,
> the implicit removal of each object from the global list was not.  Make this operation atomic.
> 
> Modified:
>    llvm/trunk/lib/VMCore/Attributes.cpp
> 
> Modified: llvm/trunk/lib/VMCore/Attributes.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/VMCore/Attributes.cpp?rev=118461&r1=118460&r2=118461&view=diff
> ==============================================================================
> --- llvm/trunk/lib/VMCore/Attributes.cpp (original)
> +++ llvm/trunk/lib/VMCore/Attributes.cpp Mon Nov  8 18:27:03 2010
> @@ -106,7 +106,10 @@
> // AttributeListImpl Definition
> //===----------------------------------------------------------------------===//
> 
> +
> namespace llvm {
> +static ManagedStatic<sys::SmartMutex<true> > ALMutex;
> +
> class AttributeListImpl : public FoldingSetNode {
>   sys::cas_flag RefCount;
> 
> @@ -122,10 +125,15 @@
>     RefCount = 0;
>   }
> 
> -  void AddRef() { sys::AtomicIncrement(&RefCount); }
> +  void AddRef() {
> +    sys::SmartScopedLock<true> Lock(*ALMutex);
> +    ++RefCount;
> +  }
>   void DropRef() {
> -    sys::cas_flag old = sys::AtomicDecrement(&RefCount);
> -    if (old == 0) delete this;
> +    sys::SmartScopedLock<true> Lock(*ALMutex);
> +    sys::cas_flag old = RefCount++;
> +    if (old == 0)
> +      delete this;
>   }

Shouldn't DropRef() decrement RefCount rather than incrementing it?