Skip to content

Conversation

tvh
Copy link

@tvh tvh commented Nov 4, 2021

  • 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>?

This adresses google/leveldb#634.

When installing this locally I had to remove gperftools first for this to work. I don't know how the bottles are built and if that will be ensured.

@carlocab
Copy link
Member

carlocab commented Nov 4, 2021

It seems weird to do this, since I expect macOS to not be affected by the issue you're referencing. I was going to try it for myself, but we already see from google/leveldb#634 (comment):

but on macOS, for example, it doesn't cause a crash.

@tvh
Copy link
Author

tvh commented Nov 4, 2021

Yes, This only seems to happen in Monterey.

@carlocab
Copy link
Member

carlocab commented Nov 4, 2021

No, I don't think that's true.

❯ sw_vers
ProductName:	macOS
ProductVersion:	12.0.1
BuildVersion:	21A559
❯ cat test.cpp
#include <dlfcn.h>
#include <string>
int main() {
    std::string s;
    dlopen("/opt/homebrew/lib/libleveldb.dylib", RTLD_NOW);
    return 0;
}
❯ make test
c++     test.cpp   -o test
❯ ./test; echo $?
0
❯ otool -L /opt/homebrew/lib/libleveldb.dylib
/opt/homebrew/lib/libleveldb.dylib:
	/opt/homebrew/opt/leveldb/lib/libleveldb.1.dylib (compatibility version 1.0.0, current version 1.23.0)
	/opt/homebrew/opt/snappy/lib/libsnappy.1.dylib (compatibility version 1.0.0, current version 1.1.9)
	/opt/homebrew/opt/gperftools/lib/libtcmalloc.4.dylib (compatibility version 10.0.0, current version 10.9.0)
	/usr/lib/libc++.1.dylib (compatibility version 1.0.0, current version 1200.3.0)
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1311.0.0)

@tvh
Copy link
Author

tvh commented Nov 4, 2021

Where I see this is here: https://gitlab.haskell.org/ghc/ghc/-/issues/20610

I wonder if that might be a different linker issue as it does successfully link and then crash.

@carlocab
Copy link
Member

carlocab commented Nov 4, 2021

It seems like your bug is something else entirely, as the reproducer in the issue you reference doesn't apply. I'm reluctant to apply a change to a formula that affects many other users when it's not at all clear that this is the right fix. Moreover, even if it were the right one, I don't know how this weighs against potentially breaking other users who rely on leveldb using tcmalloc.

I don't use GHC, so I can't really say what's happening with your crash. Does GHC call dlopen on libleveldb?

@bgamari
Copy link

bgamari commented Nov 7, 2021

I don't use GHC, so I can't really say what's happening with your crash. Does GHC call dlopen on libleveldb?

Yes, GHC does call dlopen on libleveldb.

@carlocab
Copy link
Member

carlocab commented Nov 8, 2021

I see. But it seems the dlopen by itself isn't enough to cause a crash, so there's likely something more going on.

@carlocab
Copy link
Member

Does this fix your issue? gperftools/gperftools#1315

carlocab added a commit to carlocab/homebrew-core that referenced this pull request Nov 18, 2021
Simple programs segfault when linked with tcmalloc. Patch taken from
gperftools/gperftools#1315.

Also, update the test to catch the segfault.

Closes Homebrew#88758.
@github-actions github-actions bot added the outdated PR was locked due to age label Dec 22, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 22, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants