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.

Sign up for GitHub

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

Jump to bottom

Constant-time tests on macOS #1151

Closed
sipa opened this issue Nov 14, 2022 · 5 comments · Fixed by #1412
Closed

Constant-time tests on macOS #1151

sipa opened this issue Nov 14, 2022 · 5 comments · Fixed by #1412
Labels
assurance ci

Comments

@sipa
Copy link
Contributor

sipa commented Nov 14, 2022 •
edited
Loading

I noticed this in the CI output:

We are sunsetting support for Intel-based macOS instances! Please migrate before January 1st 2023.
@sipa sipa mentioned this issue Nov 17, 2022
Merged
@maflcko
Copy link
Contributor

maflcko commented Nov 29, 2022

Is this fixed now?

@real-or-random
Copy link
Contributor

real-or-random commented Nov 29, 2022 •
edited
Loading

from #1152:

It drops valgrind and the ctime tests on macOS, as valgrind simply isn't supported anymore there. We may recover some of that functionality by adding MSan (#1155) for memory-checking there, and adding ctime-using-mtime (TODO).

I think we should leave it open until this has been fixed.

  • Enable MSan for MacOS
  • Make the constant-time tests work on MSan

@real-or-random real-or-random changed the title Cirrus CI sunsetting x86-based macOS support Constant-time tests with MSan / for MacOS Dec 5, 2022
@real-or-random
Copy link
Contributor

real-or-random commented Jan 11, 2023 •
edited
Loading

Oh, the valgrind fork now works on Ventura LouisBrunner/valgrind-macos#54 (comment) (except for memory leak tracking, which is still WIP but that shouldn't be required for our purposes.) So we should probably revert c0ae48c partly.

@real-or-random real-or-random mentioned this issue Apr 12, 2023
Merged
@hebasto
Copy link
Member

hebasto commented Jun 5, 2023

  • Enable MSan for MacOS

https://clang.llvm.org/docs/MemorySanitizer.html#supported-platforms:

MemorySanitizer is supported on the following OS:

  • Linux
  • NetBSD
  • FreeBSD

@real-or-random
Copy link
Contributor

real-or-random commented Jun 11, 2023 •
edited
Loading

Oh indeed, MSan doesn't work on macOS...

  • Make the constant-time tests work on MSan

And this has been solved by #1169.

This means that there's nothing to do for us here except to wait for LouisBrunner/valgrind-macos#56. This will make it possible to run Valgrind on macOS again.

Leaving this open to track this.

@real-or-random real-or-random changed the title Constant-time tests with MSan / for MacOS Constant-time tests on macOS Jun 11, 2023
@real-or-random real-or-random mentioned this issue Aug 15, 2023
Open
17 tasks
@real-or-random real-or-random mentioned this issue Aug 28, 2023
Merged
@real-or-random real-or-random linked a pull request Aug 28, 2023 that will close this issue
ci: Switch macOS from Ventura to Monterey and add Valgrind #1412
Merged
real-or-random added a commit that referenced this issue Aug 29, 2023
@real-or-random
Merge #1412: ci: Switch macOS from Ventura to Monterey and add Valgrind
65c79fe 
c223d7e ci: Switch macOS from Ventura to Monterey and add Valgrind (Hennadii Stepanov)

Pull request description:

  This PR switches the macOS native job from Ventura to Monterey, which allows to support Valgrind.

  Both runners--`macos-12` and `macos-13`--have the same clang compilers installed:
  - https://github.com/actions/runner-images/blob/main/images/macos/macos-12-Readme.md
  - https://github.com/actions/runner-images/blob/main/images/macos/macos-13-Readme.md

  But Valgrind works fine on macOS Monterey, but not on Ventura.

  See: #1392 (comment).

  The Homebrew's Valgrind package is cached once it has been built (as it was before #1152). Therefore, the `actions/cache@*` action is needed to be added to the list of the allowed actions.

  #1412 (comment):
  > By the way, this solves #1151.

ACKs for top commit:
  real-or-random:
    ACK c223d7e I tested that a cttest failure makes CI fail: https://github.com/real-or-random/secp256k1/actions/runs/6010365844

Tree-SHA512: 5e72d89fd4d82acbda8adeda7106db0dad85162cca03abe8eae9a40393997ba36a84ad7b12c4b32aec5e9230f275738ef12169994cd530952e2b0b963449b231
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
assurance ci
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

ci: Switch macOS from Ventura to Monterey and add Valgrind hebasto/secp256k1
4 participants
@sipa @real-or-random @maflcko @hebasto