Allow Neutrino QNX SDK dependent values for PTHREAD_MUTEX_INITIALIZER
Description
This change tries to determine the value of a constant at compile time of libc: build.rs reads and "parses" the content of the C header file pthread.h. "Parse" means that it checks for the usage of other constants, which is sufficient for all observed cases.
Background:
In Neutrino QNX, some "constant" values get changed between releases of the toolchain, including in minor patch-versions. Whenever you get a new "QNX toolchain" (SDP), C and C++ developers know that they must recompile everything.
So far, I've only observed three variations, but there could be more that I'm not aware of. This is why I don't want to add and maintain a possibly long list of compiler targets, this would not work well.
Using this patch requires the environment variable QNX_TARGET which is always the case when compiling for QNX (required by the linker).
Conceptionally this change is similar to what FreeBSD is doing, but reads header file instead of running an external process (freebsd-version).
CC and todo list
CC: @jonathanpallant @japaric @gh-tr @AkhilTThomas
- Relevant tests in
libc-test/semverhave been updated: 👎 unfortunately impossible - [x] No placeholder or unstable values like
*LASTor*MAXare included (see #3131) - [x] Tested locally (
cd libc-test && cargo test --target mytarget); especially relevant for platforms that may not be checked in CI
r? @tgross35
rustbot has assigned @tgross35. They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.
Use r? to explicitly pick a reviewer
@flba-eb We had discussed this but it was not clear for me as to why we cannot define the values for these consts under a cfg flag for each of the variants. Like this as an example
cfg_if! {
if #[cfg(target_env= "nto71")] {
pub const PTHREAD_MUTEX_INITIALIZER: pthread_mutex_t = pthread_mutex_t {
__u: 0x80000000,
__owner: 0xffffffff,
};
}
else if #[cfg(target_env = "nto80")] {
pub const PTHREAD_MUTEX_INITIALIZER: pthread_mutex_t = pthread_mutex_t {
__u: _NTO_SYNC_NONRECURSIVE,
__owner: _NTO_SYNC_MUTEX_FREE,
};
}
else {
//...
}
}
@flba-eb We had discussed this but it was not clear for me as to why we cannot define the values for these consts under a cfg flag for each of the variants. Like this as an example
cfg_if! { if #[cfg(target_env= "nto71")] { pub const PTHREAD_MUTEX_INITIALIZER: pthread_mutex_t = pthread_mutex_t { __u: 0x80000000, __owner: 0xffffffff, }; } else if #[cfg(target_env = "nto80")] { pub const PTHREAD_MUTEX_INITIALIZER: pthread_mutex_t = pthread_mutex_t { __u: _NTO_SYNC_NONRECURSIVE, __owner: _NTO_SYNC_MUTEX_FREE, }; } else { //... } }
Well, basically this is what the change does, but it uses conditional compilation switches set by build.rs instead of target_env. To change target_env I would need to add a lot of compiler targets, and I wouldn't even know how to name them. Also, in this case only one value changed, but I would not be surprised if other versions (project specific) of QNX toolchains use other values and also change other constants. We would get an explosion of compiler targets.
I would prefer to determine the actual value in the build.rs script and use it in the libc code, but this does not work:
- Conditional compilation cannot "transport" data (like a C preprocessor definition)
- Generating Rust code would work theoretically (I've tried it) but will break libc's test framework (which parses the Rust code to generate C code from it).
Well, basically this is what the change does, but it uses conditional compilation switches set by
build.rsinstead oftarget_env. To changetarget_envI would need to add a lot of compiler targets, and I wouldn't even know how to name them. Also, in this case only one value changed, but I would not be surprised if other versions (project specific) of QNX toolchains use other values and also change other constants. We would get an explosion of compiler targets.I would prefer to determine the actual value in the build.rs script and use it in the libc code, but this does not work:
- Conditional compilation cannot "transport" data (like a C preprocessor definition)
- Generating Rust code would work theoretically (I've tried it) but will break libc's test framework (which parses the Rust code to generate C code from it).
IMHO this is still not a scalable solution as the other structs will have different number and named elements and hence users will need to extend the parser to more elements. I'd prefer that the consts/structs are divided into modules specific files like.
unix>nto>
├── aarch64.rs
├── mod.rs
├── neutrino.rs
├── nto7071
│ └── mod.rs
├── nto80
│ └── mod.rs
└── x86_64.rs
Well, basically this is what the change does, but it uses conditional compilation switches set by
build.rsinstead oftarget_env. To changetarget_envI would need to add a lot of compiler targets, and I wouldn't even know how to name them. Also, in this case only one value changed, but I would not be surprised if other versions (project specific) of QNX toolchains use other values and also change other constants. We would get an explosion of compiler targets. I would prefer to determine the actual value in the build.rs script and use it in the libc code, but this does not work:
- Conditional compilation cannot "transport" data (like a C preprocessor definition)
- Generating Rust code would work theoretically (I've tried it) but will break libc's test framework (which parses the Rust code to generate C code from it).
IMHO this is still not a scalable solution as the other structs will have different number and named elements and hence users will need to extend the parser to more elements. I'd prefer that the consts/structs are divided into modules specific files like.
unix>nto> ├── aarch64.rs ├── mod.rs ├── neutrino.rs ├── nto7071 │ └── mod.rs ├── nto80 │ └── mod.rs └── x86_64.rs
I don't want to determine all values at compile time (in build.rs). Only when necessary, e.g. between different "7.1" versions. For 7.0, 7.1 and 8.0 it should be perfectly fine to hard-code all remaining values (>99%). If it makes sense to store them in different more specific module files then let's do it! For now, I would like to only have the mutex initializer value determined at compile time. Maybe in one or two years, we have two or three such values, but hopefully not more.
I don't want to determine all values at compile time (in build.rs). Only when necessary, e.g. between different "7.1" versions. For 7.0, 7.1 and 8.0 it should be perfectly fine to hard-code all remaining values (>99%). If it makes sense to store them in different more specific module files then let's do it! For now, I would like to only have the mutex initializer value determined at compile time. Maybe in one or two years, we have two or three such values, but hopefully not more.
In that case i agree :) Once the libc PR for iosock lands i'll start with the splitting into os version specific modules.
I don't think we can go with this approach: this isn't robust against #ifs and I don't think we really want libc to start reading system headers anyway.
Well, basically this is what the change does, but it uses conditional compilation switches set by
build.rsinstead oftarget_env. To changetarget_envI would need to add a lot of compiler targets, and I wouldn't even know how to name them. Also, in this case only one value changed, but I would not be surprised if other versions (project specific) of QNX toolchains use other values and also change other constants. We would get an explosion of compiler targets.
Could you elaborate this a bit - do you mean that e.g. within the -qnx700 target this value may change depending on minor version (7.0.0 vs. 7.0.1)?
There is some recent discussion on breaking changes within a target string, if this applies here then it would be good if you could comment on that thread https://rust-lang.zulipchat.com/#narrow/channel/219381-t-libs/topic/How.20to.20handle.20breaking.20changes.20in.20libc.201.2E0
I don't think we can go with this approach: this isn't robust against
#ifs and I don't think we really wantlibcto start reading system headers anyway.Well, basically this is what the change does, but it uses conditional compilation switches set by
build.rsinstead oftarget_env. To changetarget_envI would need to add a lot of compiler targets, and I wouldn't even know how to name them. Also, in this case only one value changed, but I would not be surprised if other versions (project specific) of QNX toolchains use other values and also change other constants. We would get an explosion of compiler targets.Could you elaborate this a bit - do you mean that e.g. within the
-qnx700target this value may change depending on minor version (7.0.0 vs. 7.0.1)?
It can be even more subtle than that. In one project, we have a project specific version like myproject-bla-bla2-1-4-8-0-r00006.1 which got updated to myproject-bla-bla2-1-5-3-0-r00008.2. The first version uses the same value for PTHREAD_RMUTEX_INITIALIZER as the official 7.1 version does, but the second version uses a different value. Both call themselves "QNX 7.1".
I am fully aware that my "parsing" approach is error-prone, and I've considered calling the C preprocessor instead and parsing it's output. However, I've decided against that because:
- I get a value that I cannot easily get into the libc source code (conditional compilation does not allow that, only features like "value_is_80" or "value_is_82".
- It should not matter, as the QNX users should compile and run the libc test application anyway each time their QNX toolchain is updated. In case my parsing is wrong (or any other value has changed), the test application will detect it.
There is some recent discussion on breaking changes within a target string, if this applies here then it would be good if you could comment on that thread https://rust-lang.zulipchat.com/#narrow/channel/219381-t-libs/topic/How.20to.20handle.20breaking.20changes.20in.20libc.201.2E0
I will comment there.
As an alternative option, build.rs could compile a simple C file which defines a global variable and extract the value with objdump. Something like:
#include <pthread.h>
pthread_mutex_t _PTHREAD_MUTEX_INITIALIZER = PTHREAD_MUTEX_INITIALIZER;
$ qcc -Vgcc_ntoaarch64le -c check.c check.o && ntoaarch64-objdump check.o -d -j .data
check.o: file format elf64-littleaarch64
Disassembly of section .data:
0000000000000000 <_PTHREAD_MUTEX_INITIALIZER>:
0: 00 00 00 80 ff ff ff ff ........
Then build.rs could parse the output... Would this be acceptable?
I'm sorry for not following up here; I think this probably would have been a good candidate for Zulip discussion.
Here is my stance on a few different approaches:
-
Parsing the header files ourselves I unfortunately have to say is a no-go: it's not a simple thing to have and maintain in libc's source, and it's too easy to break by something like a trivial
#ifdefadded to the Neutrino source code. -
Doing a check build seems better but I'm iffy because that probably means we need to expose cc config somehow. You'd need to bring this up on the t-libs Zulip and convince the others that this this is reasonable to be doing in libc.
If this winds up being the approach, it should be possible to get the info from the compiler invoked with
-E -dMrather than checking an object file. -
My preferred option would be checking an environment variable that can be set as part of a build from another build system. Or querying a
tell-me-the-qnx-configCLI util if such a thing exists, though that has some of the same problems as building a C file. We already have these kind of approaches for a few different targets.
If QNX needs more than that or if there is the possibility that we need more of these options in the future, I think you may want to look into creating a dedicated crate for QNX bindings that allows for the needed config (having maintainers that have access to and familiarity with QNX would probably be pretty helpful). We already have a handful of such target-specific crates used by std, e.g. fortanix-sgx-abi, hermit-abi, r-efi, wasi, etc.
@rustbot author
Reminder, once the PR becomes ready for a review, use @rustbot ready.