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

Make methods exported in shared libraries non-inline #880

Open
simark opened this issue Aug 10, 2015 · 4 comments
Open

Make methods exported in shared libraries non-inline #880

simark opened this issue Aug 10, 2015 · 4 comments

Comments

@simark
Copy link
Contributor

simark commented Aug 10, 2015

Source: build failure of the buildroot package
http://autobuild.buildroot.net/results/e14/e14e1700d4fe359c56be57587bdb509e002e5753/build-end.log

OLA is built with -fvisibility-inlines-hidden. This causes methods defined as inline to be hidden to other object files by default. It seems like this can be a problem for methods that are exported by a shared library. The fix is to move the implementation of the method to the .cpp file.

@nomis52
Copy link
Member

nomis52 commented Aug 10, 2015

+1

@simark
Copy link
Contributor Author

simark commented Aug 10, 2015

Just to explore the possibilities, why was the -fvisibility-inlines-hidden switch added? From what I understand, it's supposed to improve startup performance (since it reduces the number of symbols the dynamic linker has to resolve). What was the impact of it on performance?

Removing the switch would also probably fix this particular issue, but I would like to know if it would be acceptable.

@nomis52
Copy link
Member

nomis52 commented Aug 13, 2015

Well that was a bit of digging.

This was added in bfc1d99 . Unfortunately that was before we started using pull requests, so there isn't a comment history on what I was thinking. I can't find anything in my email either.

Turning it off for me continues to work using clang-602.0.53 on OS X but that's not a guarantee it doesn't break somewhere else. I think I'd prefer to see a fix that either makes the necessary functions non-inline, or marks them as visible.

@simark
Copy link
Contributor Author

simark commented Aug 13, 2015

I'd be curious to see the original failure. From what I understand, having this option can improve performance, but not having it shouldn't be a reason for a build failure.

@nomis52 nomis52 removed this from the 0.9.8 milestone Sep 26, 2015
commodo added a commit to commodo/packages that referenced this issue Dec 14, 2015
Reported buildbot issue is:
/store/buildbot/slave/ar71xx/build/build_dir/target-mips_34kc_musl-1.1.11/ola-0.9.7/plugins/openpixelcontrol/.libs/libolaopenpixelcontrol.so: undefined reference to `ola::network::TCPSocket::ReadDescriptor() const'
collect2: error: ld returned 1 exit status

There's also a discussion (attempt) to fix this on the buildroot project:
  https://patchwork.ozlabs.org/patch/503884/

This bug has been reported (from the buildroot project), here:
  OpenLightingProject/ola#880

This commit introduced the issue:
  OpenLightingProject/ola@bfc1d99

So, until, the upstream project (ola) fixes this, this fix
looks like the quickest/simplest workaround to have this package build.

Signed-off-by: Alexandru Ardelean <ardeleanalex@gmail.com>
commodo added a commit to commodo/packages that referenced this issue Dec 14, 2015
Reported buildbot issue is:
/store/buildbot/slave/ar71xx/build/build_dir/target-mips_34kc_musl-1.1.11/ola-0.9.7/plugins/openpixelcontrol/.libs/libolaopenpixelcontrol.so: undefined reference to `ola::network::TCPSocket::ReadDescriptor() const'
collect2: error: ld returned 1 exit status

There's also a discussion (attempt) to fix this on the buildroot project:
  https://patchwork.ozlabs.org/patch/503884/

This bug has been reported (from the buildroot project), here:
  OpenLightingProject/ola#880

This commit introduced the issue:
  OpenLightingProject/ola@bfc1d99
specifically the `-fvisibility-inlines-hidden` switch.

So, until, the upstream project (ola) fixes this, this fix
looks like the quickest/simplest workaround to have this package build.

Signed-off-by: Alexandru Ardelean <ardeleanalex@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants