[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