Allow specifying the ELF TLS ABI #132480
Allow specifying the ELF TLS ABI #132480
Conversation
TLSDESC is an alternate method of TLS access. It is part of the ELF TLS ABI, and is already commonly used in other toolchains. LLVM supports this for most ELF backends (X86_64, Aarch64, RISC-V). It has always been the default for Aarch64, but support for RISC-V and X86_64 were added before the last LLVM release. More information on TLSDESC can be found in: https://android.googlesource.com/platform/bionic/+/HEAD/docs/elf-tls.md or the original RFC: https://www.fsfla.org/~lxoliva/writeups/TLS/RFC-TLSDESC-ARM.txt This patch adds a new unstable option `-Z tls-dialect`, which matches the common `-mtls-dialect=` option used in Clang, GCC, and other toolchains. This option only needs to be passed through to LLVM to update the code generation. For targets like Aarch64 that always use TLSDESC, this has no effect. Eventually this flag should probably be stabilized since it is an important part of a program's ABI. In the future, if LLVM changes its defaults for TLS to enable TLSDESC for a particular platform or architecture, users who do not wish to change can use the new option to select their desired TLS ABI.
rustbot
added
S-waiting-on-review
|
These commits modify compiler targets. |
|
r? @tmandry |
|
Also, I'm open to testing this differently, but I'm not aware of any way to be sure the codegen flags are set appropriately, other than to see if LLVM spits out the right bits, since there isn't anything to check in the LLVM IR. |
|
The job Click to see the possible cause of the failure (guessed by this bot) |
| @@ -2114,6 +2123,9 @@ written to standard error output)"), | |||
| #[rustc_lint_opt_deny_field_access("use `Session::tls_model` instead of this field")] | |||
| tls_model: Option<TlsModel> = (None, parse_tls_model, [TRACKED], | |||
| "choose the TLS model to use (`rustc --print tls-models` for details)"), | |||
| #[rustc_lint_opt_deny_field_access("use `Session::tls_model` instead of this field")] | |||
| @@ -825,6 +825,7 @@ pub enum PrintKind { | |||
| RelocationModels, | |||
| CodeModels, | |||
| TlsModels, | |||
| TlsDialect, | |||
There was a problem hiding this comment.
Why is this added? Do you have any actual need to know which tls dialect will ne used by rustc on a given target?
There was a problem hiding this comment.
Also it should probably be made available on nightly only so we can remove it again once the traditional tls dialect is no longer used anywhere.
There was a problem hiding this comment.
Why is this added? Do you have any actual need to know which tls dialect will ne used by rustc on a given target?
I followed the example for TlsModels, since they seem of equivalent diagnostic value to me. For Android RISC-V targets TLSDESC is the default, and I think it may be a requirement.
Also it should probably be made available on nightly only so we can remove it again once the traditional tls dialect is no longer used anywhere.
I'm not sure that will ever happen. TLSDESC requires support in the dynamic linker, so its imaginable that not all libcs will support that. For instance I've seen embedded targets that implement something like a loader service that support __tls_get_addr with overlays, but do not support TLSDESC.
At any rate, I'm not sure we will ever drop support for traditional TLS in LLVM. It's been discussed, but I think its unlikely until GNU toolchains drop support.
There was a problem hiding this comment.
I'm not sure that will ever happen. TLSDESC requires support in the dynamic linker, so its imaginable that not all libcs will support that.
It can be the default on targets that do support it and not having an option to enable it on targets that don't support it.
At any rate, I'm not sure we will ever drop support for traditional TLS in LLVM. It's been discussed, but I think its unlikely until GNU toolchains drop support.
I mean rustc dropping support for configuring it to anything other than the default for the target.
There was a problem hiding this comment.
I'm not sure that will ever happen. TLSDESC requires support in the dynamic linker, so its imaginable that not all libcs will support that.
It can be the default on targets that do support it and not having an option to enable it on targets that don't support it.
I'm not sure how you can make that determination, since you can't know which libc someone is using ahead of time. Being able to configure and opt in/out is necessary, IMO.
At any rate, I'm not sure we will ever drop support for traditional TLS in LLVM. It's been discussed, but I think its unlikely until GNU toolchains drop support.
I mean rustc dropping support for configuring it to anything other than the default for the target.
Failing to expose a code generation option doesn't sound right to me. There are valid reasons someone may want to select a particular variant of an option differently than the default. Until LLVM (and I guess all backends?) drops support altogether, I'm not sure its a good idea for rustc to stop exposing that option altogether.
There was a problem hiding this comment.
If there is a chance that LLVM ever drops the option, then no, we should not expose the option.
AFAIK, LLVM has no plans to drop support. Its hard to say never, but as a mater of practicality, I doubt it will be removed any time soon. If it does, we'd probably have a long period where its listed as "deprecated". Even then its possible it wouldn't actually be removed. There is precedent for that.
There will be options to pass arguments directly to codegen backends, and if those suffice, then great, but otherwise we should resist getting into the habit of gaslighting our users by tricking them into thinking they have power and then not being able to fulfill their requests later because of the whims of those codegen backends.
In general, passing -mllvm options often fails for LTO builds, because frontends tend to fail to pass those along correctly to the linker via -Wl,-mllvm,-option. This was particularly painful for RISC-V targets whose target features were not propagated to the linker correctly, and required extra build plumbing to solve. It's arguably a failing of LLVM for not encoding those options in the IR. How to solve this class of problems has been being discussed for a long time now, and I don't think we'll arrive at a solution any time soon.
It would be good to spell those out.
First, this comment was about compiler options in general. Defaults are usually chosen to make sense for the most common software. Projects may want to leverage knowledge they have about their system to improve performance, security, reduce size, etc. It's quite hard to guess what requirements people may have, and failing to expose options that allows them to deviate from the default feels like the wrong approach for a compiler.
For TLS dialects, I'd say performance is the most obvious motivation. Another thing to consider is that you may want to support different configurations for the same target triple. For instance, Android after some particular SDK version may want to drop TLSDESC support, but you may need to retain the ability to target earlier SDKs that predate TLSDESC support in bionic. Similarly, we know that Fuchsia's driver ABI will only support TLSDESC, but the rest of the system will likely be more flexible about supporting both TLSDESC and traditional TLS.
There was a problem hiding this comment.
For instance, Android after some particular SDK version may want to drop TLSDESC support, but you may need to retain the ability to target earlier SDKs that predate TLSDESC support in bionic.
Is Android planning to drop support for not using TLSDESC before the last LTS NDK that doesn't support TLSDESC is no longer supported? We only support the most recent LTS NDK.
Similarly, we know that Fuchsia's driver ABI will only support TLSDESC, but the rest of the system will likely be more flexible about supporting both TLSDESC and traditional TLS.
If Fuchsia has a different ABI between regular userspace and drivers, it would make sense to have separate targets, right? Does anyone care about supporting Fuchsia versions from before TLSDESC got introduced in the first place?
There was a problem hiding this comment.
It's quite hard to guess what requirements people may have, and failing to expose options that allows them to deviate from the default feels like the wrong approach for a compiler.
This argument generalizes too much. It could mean that every single decision the compiler could make should have a user-facing option.
Compiler flags are part of our user interface. Which means they need testing, we need to consider compatibility, ABI unsoundness, terrible linker errors and so on. So adding one should come with a stronger motivation than "it'd be nice to have a knob, even though there's no immediate need".
As bjorn says it's possible that this decision is better left to a combination of target specifications and the compiler checking if it's supported (e.g. based on LLVM version) and determining the correct setting based on that.
There was a problem hiding this comment.
For instance, Android after some particular SDK version may want to drop TLSDESC support, but you may need to retain the ability to target earlier SDKs that predate TLSDESC support in bionic.
Is Android planning to drop support for not using TLSDESC before the last LTS NDK that doesn't support TLSDESC is no longer supported? We only support the most recent LTS NDK.
I doubt it, but I'm not an Android maintainer and the precise details are outside of my area of expertise. I mentioned this, since similar concerns were brought up on the LLVM side w.r.t. emulated TLS and the NDK's need to maintain support for it until the NDK versions that used it were no longer supported. Those time horizons were typically long, IIRC. If Rust has a support story here tied to the NDK then maybe the concern I raised is moot.
Similarly, we know that Fuchsia's driver ABI will only support TLSDESC, but the rest of the system will likely be more flexible about supporting both TLSDESC and traditional TLS.
If Fuchsia has a different ABI between regular userspace and drivers, it would make sense to have separate targets, right?
Drivers in Fuchsia are still user-level programs, and I'm fairly sure we don't want to introduce another triple for them.
Does anyone care about supporting Fuchsia versions from before TLSDESC got introduced in the first place?
Fuchsia was brought up as an example of a scenario where its useful to have such knobs. I don't think the focus should be on Fuchsia specifically. FWIW, though, we do certainly support older versions of our SDK, though those are usually tied to a specific toolchain and set of libraries.
There was a problem hiding this comment.
It's quite hard to guess what requirements people may have, and failing to expose options that allows them to deviate from the default feels like the wrong approach for a compiler.
This argument generalizes too much. It could mean that every single decision the compiler could make should have a user-facing option.
There's an argument that it should, even if its not easy to access. Not all software is created equal, and not all compiler decisions are appropriate everywhere. I'm not suggesting that be made the case, just that in many cases not leaving a escape hatch for an optimization leads to worse performance, code size, or a security problem that has to be worked around.
Compiler flags are part of our user interface. Which means they need testing, we need to consider compatibility, ABI unsoundness, terrible linker errors and so on. So adding one should come with a stronger motivation than "it'd be nice to have a knob, even though there's no immediate need".
I don't view this as "nice to have," I view it as a requirement. Given the history of these options, it seems irrational to fail to give developers the choice of what to do if their needs differ from the default. I'm fine to leave the disagreement there, but I'm not doing this work because projects I support don't need it. Clearly the fact that I'm implementing it suggests some level of need.
As bjorn says it's possible that this decision is better left to a combination of target specifications and the compiler checking if it's supported (e.g. based on LLVM version) and determining the correct setting based on that.
I disagree. IMO, setting defaults based on the triple is sensible, but locking users in is not, especially when the backend supports both.
| @@ -2114,6 +2123,9 @@ written to standard error output)"), | |||
| #[rustc_lint_opt_deny_field_access("use `Session::tls_model` instead of this field")] | |||
| tls_model: Option<TlsModel> = (None, parse_tls_model, [TRACKED], | |||
| "choose the TLS model to use (`rustc --print tls-models` for details)"), | |||
| #[rustc_lint_opt_deny_field_access("use `Session::tls_model` instead of this field")] | |||
| tls_dialect: Option<TlsDialect> = (None, parse_tls_dialect, [TRACKED], | |||
There was a problem hiding this comment.
Maybe name this -Zuse-tlsdesc and have it take a bool? I don't suppose we get another dialect before the transition from traditional to tlsdesc TLS support is finished, right?
There was a problem hiding this comment.
I'm OK w/ that, but I think its usually better to support an existing spelling when there are existing conventions in other compilers. We'd have to check w/ folks doing the GCC backend work, but its possible the variants would be easier to integrate for them.
We've talked about adding support for a 3 GOT slot variant of TLSDESC. That would require support from the linker and libc. There are advantages to doing it that way, like avoiding a level of indirection to read the descriptor for the dynamic cases, but I'm not sure how likely that is to ever happen. I'd like to see it, and was hoping that would be the ABI for RISC-V, but it would be a win for any architecture.
But I can change this to a boolean if you'd like.
Not quite. You did still have to deal with a precompiled standard library that uses TLSDESC in that case. I think we did have to keep TLSDESC disabled by default until the minimum supported libc and linker for the target support TLSDESC, at which point nobody needs to disable TLSDESC usage. |
Does that mean that all compilation units in a binary need to use the same TLS dialect, or otherwise it would be unsound? If that is the case, this is should use the target modifier mechanism of rust-lang/rfcs#3716 once that exists. In the meantime, that should at least be documented clearly (as it may also need |
|
☔ The latest upstream changes (presumably #125579) made this pull request unmergeable. Please resolve the merge conflicts. |
Today, if the standard library is compiled w/ whatever LLVM says is the default, and you have no mechanism to opt out of it. Users should be able to choose, even if they need to compile their own standard library. There also isn't a problem mixing traditional TLS and TLSDESC so long as the target libc supports both. If it only supports one, then you're kind of out of luck. That's one reason I don't expect this option to go away: you may need to target a system that only supports one specific flavor of TLS. For example, Fuchsia's driver ABI is only expected to work w/ TLSDESC, and they won't have access to |
In general, no. Most libc implementations support both. That isn't universally true, but they aren't incompatible to use together. Whether both are supported by the libc is a different question.
I don't think that's required here. This is a different issue than something like mixing RISC-V atomics ABIs or Shadow Call Stack implementations, where you're mixing objects that are fundamentally incompatible/broken. The dynamic linker should issue an error about an unsupported relocation type when loading a module if it doesn't support TLSDESC. Similarly, you should get some kind of linker error if you use traditional TLS and there isn't an implementation of |
We would only make TLSDESC the default once we bump the minimal libc version we need to one that always supports TLSDESC. At that point even disabling TLSDESC would not create a working executable for the target anymore as we did also start to depend on api's that older libc versions don't support. |
should. does it always? is this hypothesis or verified experiment? |
If you need to target a system that only supports one specific flavor of TLS, then all adding a flag to the CLI does is expose a way to get things wrong. If you need to generate a code for such a unique target tuple, then you probably should be generating code for a unique target tuple, instead of trying to pretend some other target adequately describes what you need. |
There was a problem hiding this comment.
We support non-ELF targets. This option only affects ELF targets. Not all the world is Linux. Please add a UI test that verifies this compiler option is rejected and compilation ceases if you try to use it for a non-ELF target. This will become more important if any other object formats adopt a similar "dialect" but don't implement it exactly the same.
Bionic, musl, and glibc do. I can't speak to every implementation, though. |
Targets often support multiple variations, and users should be able to pick which one they use. Adding an extra tuple for every combination of potential features doesn't sound right, but maybe I'm misunderstanding what you're saying? For targets that can't/won't support this at all (e.g. non-ELF targets), an error from the frontend sounds like a reasonable approach.
Thanks for reminding me, I had meant to add something along those lines for COFF and MachO, and forgot. |
I agree with that as well. Perhaps I am just feeling skeptical as to what model you are thinking of, here, of a "target that only supports a specific TLS dialect", that it requires such configuration? I think it's very important to actually understand the model of the actual usage, instead of exposing compiler flags "just in case", because some of rustc's compiler options have been exposed but then later proved to behave in ways that break, harshly, with the model of some actual users of the compiler. For instance, if a target doesn't support a TLS dialect, but is ELF, and we know it doesn't support it, should we error on the clash? Prefer the target's value and silently ignore the user's request? Pass it through and just say "que sera, que sera", even though the codegen may then be invalid? What will people actually want? Ideally, we do not guess as to which people will later expect. Maybe the primary importance of this is only for our internal documentation so we know what model we're trying to uphold. |
Also, to be clear, we don't have to answer this right now, I'm just pointing out that this is a question that can in fact be answered different ways, so we should try to actually make note of an unresolved question. |
There are going to be subsets of Fuchsia that will have different requirements than we're currently planning to impose on other parts of the system. As mentioned earlier, Fuchsia drivers will be required to use TLSDESC, and those won't have a different target triple. Right now, the plan of record is that other components probably should use TLSDESC too, but won't be required (though that is still TBD). My understanding of Rust support in Android and its relation it the NDK is very incomplete, but I think they'll need to support both options depending on the SDK being targeted. Do take that with a grain of salt, as I could easily be incorrect about how those things intersect for Android specifically. Lastly, I've seen embedded systems that don't have a libc, or at least don't use one with TLS support, but have implemented TLS support themselves. Those systems need to be able to select their code generation but the target triple used is typically something a bit generic for baremetal systems. Since they're providing support outside of a libc(or on in addition to one), the triple also isn't informative about the code generation requirements.
I don't know that there's a whole lot you can do about that problem, at least in the general case. Compilers often expose powerful options to users, and sometimes those conflict. I've seen this happen the most frequently with security related options, but there are certainly code generation options that may violate some invariants that people rely upon. Its hard to know how to balance those exactly, since there are bound to be users that need/would greatly benefit from those options, but using them correctly requires a certain degree of familiarity with the low level details of the program and its invariants. Other that better documentation and outreach I'm not sure we can do all that much to satisfy the expert user and not introduce a potential foot gun for other users.
I'm far from an authority on how compiler UX, but if we know for sure, then I'd say "yes, we should emit an error." If LLVM knows its incompatible, its going to error anyway, just much later. It's always better to issue such an error in the frontend if you can, and I'd expect the frontend to behave the same way as it does for non-ELF targets. "Knows" is doing some heavy lifting here though. |
Using TLSDESC for the entire system should be fine, right? I presume there are no components that must not use TLSDESC and at least as far as I can tell as outsider, there is no upstream support for Fuchsia versions older than the latest one and nobody outside of Google depends on using old Fuchsia versions either, so having the first version of Fuchsia that supports TLSDESC as minimum supported version on the rust side should be fine, thus enabling the fuchsia rust target to always enable TLSDESC.
We don't have stable support for TLS on any baremetal targets at all. All stable TLS usage goes through the |
I don't think that is for a toolchain to decide. We're not prepared to make that determination yet for our entire platform and the ABIs we support. If we do, then sure, maybe that's fine for Fuchsia. But Fuchsia isn't the only platform that can use the option.
I'm not familiar with target spec json mechanism, so I'll have to look at that. What I'm saying is that there are real needs in this space to be able to generate code this way for real products. |
The
The reason I stated all those as options we may select between is because without documenting such, a reviewer can wind up approving a PR that subtly changes the behavior in ways users don't expect and was insufficiently tested. It may change to directly contradict its previous behavior in some cases. Then, if not many people are using the combination of circumstances that changed behavior, and they aren't inspecting the assembly very closely, it may be that no one reports a regression for years. Then, when someone finally does report it, this can make everyone unhappy if we even discuss a revert of what was thought to be a stable behavior and has become relied on for some time. This has happened to rustc for poorly-documented compiler options before, and you bet it has happened to the C compilers! The best way to prevent this is often to actually write down the spec instead of "I dunno, whatever gcc or clang does I guess?" so that there is an intention to extrapolate from when later combinations come up. |
TLSDESC is an alternate method of TLS access. It is part of the ELF TLS ABI, and is already commonly used in other toolchains. LLVM supports this for most ELF backends (X86_64, Aarch64, RISC-V). It has always been the default for Aarch64, but support for RISC-V and X86_64 were added before the last LLVM release.
More information on TLSDESC can be found in:
https://android.googlesource.com/platform/bionic/+/HEAD/docs/elf-tls.md or the original RFC:
https://www.fsfla.org/~lxoliva/writeups/TLS/RFC-TLSDESC-ARM.txt
This patch adds a new unstable option
-Z tls-dialect, which matches the common-mtls-dialect=option used in Clang, GCC, and other toolchains. This option only needs to be passed through to LLVM to update the code generation. For targets like Aarch64 that always use TLSDESC, this has no effect. Eventually this flag should probably be stabilized since it is an important part of a program's ABI.In the future, if LLVM changes its defaults for TLS to enable TLSDESC for a particular platform or architecture, users who do not wish to change can use the new option to select their desired TLS ABI.