[PATCH] [lsan] [mips] adding support of lsan for mips64/mips64el arch
Sergey Matveev
earthdok at google.com
Tue Oct 28 13:11:05 PDT 2014
>>! In D5616#32, @kumarsukhani wrote:
>> How did you test this patch?
>
> I just ran the testcases of lsan on mips rpe-100 board, and majority of LeakSanitizer-Standalone test cases are working.
Which tests are failing and why? And what about LeakSanitizer-AddressSanitizer?
(I imagine at least use_tls_pthread_specific_static.cc would be failing because of the incorrect value of kTlsPreTcbSize.)
================
Comment at: lib/sanitizer_common/sanitizer_linux_libcdep.cc:236
@@ +235,3 @@
+# elif defined(__mips__)
+// For MIPS TLS variant I is used
+// The thread pointer (in hardware register $29) points to
----------------
Comment indentation should match code indentation.
================
Comment at: lib/sanitizer_common/sanitizer_linux_libcdep.cc:238
@@ +237,3 @@
+// The thread pointer (in hardware register $29) points to
+// the end of the TCB + 0x7000
+
----------------
This comment doesn't give the complete picture. How about:
"MIPS uses TLS variant I. The thread pointer (in hardware register $29) points to the end of the TCB + 0x7000. The pthread_descr structure is immediately in front of the TCB. kTlsPreTcbSize includes the size of the TCB and the size of pthread_descr."
================
Comment at: lib/sanitizer_common/sanitizer_linux_libcdep.cc:241
@@ +240,3 @@
+ const uptr kTlsTcbOffset = 0x7000;
+ const uptr kTlsPreTcbSize = 0x10;
+
----------------
This can't be correct. How did you obtain this constant?
In glibc sources we have:
95 # define TLS_PRE_TCB_SIZE \
96 (sizeof (struct pthread) \
97 + ((sizeof (tcbhead_t) + TLS_TCB_ALIGN - 1) & ~(TLS_TCB_ALIGN - 1)))
tcbhead_t alone is 16 bytes. struct pthread is much bigger.
This also needs a test, like the one we have for kThreadDescriptorSize. The size of struct pthread will change between glibc versions.
================
Comment at: lib/sanitizer_common/sanitizer_linux_libcdep.cc:284
@@ -268,1 +283,3 @@
+ *addr = ThreadSelf();
+ *size = GetTlsSize();
# else
----------------
I don't think this gives the correct TLS size. Your kTlsPreTcbSize above is too small, so *addr as you're currently computing it is too big. So if *addr is incorrect, *size can't be correct - otherwise we would be accessing past the end of the TLS and LSan would crash.
Probably you need something like
*size = GetTlsSize() + kTlsPreTcbSize;
or
*size = GetTlsSize() + kTlsPreTcbSize + kTlsTcbOffset;
I can't say what exactly without digging into glibc sources. Could you please do that?
http://reviews.llvm.org/D5616
More information about the llvm-commits
mailing list