[patches] Cherry pick CLS for: Emulated TLS + shared-library breakage

Bug: android/ndk#1551 b8f04a670f2 [builtins] Try to ensure single copy of emulated TLS state Test: N/A Change-Id: I3d537a8415f428fafc4822b8bdbc33b70e09d261
riscv-android-src · Sep 2, 2021 · 50f8789 · 50f8789
1 parent a4698b8
commit 50f8789
Show file tree

Hide file tree

Showing 2 changed files with 60 additions and 1 deletion.
diff --git a/patches/PATCHES.json b/patches/PATCHES.json
@@ -233,6 +233,12 @@
         "start_version": 416183,
         "end_version": 429795
     },
+    {
+        "comment": "[UPSTREAM] [builtins] Try to ensure single copy of emulated TLS state",
+        "rel_patch_path": "cherry/b8f04a670f27a84412099dd025fa762ee58f4c1a.patch",
+        "start_version": 416183,
+        "end_version": 434407
+    },
     {
         "comment": "Revert two changes that break Android builds.",
         "rel_patch_path": "Revert-two-changes-that-break-Android-builds.v5.patch",
@@ -299,4 +305,4 @@
         "start_version": 416183,
         "end_version": null
     }
-]
+]
diff --git a/patches/cherry/b8f04a670f27a84412099dd025fa762ee58f4c1a.patch b/patches/cherry/b8f04a670f27a84412099dd025fa762ee58f4c1a.patch
@@ -0,0 +1,53 @@
+From b8f04a670f27a84412099dd025fa762ee58f4c1a Mon Sep 17 00:00:00 2001
+From: Shoaib Meenai <smeenai@fb.com>
+Date: Thu, 29 Jul 2021 18:39:04 -0700
+Subject: [PATCH] [builtins] Try to ensure single copy of emulated TLS state
+
+Multiple copies of emulated TLS state means inconsistent results when
+accessing the same thread-local variable from different shared objects
+(https://github.com/android/ndk/issues/1551). Making `__emutls_get_address`
+be a weak default visibility symbol should make the dynamic linker
+ensure only a single copy gets used at runtime. This is best-effort, but
+the more robust approach of putting emulated TLS into its own shared
+object would (a) be a much bigger change, and (b) shared objects are
+pretty heavyweight, and adding a new one to a space-constrained
+environment isn't an easy sell. Given the expected rarity of direct
+accesses to emulated TLS variables across different shared objects, the
+best-effort approach should suffice.
+
+Reviewed By: danalbert, rprichard
+
+Differential Revision: https://reviews.llvm.org/D107127
+---
+ compiler-rt/lib/builtins/emutls.c | 15 +++++++++++++++
+ 1 file changed, 15 insertions(+)
+
+diff --git a/compiler-rt/lib/builtins/emutls.c b/compiler-rt/lib/builtins/emutls.c
+index 98cabd917d6c..88af348c9e22 100644
+--- a/compiler-rt/lib/builtins/emutls.c
++++ b/compiler-rt/lib/builtins/emutls.c
+@@ -374,6 +374,21 @@ emutls_get_address_array(uintptr_t index) {
+   return array;
+ }
+
++#ifndef _WIN32
++// Our emulated TLS implementation relies on local state (e.g. for the pthread
++// key), and if we duplicate this state across different shared libraries,
++// accesses to the same TLS variable from different shared libraries will yield
++// different results (see https://github.com/android/ndk/issues/1551 for an
++// example). __emutls_get_address is the only external entry point for emulated
++// TLS, and by making it default visibility and weak, we can rely on the dynamic
++// linker to coalesce multiple copies at runtime and ensure a single unique copy
++// of TLS state. This is a best effort; it won't work if the user is linking
++// with -Bsymbolic or -Bsymbolic-functions, and it also won't work on Windows,
++// where the dynamic linker has no notion of coalescing weak symbols at runtime.
++// A more robust solution would be to create a separate shared library for
++// emulated TLS, to ensure a single copy of its state.
++__attribute__((visibility("default"), weak))
++#endif
+ void *__emutls_get_address(__emutls_control *control) {
+   uintptr_t index = emutls_get_index(control);
+   emutls_address_array *array = emutls_get_address_array(index--);
+-- 
+2.33.0.259.gc128427fd7-goog
+