From 50f87893f65d58739909c2ace165cb93408c4946 Mon Sep 17 00:00:00 2001 From: Pirama Arumuga Nainar Date: Wed, 1 Sep 2021 21:00:21 -0700 Subject: [PATCH] [patches] Cherry pick CLS for: Emulated TLS + shared-library breakage Bug: https://github.com/android/ndk/issues/1551 b8f04a670f2 [builtins] Try to ensure single copy of emulated TLS state Test: N/A Change-Id: I3d537a8415f428fafc4822b8bdbc33b70e09d261 --- patches/PATCHES.json | 8 ++- ...4a670f27a84412099dd025fa762ee58f4c1a.patch | 53 +++++++++++++++++++ 2 files changed, 60 insertions(+), 1 deletion(-) create mode 100644 patches/cherry/b8f04a670f27a84412099dd025fa762ee58f4c1a.patch diff --git a/patches/PATCHES.json b/patches/PATCHES.json index e947b93..d6ff305 100644 --- 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 } -] +] \ No newline at end of file diff --git a/patches/cherry/b8f04a670f27a84412099dd025fa762ee58f4c1a.patch b/patches/cherry/b8f04a670f27a84412099dd025fa762ee58f4c1a.patch new file mode 100644 index 0000000..9b14f7b --- /dev/null +++ b/patches/cherry/b8f04a670f27a84412099dd025fa762ee58f4c1a.patch @@ -0,0 +1,53 @@ +From b8f04a670f27a84412099dd025fa762ee58f4c1a Mon Sep 17 00:00:00 2001 +From: Shoaib Meenai +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 +