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

More valgrind fixes #12990

Closed
wants to merge 3 commits into from
Closed

Conversation

pdillinger
Copy link
Contributor

@pdillinger pdillinger commented Sep 4, 2024

Summary:

  • Attempt fix valgrind FP on std::optional #12936 was insufficient to fix the std::optional false positives. Making a fix validated in CI this time (see Valgrind update ci #12991)
  • valgrind grinds to a halt on startup on my dev machine apparently because it expects internet access. Disable its attempts to access the internet when git is using a proxy.
  • Move PORTABLE=1 from CI job to the Makefile. Without it, valgrind complains about illegal instructions (too new)

Test Plan: manual, watch nightly valgrind job

Summary:
* facebook#12936 was insufficient to fix the std::optional false positives.
  Making another attempt (don't have a local reproducer)
* valgrind grinds to a halt on startup on my dev machine apparently
  because it expects internet access. Disable its attempts to access the
  internet when git is using a proxy.
* Move PORTABLE=1 from CI job to the Makefile. Without it, valgrind
  complains about illegal instructions (too new)

Test Plan: manual, watch nightly valgrind job
@facebook-github-bot
Copy link
Contributor

@pdillinger has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@pdillinger has updated the pull request. You must reimport the pull request before landing.

@facebook-github-bot
Copy link
Contributor

@pdillinger has updated the pull request. You must reimport the pull request before landing.

@pdillinger
Copy link
Contributor Author

Validated in GitHub Actions valgrind run in #12991

@facebook-github-bot
Copy link
Contributor

@pdillinger has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@pdillinger pdillinger changed the title Attempt more valgrind fixes More valgrind fixes Sep 4, 2024
@facebook-github-bot
Copy link
Contributor

@pdillinger has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@pdillinger
Copy link
Contributor Author

@facebook-github-bot
Copy link
Contributor

@pdillinger merged this pull request in 4577b67.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants