[PATCH] D17883: [ASAN] Add support for mips/mips64 android

Duane Sand via llvm-commits llvm-commits at lists.llvm.org
Mon Mar 14 12:14:09 PDT 2016

duanesand added inline comments.

Comment at: lib/sanitizer_common/sanitizer_linux_libcdep.cc:160
@@ -160,1 +159,3 @@
 static uptr g_tls_size;
Sorry, my mistake;  this should revert back to the original condition, without SANITIZER_MIPS

Comment at: lib/sanitizer_common/sanitizer_linux_libcdep.cc:162
@@ -160,3 +161,3 @@
 static uptr g_tls_size;
This #endif should be moved down around all things using this variable; either after TlsPreTcbSize, or better yet, after InitTlsSize.

Comment at: lib/sanitizer_common/sanitizer_linux_libcdep.cc:173
@@ -171,3 +172,3 @@
 // head structure. It lies before the static tls blocks.
 static uptr TlsPreTcbSize() {
 # if defined(__mips__)
This function is only used when the nonempty version of InitTlsSize() is used, ie when


This TlsPreTcbSize() function must similarly be skipped when that condition is false, otherwise there is an unresolved ref to  g_tls_size  during mips Android builds.

Comment at: lib/sanitizer_common/sanitizer_linux_libcdep.cc:189
@@ -187,3 +188,3 @@
 void InitTlsSize() {
 // all current supported platforms have 16 bytes stack alignment
This condition (which must match line 159 in the original) is prone to breaking again in the future.  It would be best to not duplicate it, or else give it a positive name like  SANITIZER_USES_TLS.  

Rather than defining it as a bunch of negatives (against what world of possible os's?), I suggest redefining it more exactly as
if that is indeed the current situation

Comment at: lib/sanitizer_common/sanitizer_linux_libcdep.cc:207
@@ -205,3 +206,3 @@
 #if (defined(__x86_64__) || defined(__i386__) || defined(__mips__) \
This defines a do-nothing body for InitTlsSize for some os's including Android.  That allows its callers' source to be free of os dependencies, which is good.  But the current way of skipping its body requires two matching instances of a non-obvious #if.  These could be combined into a single #if by repeating the entire header and body of the do-nothing function in an #else part, ie

void InitTlsSize() { }

Comment at: lib/sanitizer_common/sanitizer_platform_limits_posix.h:554
@@ -545,1 +553,3 @@
+    };
+    __sanitizer_sigset_t sa_mask;
final  };  is missing



More information about the llvm-commits mailing list