[PATCH] D154397: [Driver] Default to -ftls-model=initial-exec on SerenityOS

[Daniel Bertalan via Phabricator via cfe-commits]

BertalanD added a comment.

Hi MaskRay!

Your blog post (alongside the Drepper TLS doc) proved to be a very helpful 
resource for our implementation of the dynamic TLS interface. I currently have 
a SerenityOS PR  open that 
implements `__tls_get_addr` and TLSDESC on top of our current TLS 
infrastructure which only supports static TLS blocks. (Long story short, our 
loader passes the initial TLS image to the kernel, which automagically copies 
it over and sets up the thread pointer whenever a new thread is created. We 
need to move it to the userspace if we want to have a dynamic thread vector and 
separately allocated TLS blocks). I'd love it if you'd take a look at the PR.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154397/new/

https://reviews.llvm.org/D154397

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154397: [Driver] Default to -ftls-model=initial-exec on SerenityOS

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

I don't think this belongs to the upstream, either. Note: musl's TLS 
implementation is very clean and  may be a good reference.

https://maskray.me/blog/2021-02-14-all-about-thread-local-storage

> glibc ld.so reserves some space in static TLS blocks and allows dlopen on 
> such a shared object if its TLS size is small. There could be an obscure 
> reason for using such an attribute: general dynamic and local dynamic TLS 
> models are not async-signal-safe in glibc. However, other libc 
> implementations may not reserve additional TLS space for dlopen'ed 
> initial-exec shared objects, e.g. musl will error.

glibc's model makes it very error-prone, even if dlopen TLS is supported to 
some extent.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154397/new/

https://reviews.llvm.org/D154397

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154397: [Driver] Default to -ftls-model=initial-exec on SerenityOS

[Andrew Kaster via Phabricator via cfe-commits]

ADKaster abandoned this revision.
ADKaster added a comment.

In D154397#4470094 , @jrtc27 wrote:

> This is pretty dodgy, I don't think it belongs upstream

Fair enough, the original patch had "probably don't upstream this" in the 
commit description. @BertalanD mentioned to me on discord that he had a WIP 
patch for proper TLS on x86_64 and aarch64 cooking already . I'll drop this one 
and by the time the series lands we'll most likely have the TLS support in 
serenity already (or we can add it to the default clang args we already use for 
setting --sysroot for Ports).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154397/new/

https://reviews.llvm.org/D154397

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154397: [Driver] Default to -ftls-model=initial-exec on SerenityOS

[Jessica Clarke via Phabricator via cfe-commits]

jrtc27 added a comment.

This is pretty dodgy, I don't think it belongs upstream


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154397/new/

https://reviews.llvm.org/D154397

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154397: [Driver] Default to -ftls-model=initial-exec on SerenityOS

[Andrew Kaster via Phabricator via cfe-commits]

ADKaster created this revision.
Herald added a project: All.
ADKaster requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.

Force Clang use the initial-exec TLS model instead of the default
local-exec when building code for Serenity.

This patch should be removed when the SerenityOS program loader
implements proper TLS support.

Depends on D154396 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D154397

Files:
  clang/lib/Driver/ToolChains/Clang.cpp


Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -6170,7 +6170,13 @@
   Args.AddLastArg(CmdArgs, 
options::OPT_fvisibility_inlines_hidden_static_local_var,

options::OPT_fno_visibility_inlines_hidden_static_local_var);
   Args.AddLastArg(CmdArgs, options::OPT_fvisibility_global_new_delete_hidden);
-  Args.AddLastArg(CmdArgs, options::OPT_ftlsmodel_EQ);
+  if (Triple.isOSSerenity()) {
+StringRef tls_model =
+Args.getLastArgValue(options::OPT_ftlsmodel_EQ, "initial-exec");
+CmdArgs.push_back(Args.MakeArgString("-ftls-model=" + tls_model));
+  } else {
+Args.AddLastArg(CmdArgs, options::OPT_ftlsmodel_EQ);
+  }
 
   if (Args.hasFlag(options::OPT_fnew_infallible,
options::OPT_fno_new_infallible, false))


Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -6170,7 +6170,13 @@
   Args.AddLastArg(CmdArgs, options::OPT_fvisibility_inlines_hidden_static_local_var,
options::OPT_fno_visibility_inlines_hidden_static_local_var);
   Args.AddLastArg(CmdArgs, options::OPT_fvisibility_global_new_delete_hidden);
-  Args.AddLastArg(CmdArgs, options::OPT_ftlsmodel_EQ);
+  if (Triple.isOSSerenity()) {
+StringRef tls_model =
+Args.getLastArgValue(options::OPT_ftlsmodel_EQ, "initial-exec");
+CmdArgs.push_back(Args.MakeArgString("-ftls-model=" + tls_model));
+  } else {
+Args.AddLastArg(CmdArgs, options::OPT_ftlsmodel_EQ);
+  }
 
   if (Args.hasFlag(options::OPT_fnew_infallible,
options::OPT_fno_new_infallible, false))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits