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

openssh: inreplace to apply workaround to Apple clang 14.0.3 #135373

Merged
merged 2 commits into from Jun 30, 2023

Conversation

cho-m
Copy link
Member

@cho-m cho-m commented Jun 30, 2023

  • Have you followed the guidelines for contributing?
  • Have you ensured that your commits follow the commit style guide?
  • Have you checked that there aren't other open pull requests for the same formula update/change?
  • Have you built your formula locally with brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
  • Is your test running fine brew test <formula>, where <formula> is the name of the formula you're submitting?
  • Does your build pass brew audit --strict <formula> (after doing brew install --build-from-source <formula>)? If this is a new formula, does it pass brew audit --new <formula>?

Comment on lines +74 to +77
# Also can segfault at runtime: https://github.com/Homebrew/homebrew-core/issues/135200
if Hardware::CPU.intel? && DevelopmentTools.clang_build_version == 1403
inreplace "configure", "-fzero-call-used-regs=all", "-fzero-call-used-regs=used"
end
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corresponding configure.ac:

	# clang 15 seems to have a bug in -fzero-call-used-regs=all.  See
	# https://bugzilla.mindrot.org/show_bug.cgi?id=3475 and
	# https://github.com/llvm/llvm-project/issues/59242
	case "$CLANG_VER" in
	15.*) OSSH_CHECK_CFLAG_COMPILE([-fzero-call-used-regs=used]) ;;
	*)    OSSH_CHECK_CFLAG_COMPILE([-fzero-call-used-regs=all]) ;;
	esac

Apple clang 14.0.3 is based on LLVM 15. Upstream fix would require changing configure.ac logic to detect this particular version. Just using simple inreplace for now and maybe we can add details to existing upstream bug.

Haven't checked if root cause of every reported OpenSSH issue, but worked for me locally to fix segfault issue.

Corresponding LLVM bug fix https://reviews.llvm.org/D139679 went in 15.0.7. Looks like it was zeroing out some registers which could lead to original ssh-keygen issue (i.e. argc/argv getting zeroed) and the segfault issue where some variables were NULL.

@cho-m cho-m changed the title openssh: inreplace configure to apply workaround to Apple clang 14.0.3 openssh: inreplace to apply workaround to Apple clang 14.0.3 Jun 30, 2023
@mitchblank
Copy link
Contributor

I know that right now this is just in testing, but please bump revision for this when it ships.

I've had a couple times recently where I've hit problems in homebrew only to find that the underlying bug has been fixed and re-bottled so I needed to manually brew reinstall to fix my machine. The current openssh package is pretty busted (at least on Intel) so I think it would definitely be wise to force an upgrade to the fixed bottles when they are available.

@Bo98
Copy link
Member

Bo98 commented Jun 30, 2023

Sounds like llvm/llvm-project#57692 so this is looking like the correct fix.

@cho-m cho-m marked this pull request as ready for review June 30, 2023 01:50
@github-actions
Copy link
Contributor

🤖 An automated task has requested bottles to be published to this PR.

@github-actions
Copy link
Contributor

⚠️ Bottle publish failed. CC @carlocab

@github-actions
Copy link
Contributor

🤖 An automated task has requested bottles to be published to this PR.

@github-actions github-actions bot added the CI-published-bottle-commits The commits for the built bottles have been pushed to the PR branch. label Jun 30, 2023
@BrewTestBot BrewTestBot added this pull request to the merge queue Jun 30, 2023
Merged via the queue into Homebrew:master with commit 01e9e03 Jun 30, 2023
12 checks passed
@cho-m cho-m deleted the openssh-inreplace branch June 30, 2023 06:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI-published-bottle-commits The commits for the built bottles have been pushed to the PR branch.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants