[PATCH] [lsan] [mips] adding support of lsan for mips64/mips64el arch

Sergey Matveev earthdok at google.com
Mon Dec 8 07:40:38 PST 2014


>>! In D5616#76, @kumarsukhani wrote:
> @earthdok: I am not sure if mips stores the value of pthread struct size anywhere the way x86 does to write a UNIT test for pthread struct size.
> Btw why x86_64 is using the hard codes values, even though they have direct access to pthread structure size?

The test that we have for x86_64 doesn't read the size from the struct. Rather, it relies on assumptions regarding how the TCB is positioned relative to the beginning of the stack. We could use that method to actually determine the TCB size at runtime. However: 1) it requires spawning a thread, which is pretty awkward and intrusive to user code and 2) we would have no way of knowing if those assumptions no longer held true.

If the size was stored in the struct itself, that would certainly make things easier, but to the best of my knowledge it isn't.

================
Comment at: lib/sanitizer_common/sanitizer_linux_libcdep.cc:185
@@ +184,3 @@
+  const uptr kTlsTcbAlign = 16;
+  const uptr kTlsPreTcbSize = (ThreadDescriptorSize() + kTcbHead +
+                               kTlsTcbAlign - 1) & ~(kTlsTcbAlign -1);
----------------
No need to introduce a separate constant, just return this.

================
Comment at: lib/sanitizer_common/sanitizer_linux_libcdep.cc:262
@@ -235,1 +261,3 @@
+                .set\tpop" : "=r" (thread_pointer));
+  descr_addr = thread_pointer - kTlsTcbOffset - getTlsPreTcbSize();
 # else
----------------
Let's drop "get", to be consistent with ThreadSelfOffset(), ThreadDescriptorSize().

================
Comment at: lib/sanitizer_common/sanitizer_linux_libcdep.cc:329
@@ -297,1 +328,3 @@
   return g_tls_size;
+#elif defined(__mips__)
+  uptr kdl_tls_static_align = 16;
----------------
There's already a "|| defined(__mips__)" above, so you will never hit this branch.

================
Comment at: lib/sanitizer_common/sanitizer_linux_libcdep.cc:330
@@ +329,3 @@
+#elif defined(__mips__)
+  uptr kdl_tls_static_align = 16;
+  return ((g_tls_size + getTlsPreTcbSize() + kdl_tls_static_align -1)
----------------
const uptr kDlTlsStaticAlign

================
Comment at: lib/sanitizer_common/sanitizer_linux_libcdep.cc:331
@@ +330,3 @@
+  uptr kdl_tls_static_align = 16;
+  return ((g_tls_size + getTlsPreTcbSize() + kdl_tls_static_align -1)
+         & ~(kdl_tls_static_align - 1));
----------------
Why not do this calculation once in InitTlsSize().

================
Comment at: lib/sanitizer_common/sanitizer_linux_libcdep.cc:334
@@ -298,1 +333,3 @@
+#else
+#  error "unsupported CPU arch"
 #endif
----------------
You're breaking compilation on every arch other than x86-64 and MIPS. Why?

http://reviews.llvm.org/D5616






More information about the llvm-commits mailing list