<div class="gmail_quote">On Mon, Jun 11, 2012 at 5:21 PM, Argyrios Kyrtzidis <span dir="ltr"><<a href="mailto:akyrtzi@gmail.com" target="_blank">akyrtzi@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Author: akirtzidis<br>
Date: Mon Jun 11 19:21:31 2012<br>
New Revision: 158342<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=158342&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=158342&view=rev</a><br>
Log:<br>
For llvm::sys::ThreadLocalImpl instead of malloc'ing the platform-specific<br>
thread local data, embed them in the class using a uint64_t and make sure<br>
we get compiler errors if there's a platform where this is not big enough.<br>
<br>
This makes ThreadLocal more safe for using it in conjunction with CrashRecoveryContext.<br>
<br>
Related to crash in rdar://11434201.<br>
<br>
Modified:<br>
 Â  Â llvm/trunk/include/llvm/Support/ThreadLocal.h<br>
 Â  Â llvm/trunk/lib/Support/ThreadLocal.cpp<br>
 Â  Â llvm/trunk/lib/Support/Windows/ThreadLocal.inc<br>
<br>
Modified: llvm/trunk/include/llvm/Support/ThreadLocal.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/ThreadLocal.h?rev=158342&r1=158341&r2=158342&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/ThreadLocal.h?rev=158342&r1=158341&r2=158342&view=diff</a><br>

==============================================================================<br>
--- llvm/trunk/include/llvm/Support/ThreadLocal.h (original)<br>
+++ llvm/trunk/include/llvm/Support/ThreadLocal.h Mon Jun 11 19:21:31 2012<br>
@@ -15,6 +15,7 @@<br>
 #define LLVM_SYSTEM_THREAD_LOCAL_H<br>
<br>
 #include "llvm/Support/Threading.h"<br>
+#include "llvm/Support/DataTypes.h"<br>
 #include <cassert><br>
