On non-musl Linux, strerror_r should be linked to __xpg_strerror_r #253
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: NODA, Kai <nodakai@gmail.com>
It was updated recently, so the test is known to fail depending on the libc version of the test environment. Signed-off-by: NODA, Kai <nodakai@gmail.com>
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
|
⌛ Testing commit 4d1efd9 with merge fb2f0bb... |
bors
added a commit
that referenced
this pull request
Apr 3, 2016
On non-musl Linux, strerror_r should be linked to __xpg_strerror_r Currently `libc::strerror_r()` wrongly returns a `c_char` pointer as a large `c_int`. Also exclude `MS_RMT_MASK` from `libc-test`.
|
☀️ Test successful - status-appveyor, travis |
danielverkamp
pushed a commit
to danielverkamp/libc
that referenced
this pull request
Apr 28, 2020
* Move from #[inline(always)] to #[inline] This commit blanket changes all `#[inline(always)]` annotations to `#[inline]`. Fear not though, this should not be a regression! To clarify, though, this change is done out of correctness to ensure that we don't hit stray LLVM errors. Most of the LLVM intrinsics and various LLVM functions we actually lower down to only work correctly if they are invoked from a function with an appropriate target feature set. For example if we were to out-of-the-blue invoke an AVX intrinsic then we get a [codegen error][avx-error]. This error comes about because the surrounding function isn't enabling the AVX feature. Now in general we don't have a lot of control over how this crate is consumed by downstream crates. It'd be a pretty bad mistake if all mistakes showed up as scary un-debuggable codegen errors in LLVM! On the other side of this issue *we* as the invokers of these intrinsics are "doing the right thing". All our functions in this crate are tagged appropriately with target features to be codegen'd correctly. Indeed we have plenty of tests asserting that we can codegen everything across multiple platforms! The error comes about here because of precisely the `#[inline(always)]` attribute. Typically LLVM *won't* inline functions across target feature sets. For example if you have a normal function which calls a function that enables AVX2, then the target, no matter how small, won't be inlined into the caller. This is done for correctness (register preserving and all that) but is also how these codegen errors are prevented in practice. Now we as stdsimd, however, are currently tagging all functions with "always inline this, no matter what". That ends up, apparently, bypassing the logic of "is this even possible to inline". In turn we start inlining things like AVX intrinsics into functions that can't actually call AVX intrinsics, creating codegen errors at compile time. So with all that motivation, this commit switches to the normal inline hints for these functions, just `#[inline]`, instead of `#[inline(always)]`. Now for the stdsimd crate it is absolutely critical that all functions are inlined to have good performance. Using `#[inline]`, however, shouldn't hamper that! The compiler will recognize the `#[inline]` attribute and make sure that each of these functions is *candidate* to being inlined into any and all downstream codegen units. (aka if we were missing `#[inline]` then LLVM wouldn't even know the definition to inline most of the time). After that, though, we're relying on LLVM to naturally inline these functions as opposed to forcing it to do so. Typically, however, these intrinsics are one-liners and are trivially inlineable, so I'd imagine that LLVM will go ahead and inline everything all over the place. All in all this change is brought about by rust-lang#253 which noticed various codegen errors. I originally thought it was due to ABI issues but turned out to be wrong! (although that was also a bug which has since been resolved). In any case after this change I was able to get the example in rust-lang#253 to execute in both release and debug mode. Closes rust-lang#253 [avx-error]: https://play.rust-lang.org/?gist=50cb08f1e2242e22109a6d69318bd112&version=nightly * Add inline(always) on eflags intrinsics Their ABI actually relies on it! * Leave #[inline(always)] on portable types They're causing test failures on ARM, let's investigate later.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently
libc::strerror_r()wrongly returns ac_charpointer as a largec_int.Also exclude
MS_RMT_MASKfromlibc-test.