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

Use -fvisibility=hidden for spdlog #5427

Merged
merged 2 commits into from
Jan 17, 2025

Conversation

johnkerl
Copy link
Contributor

@johnkerl johnkerl commented Jan 17, 2025

This is a branch coded up by @teo-tsirpanis . I'm putting up a PR for it, at his expicit request.

The problem to be solved is intermittent errors in TileDB-SOMA, which uses this repo as a dependency. For dev work, on a Mac, I build both TileDB Core and TileDB-SOMA from source. The intersection of those three things makes me, perhaps, unusual. Regardless, I have (until this PR) codepaths which, quite simply, always segfault.

The segfaulting patterns involve innocuous logging statements. The stack traces are of this form: https://gist.github.com/johnkerl/b1e07a70906177f498f6e9efa2979376

Notes:

  • Note the arrowed lines
  • A variable in one frame called formatted is being passed to another with argument name dest
  • In lldb, p sizeof(formatted) in the upper frame shows 24 while p sizeof(dest) in the lower frame shows 288
  • This is across the boundary from libtiledb.dylib spdlog::logger to libtiledbsoma.dylib spdlog::logger, and back -- so it's an ABI issue apparently.
  • I compiled/reran with 1.14.1 for soma & core spdlog both, yet the issue persists

I also tried -fvisibility=hidden within TileDB-SOMA's copy of spdlog and was not able to resolve the problem. Meanwhile @teo-tsirpanis's solution here does resolve the problem.

[sc-62143]


TYPE: IMPROVEMENT
DESC: Use -fvisibility=hidden for spdlog

Copy link
Member

@teo-tsirpanis teo-tsirpanis left a comment

Choose a reason for hiding this comment

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

Thanks! gabime/spdlog#3322 will fix the issue upstream.

@johnkerl
Copy link
Contributor Author

@teo-tsirpanis @ihnorton thank you for your approval!

I have no merge permissions on this repo -- please merge if you're OK with merging.

@teo-tsirpanis teo-tsirpanis merged commit ba3aed3 into main Jan 17, 2025
63 checks passed
@teo-tsirpanis teo-tsirpanis deleted the teo/ephemeral/spdlog-fvisibility-hidden branch January 17, 2025 22:07
@johnkerl
Copy link
Contributor Author

Thank you @teo-tsirpanis !

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

Successfully merging this pull request may close these issues.

3 participants