<br>
 namespace llvm {<br>
@@ -22,7 +23,12 @@<br>
 Â  Â  // ThreadLocalImpl - Common base class of all ThreadLocal instantiations.<br>
 Â  Â  // YOU SHOULD NEVER USE THIS DIRECTLY.<br>
 Â  Â  class ThreadLocalImpl {<br>
- Â  Â  Â void* data;<br>
+ Â  Â  Â typedef uint64_t ThreadLocalDataTy;<br>
+ Â  Â  Â /// \brief Platform-specific thread local data.<br>
+ Â  Â  Â ///<br>
+ Â  Â  Â /// This is embedded in the class and we avoid malloc'ing/free'ing it,<br>
+ Â  Â  Â /// to make this class more safe for use along with CrashRecoveryContext.<br>
+ Â  Â  Â ThreadLocalDataTy data;<br></blockquote><div><br></div><div>If you're embedding arbitrary data into this, please use a character array to satisfy C++ aliasing rules. Also I would add something to force alignment:</div>
<div><br></div><div>union {</div><div>  char data[sizeof(uint64_t)];</div><div>  struct {</div><div>  Â  uint64_t dummy_alignment_member;</div><div>  }</div><div>}</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

 Â  Â  public:<br>
 Â  Â  Â  ThreadLocalImpl();<br>
 Â  Â  Â  virtual ~ThreadLocalImpl();<br>
<br>
Modified: llvm/trunk/lib/Support/ThreadLocal.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/ThreadLocal.cpp?rev=158342&r1=158341&r2=158342&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/ThreadLocal.cpp?rev=158342&r1=158341&r2=158342&view=diff</a><br>

==============================================================================<br>
--- llvm/trunk/lib/Support/ThreadLocal.cpp (original)<br>
+++ llvm/trunk/lib/Support/ThreadLocal.cpp Mon Jun 11 19:21:31 2012<br>
@@ -41,30 +41,29 @@<br>
 using namespace sys;<br>
<br>
 ThreadLocalImpl::ThreadLocalImpl() : data(0) {<br>
- Â pthread_key_t* key = new pthread_key_t;<br>
+ Â typedef int SIZE_TOO_BIG[sizeof(pthread_key_t) <= sizeof(data) ? 1 : -1];<br>
+ Â pthread_key_t* key = reinterpret_cast<pthread_key_t*>(&data);<br>
 Â  int errorcode = pthread_key_create(key, NULL);<br>
 Â  assert(errorcode == 0);<br>
 Â  (void) errorcode;<br>
- Â data = (void*)key;<br>
 }<br>
<br>
 ThreadLocalImpl::~ThreadLocalImpl() {<br>
- Â pthread_key_t* key = static_cast<pthread_key_t*>(data);<br>
+ Â pthread_key_t* key = reinterpret_cast<pthread_key_t*>(&data);<br>
 Â  int errorcode = pthread_key_delete(*key);<br>
 Â  assert(errorcode == 0);<br>
 Â  (void) errorcode;<br>
- Â delete key;<br>
 }<br>
<br>
 void ThreadLocalImpl::setInstance(const void* d) {<br>
- Â pthread_key_t* key = static_cast<pthread_key_t*>(data);<br>
+ Â pthread_key_t* key = reinterpret_cast<pthread_key_t*>(&data);<br>
 Â  int errorcode = pthread_setspecific(*key, d);<br>
 Â  assert(errorcode == 0);<br>
 Â  (void) errorcode;<br>
 }<br>
<br>
 const void* ThreadLocalImpl::getInstance() {<br>
- Â pthread_key_t* key = static_cast<pthread_key_t*>(data);<br>
+ Â pthread_key_t* key = reinterpret_cast<pthread_key_t*>(&data);<br>
 Â  return pthread_getspecific(*key);<br>
 }<br>
<br>
<br>
Modified: llvm/trunk/lib/Support/Windows/ThreadLocal.inc<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Windows/ThreadLocal.inc?rev=158342&r1=158341&r2=158342&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Windows/ThreadLocal.inc?rev=158342&r1=158341&r2=158342&view=diff</a><br>

==============================================================================<br>
--- llvm/trunk/lib/Support/Windows/ThreadLocal.inc (original)<br>
+++ llvm/trunk/lib/Support/Windows/ThreadLocal.inc Mon Jun 11 19:21:31 2012<br>
@@ -22,26 +22,25 @@<br>
 namespace llvm {<br>
 using namespace sys;<br>
<br>
-ThreadLocalImpl::ThreadLocalImpl() {<br>
- Â DWORD* tls = new DWORD;<br>
+ThreadLocalImpl::ThreadLocalImpl() : data(0) {<br>
+ Â typedef int SIZE_TOO_BIG[sizeof(DWORD) <= sizeof(data) ? 1 : -1];<br>
+ Â DWORD* tls = reinterpret_cast<DWORD*>(&data);<br>
 Â  *tls = TlsAlloc();<br>
 Â  assert(*tls != TLS_OUT_OF_INDEXES);<br>
- Â data = tls;<br>
 }<br>
<br>
 ThreadLocalImpl::~ThreadLocalImpl() {<br>
- Â DWORD* tls = static_cast<DWORD*>(data);<br>
+ Â DWORD* tls = reinterpret_cast<DWORD*>(&data);<br>
 Â  TlsFree(*tls);<br>
- Â delete tls;<br>
 }<br>
<br>
 const void* ThreadLocalImpl::getInstance() {<br>
- Â DWORD* tls = static_cast<DWORD*>(data);<br>
+ Â DWORD* tls = reinterpret_cast<DWORD*>(&data);<br>
 Â  return TlsGetValue(*tls);<br>
 }<br>
<br>
 void ThreadLocalImpl::setInstance(const void* d){<br>
- Â DWORD* tls = static_cast<DWORD*>(data);<br>
+ Â DWORD* tls = reinterpret_cast<DWORD*>(&data);<br>
 Â  int errorcode = TlsSetValue(*tls, const_cast<void*>(d));<br>
 Â  assert(errorcode != 0);<br>
 Â  (void)errorcode;<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br>