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

valgrind 3.12.0 #6231

Closed
wants to merge 1 commit into from
Closed

valgrind 3.12.0 #6231

wants to merge 1 commit into from

Conversation

zmwangx
Copy link
Contributor

@zmwangx zmwangx commented Oct 24, 2016

  • Have you followed the guidelines for contributing?
  • 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?
  • Does your build pass brew audit --strict <formula> (after doing brew install <formula>)?

Announcement:
https://sourceforge.net/p/valgrind/mailman/message/35447135/.

Highlight:

Preliminary support for MacOS 10.12 (Sierra) has been added.

Shame it needs a patch to build.

@zmwangx
Copy link
Contributor Author

zmwangx commented Oct 24, 2016

CC @mfukar who asked about Vagrind on 10.12 here.

@zmwangx
Copy link
Contributor Author

zmwangx commented Oct 24, 2016

Forgot to vendor the patch apparently. On it, before anyone calls me out.

@zmwangx zmwangx force-pushed the valgrind-3.12.0 branch 2 times, most recently from dd30ac2 to d318992 Compare October 24, 2016 15:31
@zmwangx
Copy link
Contributor Author

zmwangx commented Oct 24, 2016

Test fails on 10.12, which is expected, see https://bugs.kde.org/show_bug.cgi?id=365327#c2:

Valgrind binaries do not yet run on macOS 10.12, due to an error mapping the NULL page during load of /usr/lib/dyld, see output below:
...
valgrind: mmap-FIXED(0x0, 253952) failed in UME (load_segment1) with error 12 (Cannot allocate memory).

After all this is "preliminary support", and in the case of Valgrind it means "still useless".

We can keep

depends_on MaximumMacOSRequirement => :el_capitan

or not. Either case it's not our problem and there will be a little bit of support burden which we can deflect. Thoughts on keeping MMOSR?

@zmwangx zmwangx added maintainer feedback Additional maintainers' opinions may be needed 10.10 Yosemite is specifically affected 10.12 Sierra is specifically affected and removed 10.10 Yosemite is specifically affected labels Oct 24, 2016
@zmwangx
Copy link
Contributor Author

zmwangx commented Oct 26, 2016

Lacking feedback, I'll keep the MMOSR instead of shipping a broken binary.

Announcement:
https://sourceforge.net/p/valgrind/mailman/message/35447135/.

Highlight:

* Preliminary support for MacOS 10.12 (Sierra) has been added.
@zmwangx zmwangx closed this in b91363f Oct 26, 2016
@zmwangx zmwangx deleted the valgrind-3.12.0 branch October 26, 2016 03:10
@ilovezfs
Copy link
Contributor

@zmwangx do you feel like giving the patch in https://bugs.kde.org/show_bug.cgi?id=365327#c12 a try?

@zmwangx
Copy link
Contributor Author

zmwangx commented Apr 21, 2017

@ilovezfs I tried it and here's what happens when instrumenting a simple program:

#include <stdlib.h>

int main() {
    void *buf = malloc(65536);
    return 0;
}

Valgrind:

...
==83732== HEAP SUMMARY:
==83732==     in use at exit: 83,911 bytes in 163 blocks
==83732==   total heap usage: 179 allocs, 16 frees, 90,055 bytes allocated
==83732==
==83732== LEAK SUMMARY:
==83732==    definitely lost: 65,536 bytes in 1 blocks
==83732==    indirectly lost: 0 bytes in 0 blocks
==83732==      possibly lost: 72 bytes in 3 blocks
==83732==    still reachable: 200 bytes in 6 blocks
==83732==         suppressed: 18,103 bytes in 153 blocks
==83732== Rerun with --leak-check=full to see details of leaked memory
...

So, it is at least in a working state.

@ilovezfs
Copy link
Contributor

Hurray! Thanks for testing it.

@Homebrew Homebrew locked and limited conversation to collaborators May 4, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
10.12 Sierra is specifically affected maintainer feedback Additional maintainers' opinions may be needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants