Merge changes I59a8bc4a,Ic437d352

* changes:
  Fix dlsym and dladdr for TLS symbols
  Fix BionicAllocator comment

libc/bionic/bionic_allocator.cpp

@@ -45,11 +45,11 @@
 
 //
 // BionicAllocator is a general purpose allocator designed to provide the same
-// functionality as the malloc/free/realloc libc functions.
+// functionality as the malloc/free/realloc/memalign libc functions.
 //
 // On alloc:
-// If size is >= 1k allocator proxies malloc call directly to mmap
-// If size < 1k allocator uses SmallObjectAllocator for the size
+// If size is > 1k allocator proxies malloc call directly to mmap.
+// If size <= 1k allocator uses BionicSmallObjectAllocator for the size
 // rounded up to the nearest power of two.
 //
 // On free:
@@ -57,10 +57,10 @@
 // For a pointer allocated using proxy-to-mmap allocator unmaps
 // the memory.
 //
-// For a pointer allocated using SmallObjectAllocator it adds
+// For a pointer allocated using BionicSmallObjectAllocator it adds
 // the block to free_blocks_list in the corresponding page. If the number of
-// free pages reaches 2, SmallObjectAllocator munmaps one of the pages keeping
-// the other one in reserve.
+// free pages reaches 2, BionicSmallObjectAllocator munmaps one of the pages
+// keeping the other one in reserve.
 
 // Memory management for large objects is fairly straightforward, but for small
 // objects it is more complicated.  If you are changing this code, one simple

linker/linker.cpp

@@ -2383,9 +2383,26 @@ bool do_dlsym(void* handle,
 
   if (sym != nullptr) {
     uint32_t bind = ELF_ST_BIND(sym->st_info);
+    uint32_t type = ELF_ST_TYPE(sym->st_info);
 
     if ((bind == STB_GLOBAL || bind == STB_WEAK) && sym->st_shndx != 0) {
-      *symbol = reinterpret_cast<void*>(found->resolve_symbol_address(sym));
+      if (type == STT_TLS) {
+        // For a TLS symbol, dlsym returns the address of the current thread's
+        // copy of the symbol. This function may allocate a DTV and/or storage
+        // for the source TLS module. (Allocating a DTV isn't necessary if the
+        // symbol is part of static TLS, but it's simpler to reuse
+        // __tls_get_addr.)
+        soinfo_tls* tls_module = found->get_tls();
+        if (tls_module == nullptr) {
+          DL_ERR("TLS symbol \"%s\" in solib \"%s\" with no TLS segment",
+                 sym_name, found->get_realpath());
+          return false;
+        }
+        const TlsIndex ti { tls_module->module_id, sym->st_value };
+        *symbol = TLS_GET_ADDR(&ti);
+      } else {
+        *symbol = reinterpret_cast<void*>(found->resolve_symbol_address(sym));
+      }
       failure_guard.Disable();
       LD_LOG(kLogDlsym,
              "... dlsym successful: sym_name=\"%s\", sym_ver=\"%s\", found in=\"%s\", address=%p",

linker/linker_soinfo.cpp

@@ -297,7 +297,11 @@ ElfW(Sym)* soinfo::find_symbol_by_address(const void* addr) {
 }
 
 static bool symbol_matches_soaddr(const ElfW(Sym)* sym, ElfW(Addr) soaddr) {
+  // Skip TLS symbols. A TLS symbol's value is relative to the start of the TLS segment rather than
+  // to the start of the solib. The solib only reserves space for the initialized part of the TLS
+  // segment. (i.e. .tdata is followed by .tbss, and .tbss overlaps other sections.)
   return sym->st_shndx != SHN_UNDEF &&
+      ELF_ST_TYPE(sym->st_info) != STT_TLS &&
       soaddr >= sym->st_value &&
       soaddr < sym->st_value + sym->st_size;
 }

tests/elftls_dl_test.cpp

@@ -269,3 +269,69 @@ TEST(elftls_dl, dlclose_removes_entry) {
   GTEST_SKIP() << "test doesn't apply to glibc";
 #endif
 }
+
+// Use dlsym to get the address of a TLS variable in static TLS and compare it
+// against the ordinary address of the variable.
+TEST(elftls_dl, dlsym_static_tls) {
+  void* lib = dlopen("libtest_elftls_shared_var.so", RTLD_LOCAL | RTLD_NOW);
+  ASSERT_NE(nullptr, lib);
+
+  int* var_addr = static_cast<int*>(dlsym(lib, "elftls_shared_var"));
+  ASSERT_EQ(&elftls_shared_var, var_addr);
+
+  std::thread([lib] {
+    int* var_addr = static_cast<int*>(dlsym(lib, "elftls_shared_var"));
+    ASSERT_EQ(&elftls_shared_var, var_addr);
+  }).join();
+}
+
+// Use dlsym to get the address of a TLS variable in dynamic TLS and compare it
+// against the ordinary address of the variable.
+TEST(elftls_dl, dlsym_dynamic_tls) {
+  void* lib = dlopen("libtest_elftls_dynamic.so", RTLD_LOCAL | RTLD_NOW);
+  ASSERT_NE(nullptr, lib);
+  auto get_var_addr = reinterpret_cast<int*(*)()>(dlsym(lib, "get_large_tls_var_addr"));
+  ASSERT_NE(nullptr, get_var_addr);
+
+  int* var_addr = static_cast<int*>(dlsym(lib, "large_tls_var"));
+  ASSERT_EQ(get_var_addr(), var_addr);
+
+  std::thread([lib, get_var_addr] {
+    int* var_addr = static_cast<int*>(dlsym(lib, "large_tls_var"));
+    ASSERT_EQ(get_var_addr(), var_addr);
+  }).join();
+}
+
+// Calling dladdr on a TLS variable's address doesn't find anything.
+TEST(elftls_dl, dladdr_on_tls_var) {
+  Dl_info info;
+
+  // Static TLS variable
+  ASSERT_EQ(0, dladdr(&elftls_shared_var, &info));
+
+  // Dynamic TLS variable
+  void* lib = dlopen("libtest_elftls_dynamic.so", RTLD_LOCAL | RTLD_NOW);
+  ASSERT_NE(nullptr, lib);
+  int* var_addr = static_cast<int*>(dlsym(lib, "large_tls_var"));
+  ASSERT_EQ(0, dladdr(var_addr, &info));
+}
+
+// Verify that dladdr does not misinterpret a TLS symbol's value as a virtual
+// address.
+TEST(elftls_dl, dladdr_skip_tls_symbol) {
+  void* lib = dlopen("libtest_elftls_dynamic.so", RTLD_LOCAL | RTLD_NOW);
+
+  auto get_local_addr = reinterpret_cast<void*(*)()>(dlsym(lib, "get_local_addr"));
+  ASSERT_NE(nullptr, get_local_addr);
+  void* local_addr = get_local_addr();
+
+  Dl_info info;
+  ASSERT_NE(0, dladdr(local_addr, &info));
+
+  std::string libpath = GetTestlibRoot() + "/libtest_elftls_dynamic.so";
+  char dli_realpath[PATH_MAX];
+  ASSERT_TRUE(realpath(info.dli_fname, dli_realpath));
+  ASSERT_STREQ(libpath.c_str(), dli_realpath);
+  ASSERT_STREQ(nullptr, info.dli_sname);
+  ASSERT_EQ(nullptr, info.dli_saddr);
+}

tests/libs/elftls_dynamic.cpp

@@ -27,6 +27,24 @@
  */
 
 // This shared object test library is dlopen'ed by the main test executable.
+
+// Export a large TLS variable from a solib for testing dladdr and dlsym. The
+// TLS symbol's value will appear to overlap almost everything else in the
+// shared object, but dladdr must not return it.
+__thread char large_tls_var[4 * 1024 * 1024];
+
+extern "C" char* get_large_tls_var_addr() {
+  return large_tls_var;
+}
+
+// For testing dladdr, return an address that's part of the solib's .bss
+// section, but does not have an entry in the dynsym table and whose
+// solib-relative address appears to overlap with the large TLS variable.
+extern "C" void* get_local_addr() {
+  static char dummy[1024];
+  return &dummy[512];
+}
+
 // This variable comes from libtest_elftls_shared_var.so, which is part of
 // static TLS. Verify that a GD-model access can access the variable.
 //