Fix cmsg(3) bugs for musl and OSX #1235
Conversation
|
r? @gnzlbg (rust_highfive has picked a reviewer for you, use r? to override) |
|
Thank you for working on this @asomers. It appears that the tests are failing on some targets, maybe some more fine tuning is necessary. |
|
I'm pretty sure that the s390x and sparc64 failures are not my fault. On s390x the test program hangs. On sparc64, it gets SIGILL. In both cases, the problem happens before it invokes the first cmsg test function. It's not Travis's fault; I can duplicate the problem locally. It might be QEMU bugs. If nobody else has a better idea, I'll disable the test for those architectures. |
|
The two Android failures looks spurious; somebody should restart them. However, the s390x and sparc64 failures are more serious. They seem to be failing early in the test harness. Cargo doesn't provide a way to disable the entire test program on a per-target basis. So I'm stuck. If the decision were up to me; I would just remove those platforms from the test matrix. |
|
Can we land the fixes without the new test in the meantime? |
|
@bors: r+ |
|
📌 Commit 5b250ca has been approved by |
|
@asomers could you fill in a bug in rust-lang/rust about the sparc64 and s390x failures ? We are able to run a lot of tests in other rust-lang repos like packed_simd, jemallocator, etc. in these platforms, so I find it weird that they fail for something "this simple". |
|
@bors: retry |
|
@gnzlbg are you sure you want to merge this PR without disabling those Travis targets first? |
|
Actually, we definitely should not merge this PR without squashing first. bors r- |
|
@bors: r- (let me know when you rebase / squash) We should definitely figure out why these particular tests break on those targets (maybe a qemu bug, maybe a rustc/llvm bug), but we don't have to do that here. I don't think disabling all testing on those targets makes things better for these targets. So skipping these tests until we figure things out seems reasonable to me. |
|
Ok, now there's an ACTUAL failure on sparc64. But given how much time I've already spent on this, I'm inclined just to skip that test on sparc64 and open an issue for it. I'll let somebody who actually uses sparc64 and/or Linux fix it. The android failures are a trivial mistake. I'm locally investigating the s390x failure now. |
|
Ok @gnzlbg everything's passing now. I only had to disable the cmsg test on s390x and sparc64. Let me know when you're satisfied review-wise and I'll squash things. |
|
☔ The latest upstream changes (presumably #1217) made this pull request unmergeable. Please resolve the merge conflicts. |
|
The Cargo.lock needs updating. @alexcrichton should we just remove the |
This was an oversight from PR rust-lang#1212. It's been revealed by the new cmsg test.
This is an error from PR rust-lang#1098. The wrong definitions coincidentally work on Linux/glibc, but fail on Linux/musl.
Since these are defined in C as macros, they must be reimplemented in libc as Rust functions. They're hard to get exactly right, and they vary from platform to platform. The test builds custom C code that uses the real macros, and compares its output to the Rust versions' output for various inputs. Skip the CMSG_NXTHDR test on sparc64 linux because it hits a Bus Error. Issue rust-lang#1239 Skip the entire cmsg test program on s390x because it dumps core seemingly before the kernel finishes booting. Issue rust-lang#1240
|
Rebased, squashed, and fixed on OpenBSD. |
|
@bors: r+ |
|
📌 Commit 38cf5b1 has been approved by |
|
💔 Test failed - checks-travis |
|
@bors: retry |
|
☀️ Test successful - checks-cirrus, checks-travis, status-appveyor |
This PR fixes bugs in the cmsg(3) family of functions for Linux/musl and OSX, introduced by PR #1098 and PR #1212 . It also adds an integration test which hopefully will validate these functions on every platform.