Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

boost: fix segfaults on ppc64 #101315

Merged
merged 1 commit into from Dec 13, 2020
Merged

Conversation

@r-burns
Copy link
Contributor

@r-burns r-burns commented Oct 22, 2020

(Previous attempt: #98822)

Fixes NixOS/nix#2517

See also:
boostorg/context#72
boostorg/fiber#193

These issues have been resolved by:
boostorg/context#106
boostorg/context@d4608a4
which is merged into boost as of v1.71.0.

This feature was introduced (with the bug) in
boost v1.61 and was fixed in v1.71. So we apply
the patch to all versions in that range.

Motivation for this change
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
@r-burns r-burns force-pushed the ppc64le-boost-segfault branch from 207a189 to ed384a3 Oct 22, 2020
Fixes NixOS/nix#2517

See also:
boostorg/context#72
boostorg/fiber#193

These issues have been resolved by:
boostorg/context#106
boostorg/context@d4608a4
which is merged into boost as of v1.71.0.

This feature was introduced (with the bug) in
boost v1.61 and was fixed in v1.71. So we apply
the patch to all versions in that range.
@r-burns r-burns force-pushed the ppc64le-boost-segfault branch from ed384a3 to ca89e80 Oct 22, 2020
@r-burns
Copy link
Contributor Author

@r-burns r-burns commented Oct 31, 2020

Pinging @CrystalGamma

@CrystalGamma
Copy link
Contributor

@CrystalGamma CrystalGamma commented Oct 31, 2020

Thanks @r-burns, I only recently got around to resurrecting NixOS on my Talos after around a year of being frustrated, but I see you've been doing good work for the platform in my absence :)

Anyway, I've definitely seen this bug breaking things, Nix itself (specifically nix copy IIRC, and thus the test suite) to be precise. After the version of boost used by Nix was bumped I didn't think it necessary to submit a PR because I wasn't aware that other applications were still using that version range (and I put my contribution on hold while the platform support discussion was going on, which took over a year, after which I was too annoyed to work on nixpkgs until a few days ago).

@r-burns
Copy link
Contributor Author

@r-burns r-burns commented Oct 31, 2020

Afaict nix uses the default boost, which is still 1.69, so I still get the segfaults without this patch. Getting nix working reliably seems like core functionality so I'm hoping to save others the same headache - I'm not aware of any other packages using boost-context but applying the patch liberally like this should fix them for free.

I'd like to thank you for laying the groundwork for ppc64le on nix, there's no way I could've gotten it up and running without all the work you did. If you're thinking about diving back in, I've been able to get most of the software I use up and running, so I'd be happy to share patches to avoid duplication of effort.

@CrystalGamma
Copy link
Contributor

@CrystalGamma CrystalGamma commented Oct 31, 2020

Oh right, I misremembered … it wasn't actually fixed upstream, I added the patch to my local overlay (in a really hacky way) and forgot about it. Adding the patch in nixpkgs proper like this sounds like a better medium-term solution (the long-term solution would of course be updating all the packages to newer versions of boost).

Thanks for the offer, but I've already gone through the 'revival' process for my installation a week or two ago. In fact, I was about to start PRing some of the changes I'm using, but then I rebased my nixpkgs branch and suddenly Firefox wouldn't build anymore (segfault while compiling a Rust component). Since both downgrading to the version used before the rebase and downgrading Rust by one version didn't 'fix' it I started a system-wide bisection (great that we can do this in NixOS, but it does take a long time, the better part of 3 days in my case, though that was mostly because x265 was broken for part of the range, so I needed to find the fix (which is where I saw your name first)). I was told that the real culprit is probably rust-lang/rust#74551 (and the reason why downgrading one version didn't stop the failure from occurring was that the Rust version used for firefox was changed directly from 1.44 to 1.46, skipping 1.45 which I tried, and which is also affected), the solution to which seems to be 'update LLVM' which I intend to test later today. /ramble

Anyway, looking forward to making Nix great on ppc64le together (and maybe even ppc64 too? maybe a fun little project when it works well on LE :D) If you don't mind, I'll be CCing you on my future PRs, so you can weigh in if you have a better solution for something.

@SuperSandro2000
Copy link
Member

@SuperSandro2000 SuperSandro2000 commented Oct 31, 2020

Would it be an option to only apply this patch on ppc64 to avoid the mass rebuild?

@r-burns
Copy link
Contributor Author

@r-burns r-burns commented Nov 25, 2020

Bump - any more thoughts on this? I still think applying the patch on all platforms is the most robust solution, but I'd settle for applying on ppc64 only if that's what it takes for this to land.

@SuperSandro2000
Copy link
Member

@SuperSandro2000 SuperSandro2000 commented Nov 25, 2020

Bump - any more thoughts on this? I still think applying the patch on all platforms is the most robust solution, but I'd settle for applying on ppc64 only if that's what it takes for this to land.

The patch most likely can't cause issues on other platforms cause they don't use the file. The current way is fine with me.

@r-burns
Copy link
Contributor Author

@r-burns r-burns commented Dec 13, 2020

@SuperSandro2000 would you be willing to merge this? Or I could solicit other reviewers if you think this needs more eyes

@SuperSandro2000
Copy link
Member

@SuperSandro2000 SuperSandro2000 commented Dec 13, 2020

The patch only changes a file which seems to be specific to ppc64 which should not affect any other platform.

@SuperSandro2000 SuperSandro2000 merged commit 7fdab2f into NixOS:staging Dec 13, 2020
19 checks passed
@r-burns r-burns deleted the ppc64le-boost-segfault branch Dec 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants