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

Sergey Matveev earthdok at google.com
Thu Nov 13 12:46:59 PST 2014


================
Comment at: lib/sanitizer_common/sanitizer_linux.cc:835
@@ -834,2 +834,3 @@
   return res;
 }
+#elif defined(__mips__)
----------------
Where did clone go?

================
Comment at: lib/sanitizer_common/sanitizer_linux_libcdep.cc:190
@@ +189,3 @@
+
+#ifdef __mips__
+  // TODO(kumarsukhani): add more values as per different glibc versions
----------------
please move this #ifdef into the if block at line 201:

#if defined(`__x86_64__`) || defined(`__i386__`)
if (..)
val = ...
else if (...)
val = ...
#elif defined(`__mips__`)
val = ...
#endif

================
Comment at: lib/sanitizer_common/sanitizer_linux_libcdep.cc:192
@@ +191,3 @@
+  // TODO(kumarsukhani): add more values as per different glibc versions
+  val = FIRST_32_SECOND_64(1152, 1760);
+  return val;
----------------
How did you obtain those numbers?

We need some kind of test for those, similar to SanitizerLinux.ThreadDescriptorSize (but taking into account the actual TLS layout on MIPS).

================
Comment at: lib/sanitizer_common/sanitizer_linux_libcdep.cc:242
@@ +241,3 @@
+# elif defined(__mips__)
+
+  // MIPS uses TLS variant I. The thread pointer (in hardware register $29)
----------------
Please remove the blank lines here and below.

================
Comment at: lib/sanitizer_common/sanitizer_linux_libcdep.cc:246
@@ +245,3 @@
+  // immediately in front of the TCB. kTlsPreTcbSize includes the size of the
+  // TCB and the size of pthread_descr."
+
----------------
s/"//

================
Comment at: lib/sanitizer_common/sanitizer_linux_libcdep.cc:248
@@ +247,3 @@
+
+  const uptr kTlsTcbOffset = 0x7000;
+  const uptr kTcbHead = 16;
----------------
Only this constant is part of the ABI. kTcbHead and kTlsTcbAlign could theoretically change, so we need to test them as well.

Instead of ThreadDescriptorSize(), we probably want to expose TlsPreTcbSize(). But please figure out what the test will look like before making this change.

================
Comment at: lib/sanitizer_common/sanitizer_linux_libcdep.cc:251
@@ +250,3 @@
+  const uptr kTlsTcbAlign = 16;
+  uptr kThreadPointer;
+
----------------
This naming style is used for constants. s/kThreadPointer/thread_pointer/ and please move this closer to the asm block.

================
Comment at: lib/sanitizer_common/sanitizer_linux_libcdep.cc:253
@@ +252,3 @@
+
+  uptr kTlsPreTcbSize = (ThreadDescriptorSize() + kTcbHead + kTlsTcbAlign -1)
+                        & ~(kTlsTcbAlign -1);
----------------
const

================
Comment at: lib/sanitizer_common/sanitizer_printf.cc:116
@@ -115,3 +115,3 @@
   result += AppendUnsigned(buff, buff_end, ptr_value, 16,
-                           (SANITIZER_WORDSIZE == 64) ? 12 : 8, true);
+                           SANITIZER_POINTER_FORMAT_LENGTH, true);
   return result;
----------------
Accidental change?

http://reviews.llvm.org/D5616






More information about the llvm-commits mailing list