Viacheslav Nikolaev via llvm-dev
2016-Dec-26 06:20 UTC
[llvm-dev] A potential race on StaticList in RegisterManagedStatic
Ptr member of ManagedStaticBase is now atomic.
In ManagedStaticBase::RegisterManagedStatic we have such code:
void *Tmp = Creator();
Ptr.store(Tmp, std::memory_order_release);
DeleterFn = Deleter;
// Add to list of managed statics.
Next = StaticList;
StaticList = this;
StaticList is not atomic and not guarded by any fence.
The same applies to the members DeleterFn and Next.
Doesn't it seem reasonable to change the code to
DeleterFn = Deleter;
// Add to list of managed statics.
Next = StaticList;
StaticList = this;
Ptr.store(Tmp, std::memory_order_release);
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20161226/addc40f4/attachment.html>
Viacheslav Nikolaev via llvm-dev
2016-Dec-26 07:14 UTC
[llvm-dev] A potential race on StaticList in RegisterManagedStatic
Though it won't actually help to guard this: while (StaticList) - a fence here is needed... On Mon, Dec 26, 2016 at 9:20 AM, Viacheslav Nikolaev < viacheslav.nikolaev at gmail.com> wrote:> Ptr member of ManagedStaticBase is now atomic. > In ManagedStaticBase::RegisterManagedStatic we have such code: > > void *Tmp = Creator(); > > Ptr.store(Tmp, std::memory_order_release); > DeleterFn = Deleter; > > // Add to list of managed statics. > Next = StaticList; > StaticList = this; > > > StaticList is not atomic and not guarded by any fence. > The same applies to the members DeleterFn and Next. > > Doesn't it seem reasonable to change the code to > > DeleterFn = Deleter; > > // Add to list of managed statics. > Next = StaticList; > StaticList = this; > Ptr.store(Tmp, std::memory_order_release); >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20161226/b50e927c/attachment.html>
Mehdi Amini via llvm-dev
2016-Dec-26 07:17 UTC
[llvm-dev] A potential race on StaticList in RegisterManagedStatic
What about the lock that is taken? MutexGuard Lock(*getManagedStaticMutex()); — Mehdi> On Dec 25, 2016, at 11:14 PM, Viacheslav Nikolaev via llvm-dev <llvm-dev at lists.llvm.org> wrote: > > Though it won't actually help to guard this: while (StaticList) - a fence here is needed... > > On Mon, Dec 26, 2016 at 9:20 AM, Viacheslav Nikolaev <viacheslav.nikolaev at gmail.com <mailto:viacheslav.nikolaev at gmail.com>> wrote: > Ptr member of ManagedStaticBase is now atomic. > In ManagedStaticBase::RegisterManagedStatic we have such code: > > void *Tmp = Creator(); > > Ptr.store(Tmp, std::memory_order_release); > DeleterFn = Deleter; > > // Add to list of managed statics. > Next = StaticList; > StaticList = this; > > > StaticList is not atomic and not guarded by any fence. > The same applies to the members DeleterFn and Next. > > Doesn't it seem reasonable to change the code to > > DeleterFn = Deleter; > > // Add to list of managed statics. > Next = StaticList; > StaticList = this; > Ptr.store(Tmp, std::memory_order_release); > > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20161225/4640195e/attachment.html>