<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><div>On Jun 11, 2012, at 5:36 PM, Chandler Carruth wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><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-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex; position: static; z-index: auto; ">
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 <a href="rdar://11434201">rdar://11434201</a>.<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></blockquote><div><br></div><div>In r158346, thanks!</div><br><blockquote type="cite"><div class="gmail_quote"><div><br></div><div> </div><blockquote class="gmail_quote" style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex; position: static; z-index: auto; ">
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>
</blockquote></div><br></body></html>