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

zstd symbol name collision between vsgXchange ktx library's zstd symbols and libvulkan_radeon.so's use of the same symbol names from system's libzstd.so.1 #193

Closed
rms7326 opened this issue Jul 16, 2024 · 4 comments

Comments

@rms7326
Copy link

rms7326 commented Jul 16, 2024

We built vsgXchange so that we could use the shader and TrueType font loaders. We do not need any of the other features like ktx, assimp, etc. assimp is optional but ktx is not. When libktx is built it compiles the zstddeclib.c module which includes public symbols such as ZSTD_decompressDCtx that are also defined in libzstd.so. As can be seen in the stack trace below, the Vulkan Radeon driver calls ZSTD_decompress in /lib64/libzstd.so.1 which subsequently calls ZSTD_decompressDCtx which it finds in libvsgXchange.so.1. Both libraries have the same publicly named symbols so it is probably hit or miss as to which one you get. Anyway, this causes a crash.

/appdir/bin/libvsgXchange.so.1(ZSTD_freeDDict+0xd)
/appdir/bin/libvsgXchange.so.1(ZSTD_decompressDCtx+0x37)
/lib64/libzstd.so.1(ZSTD_decompress+0x5e)
/usr/lib64/libvulkan_radeon.so(+0x283def)
...
/appdir/bin/libvsg.so.14(_ZN3vsg16GraphicsPipeline14ImplementationC2ERNS_7ContextEPNS_6DeviceEPKNS_10RenderPassEPKNS_14PipelineLayoutERKSt6vectorINS_7ref_ptrINS_11ShaderStageEEESaISF_EERKSC_INSD_INS_21GraphicsPipelineStateEEESaISL_EEj+0x253)

I have been able to fix it with two separate work-arounds:

The first solution modifies the build file and source file:

  • Modify vsgXchange/src/ktx/build_vars.cmake such that EXTRA_INCLUDES variable is prepended instead of appended with the src/ktx/libktx directory such that it ensures zstd.h in the ktx directory gets found before the one in /usr/include. As an alternative, change the name of src/ktx/libktx/zstd.h by adding a name prefix to it such that it is uniquely found. Note that several files #include the file so those references would have to change too.
  • Modify vsgXchange/src/ktx/zstd.h line 25 changing visibility ("default") to visibility ("hidden")
    This solution effectively hides the ZSTD_blah symbols making them only visible within the vsgXchange library

The second solution just modifies the build file to use the system's libzstd.so.1 instead of the ktx copy of the source code:

  • comment out line 46 of vsgXchange/src/ktx/build_vars.cmake removing ktx/libktx/zstddeclib.c from the sources.
  • add set(EXTRA_LIBRARIES ${EXTRA_LIBRARIES} zstd) to the end of vsgXchange/src/ktx/build_vars.cmake forcing it to use the Linux system's libzstd.so library.
    This solution requires that the development version of libzstd is installed on the Linux machine building the library. Additionally these instructions are not cross platform whereas the previous solution edits Linux specific instructions.

What would you suggest is the best way to solve this issue?

@rms7326 rms7326 changed the title zstd symbol name collision between vsgXchange ktx library's zstd symbols and libvulkan_radeon.so's use of the same symbol names from system's libzstd.so1 zstd symbol name collision between vsgXchange ktx library's zstd symbols and libvulkan_radeon.so's use of the same symbol names from system's libzstd.so.1 Jul 16, 2024
@robertosfield
Copy link
Collaborator

What AMD drivers are you using? I am using Kubuntu 22.04 on my AMD 5700G desktop system without problem.

libzstd.so.1 won't be a standard library so just making it an external dependency will break the build on most OS's/distro's so the first suggested solution sounds most appropriate,

The inclusion of the ktx sources into the vsgXchange build has always been a fallback because historically ktx sources were problematic to checkout and build so I fudged ease of use by just pulling in the sources and building what we needed. It would worth revisiting this as it may now be possible to pull in ktx support via package managers.

@rms7326
Copy link
Author

rms7326 commented Jul 17, 2024

I have fedora40 using dnf. AMD doesn't supply proprietary drivers for fedora so I use their Mesa drivers enroute to some hardware acceleration. It is still pretty fast. That may be the difference between our two systems. Here is what my installation looks like:

root@tpw101:~# dnf list installed | grep radeon
radeon-profile.x86_64                                  20200824-11.fc40                       @fedora
radeon-profile-daemon.x86_64                           20190603-9.fc40                        @fedora
radeontop.x86_64                                       1.4-8.fc40                             @fedora
root@tpw101:~# dnf list installed | grep amd
amd-gpu-firmware.noarch                                20240709-1.fc40                        @updates
amd-ucode-firmware.noarch                              20240709-1.fc40                        @updates
xorg-x11-drv-amdgpu.x86_64                             23.0.0-3.fc40                          @fedora 

@robertosfield
Copy link
Collaborator

I'm using the Mesa drives under Kubuntu but as I'm on a Windows machine right now I can do any checks.

Could you apply the your first suggestion above, get it working then create a PR for it? I can then review and test out on my systems. Thanks.

@rms7326
Copy link
Author

rms7326 commented Jul 17, 2024

Will do

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

No branches or pull requests

2 participants