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

Hide qhullcpp symbol #527

Draft
wants to merge 2 commits into
base: devel
Choose a base branch
from

Conversation

jorisv
Copy link
Contributor

@jorisv jorisv commented Feb 5, 2024

Problem 1

qhullcpp is a static library that had been build with default symbol visibility strategies : all is visible.
When linking against this library, hpp-fcl get all libqhull symbols and also show them as visible.

If a thirdparty library is linking against hpp-fcl and qhullcpp, then a strange symbol mix-up can occur.
I have been able to reproduce it with pinocchio 3.

This can lead to binary conflicts, since there is no guarantee that pinocchio 3 and hpp-fcl are using the same version of qhullcpp with the same ABI (same compiler flags).

Solution

I advise to hide all qhullcpp symbols when linking against it.
I only know how to do it and test it on Linux. We should find a way to fix it on MacOS and Windows if the same issue occur.

Problem 2

hpp-fcl embed qhullcpp source code. This source code is working with some old qhull_r versions. I have not be able to build it with qhull_r 2020.2.

Solution

Maybe we can get rid of it ? Or at least document which version of qhull_r is needed.

If we keep it, then we should find a way to not export the qhullcpp symbol either (probably by creating a static library).

@jorisv jorisv self-assigned this Feb 5, 2024
@jorisv
Copy link
Contributor Author

jorisv commented Feb 5, 2024

It's maybe possible to hide symbol in macos with -hidden-lx linker flag.

On Windows, I think that symbol are hidden by default (no dllexport).

@jorisv jorisv marked this pull request as draft February 5, 2024 10:59
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.

1 participant