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

Support shared libs "properly" #3214

Open
dneto0 opened this issue Mar 6, 2020 · 12 comments
Open

Support shared libs "properly" #3214

dneto0 opened this issue Mar 6, 2020 · 12 comments

Comments

@dneto0
Copy link
Collaborator

dneto0 commented Mar 6, 2020

  • Generate a shared library corresponding to each static library we can build. (Our team's flows normally only use static libraries.)
  • Only expose entry points we deliberately want to export. (hidden visibility by default)
  • use proper SONAME for Linux: No library versions in soname #3046
    • need to determine a sensible version-numbering system. Since we don't absolutely promise backward compatibility, I think it's best to use 0... E.g. if we're in the v2020.2-dev or v2020.2 releases, then SONAME would end in .0.2020.2

See also the interesting discussion over at Shaderc: google/shaderc#381 and google/shaderc#498

@dneto0
Copy link
Collaborator Author

dneto0 commented Mar 6, 2020

cc: @greg-lunarg

@dneto0
Copy link
Collaborator Author

dneto0 commented Mar 8, 2020

Also support MACOSX_RPATH properly

@zoddicus
Copy link
Contributor

@dneto0
I recall us discussing versioning, atleast for shaderc, and were thinking something like 0.YYYY.MM.DD, so they would be monotonically increasing, but suggestive that we don't guarantee API compat.

Doing 0.YYYY.release# or 0.YYYY.release#-dev I think as you have suggested above is better, since there is a bit more meaning to the release # then just the date it was cut on.

@rpavlik
Copy link

rpavlik commented Apr 16, 2020

Anything I can do to help with this? It's blocking some of my work.

@SpaceIm
Copy link
Contributor

SpaceIm commented Apr 17, 2020

Just to mention that I have tried to fix shared libs of SPIRV-Tools for conan, with generate_export_header in CMake.
So I have a created an export header for each lib, with specific export macro for each lib.
Here what I can hightlight so far (I've worked on version 2019-2):

  • SPIRV-Tools can be properly generated and consumed as a shared lib (with C and C++ API) if SPIRV_TOOLS_EXPORT is added in front of its C++ API (in include/spirv-tools/libspirv.hpp), in addition to C API.
    I've also removed SPIRV_TOOLS_EXPORT in 2 cpp files, don't know why they were here.

  • SPIRV-Tools-opt can't find several symbols in this shared version of SPIRV-Tools (with non API symbols hidden) because it relies on several non API Symbols.
    Therefore I add to export those symbols in private headers, which doesn't smell good:

    • assembly_grammar.h:
    spv_result_t AssemblyGrammar::lookupOpcode(SpvOp opcode, spv_opcode_desc* desc) const;
    spv_result_t AssemblyGrammar::lookupOperand(spv_operand_type_t type, uint32_t operand, spv_operand_desc* desc) const;
    • disassemble.h:
    std::string spvInstructionBinaryToText(const spv_target_env env,
                                           const uint32_t* inst_binary,
                                           const size_t inst_word_count,
                                           const uint32_t* binary,
                                           const size_t word_count,
                                           const uint32_t options);
    • enum_string_mapper.h:
    bool GetExtensionFromString(const char* str, Extension* extension);
    • opcode.h: a lot of symbols, most of them actually
    • operand.h:
    bool spvIsIdType(spv_operand_type_t type);
    bool spvIsInIdType(spv_operand_type_t type);
    • table.h:
    void SetContextMessageConsumer(spv_context context, MessageConsumer consumer);

    With all these symbols exported I was able to successfully link SPIRV-Tools into SPIRV-Tools-opt

  • The really fun part began with SPIRV-Tools-link. It relies on several non API symbols of SPIRV-Tools-opt (in derived classes or classes with templated functions or with inline functions relying of others private symbols), and also in SPIRV-Tools (others than those that SPIRV-Tools-opt was not able to find). At this point I stopped.
    By itself, I think that SPIRV-Tools-link is the only lib which doesn't need to export symbols in its private headers (because this lib doesn't have private header, so it's simple).

  • Didn't have the time to try SPIRV-Tools-reduce, but this lib doesn't have any public header, so I don't know why it is installed.

  • Several tools executables also rely on private headers, they may also need additionals symbols, didn't check.

Other ugly option: use CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS and don't hide symbols for other compilers thant Visual Studio...It works (I have a working example for conan as a workaround, but it's bad practice).

Off Topic: it would be nice to don't force the build of SPIRV-Tools-shared, by providing an option for example. If build of all lib as shared is completly fixed, it would be better to have an option to build both, or let consumers build static or shared with BUILD_SHARED_LIBS

@rpavlik
Copy link

rpavlik commented May 12, 2020

I suspect that rather than an export header, a version script (linux/mac) or .def file is probably the easiest way to go here. OpenXR was careful to actually get our export header stuff right, and we still ended up using those methods to clean up and standardize our loader's exports.

For reference: ours is here

You'll probably also want to start linking -Wl,--no-undefined given the complex combo of libs mentioned: it makes undefined references as much of a problem on gcc/clang as on msvc.

@smcv
Copy link

smcv commented May 13, 2020

You'll probably also want to start linking -Wl,--no-undefined given the complex combo of libs mentioned: it makes undefined references as much of a problem on gcc/clang as on msvc.

This is excellent advice. For C code, -fno-common is another good "please be more strict" option (I'm not sure whether it applies to C++ though).

I'm not sure whether .def files work well for C++ due to name-mangling, but an export header is also a good way to control what you export.

need to determine a sensible version-numbering system. Since we don't absolutely promise backward compatibility, I think it's best to use 0... E.g. if we're in the v2020.2-dev or v2020.2 releases, then SONAME would end in .0.2020.2

Please note that there can be a difference between the "marketing" version number and the ABI version, and there is also a difference between the SONAME and the ABI version.

On Linux and similar platforms, the goal is to get this setup:

  • The SONAME, as used in -Wl,-soname,${SONAME}, contains the "major version" of the ABI, typically something like libSPIRV-Tools.so.0. It's incremented to libSPIRV-Tools.so.1 when you make an incompatible ABI change (meaning that code linked against the old ABI will no longer work).
  • The real file that implements the library is named something like libSPIRV-Tools.so.0.2020.2. The part after the SONAME can be anything you want, within reason - it acts as the "minor version" of the ABI.
  • There is a symbolic link whose name matches the SONAME, pointing to the real file. For example, libSPIRV-Tools.so.0 -> libSPIRV-Tools.so.0.2020.2.
  • There is another symbolic link that is used when you pass -lSPIRV-Tools to your compiler, either libSPIRV-Tools.so -> libSPIRV-Tools.so.0.2020.2 or libSPIRV-Tools.so -> libSPIRV-Tools.so.0.
  • You can break backwards compatibility as often as you need to; but when you do, you increase the number in the SONAME. This allows the old and new versions to remain installed together, in parallel.
  • When you haven't broken backwards compatibility, don't increase the number in the SONAME, only the "minor version" of the ABI - so if you don't break backwards compatibility for a long time, you might go from .so.0.2020.2 to .so.0.2021.5.

On Windows, one reasonably common convention is that if your SONAME on Linux is libSPIRV-Tools.so.0, then your Windows library is libSPIRV-Tools-0.dll. Again, that lets you keep libSPIRV-Tools-0.dll and libSPIRV-Tools-1.dll installed in parallel, and each executable will be linked to one or the other.

@smcv
Copy link

smcv commented May 13, 2020

https://pusling.com/blog/?p=352 has some good examples of setting the version of a shared library in CMake. The SOVERSION is what ends up in the SONAME (it would be 0 or 1 for my examples above), and the VERSION is in the name of the regular file (not a symlink), for example 0.2020.2 for my examples above.

You can use objdump -Tx libmylibrary.so | grep SONAME to check what the SONAME is, according to the DT_SONAME ELF header.

@smcv
Copy link

smcv commented May 21, 2020

Would it be helpful for me or @rpavlik to provide a pull request that sets a correct SONAME for Unix builds? That would close #3046, at least.

Some assumptions that this would need to make:

  • The libraries are all intended to be public (usable in/by third-party tools)
  • The maintainers will increase the SOVERSION when there is an incompatible change (we can document how to do this, but it would be up to the maintainers to make it happen)
  • The SOVERSION is the same across all libraries (or we could have a separate SOVERSION for each library if you want, but that's more complex, and probably more likely to go wrong)

Please let us know if any of those assumptions are wrong.

Explicit control over exported symbols (export header or version-script) would be nice to have, but is not necessarily required to fix #3046, so I think it would be worthwhile to solve #3046 first.

@dneto0
Copy link
Collaborator Author

dneto0 commented Jun 11, 2020

Thanks for your patience on and thoughtful contributions.

@ben-clayton will be looking at this and applying a consistent solution across the projects our team maintains.

@paulmenzel
Copy link

Would it be helpful for me or @rpavlik to provide a pull request that sets a correct SONAME for Unix builds? That would close #3046, at least.

I guess, that would be helpful.

@jiapei-nexera
Copy link

I can tell when I built SPIRV-Tools, spvIsInIdType function is NOT built out... How to make it built into a library??

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

8 participants