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

With system brotli, do not use versioned symbols #101576

Merged
merged 1 commit into from
Apr 30, 2024

Conversation

omajid
Copy link
Member

@omajid omajid commented Apr 25, 2024

The system version of libbrotli has symbols without the V1.0 version. Support linking against that by removing the steps to use the versioned exports file.

Without this fix, the build only works with bfd linker (I am not sure how). When lld is used, the build errors out:

ld.lld : error : version script assignment of 'V1.0' to symbol 'BrotliDecoderCreateInstance' failed: symbol not defined [runtime/src/native/libs/build-native.proj]
ld.lld : error : version script assignment of 'V1.0' to symbol 'BrotliDecoderDecompress' failed: symbol not defined [runtime/src/native/libs/build-native.proj]
ld.lld : error : version script assignment of 'V1.0' to symbol 'BrotliDecoderDecompressStream' failed: symbol not defined [runtime/src/native/libs/build-native.proj]
ld.lld : error : version script assignment of 'V1.0' to symbol 'BrotliDecoderDestroyInstance' failed: symbol not defined [runtime/src/native/libs/build-native.proj]
ld.lld : error : version script assignment of 'V1.0' to symbol 'BrotliDecoderIsFinished' failed: symbol not defined [runtime/src/native/libs/build-native.proj]
ld.lld : error : version script assignment of 'V1.0' to symbol 'BrotliEncoderCompress' failed: symbol not defined [runtime/src/native/libs/build-native.proj]
ld.lld : error : version script assignment of 'V1.0' to symbol 'BrotliEncoderCompressStream' failed: symbol not defined [runtime/src/native/libs/build-native.proj]
ld.lld : error : version script assignment of 'V1.0' to symbol 'BrotliEncoderCreateInstance' failed: symbol not defined [runtime/src/native/libs/build-native.proj]
ld.lld : error : version script assignment of 'V1.0' to symbol 'BrotliEncoderDestroyInstance' failed: symbol not defined [runtime/src/native/libs/build-native.proj]
ld.lld : error : version script assignment of 'V1.0' to symbol 'BrotliEncoderHasMoreOutput' failed: symbol not defined [runtime/src/native/libs/build-native.proj]
ld.lld : error : version script assignment of 'V1.0' to symbol 'BrotliEncoderSetParameter' failed: symbol not defined [runtime/src/native/libs/build-native.proj]
clang-18 : error : linker command failed with exit code 1 (use -v to see invocation) [runtime/src/native/libs/build-native.proj]

I was able to reproduce this issue with clang and lld 18 installed and by running:

./build.sh --cmakeargs -DCLR_CMAKE_USE_SYSTEM_BROTLI=true

The system version of libbrotli has symbols without the `V1.0` version.
Support linking against that by removing the steps to use the versioned
exports file.

Without this fix, the build only works with bfd linker (I am not sure
how). When lld is used, the build errors out:

	ld.lld : error : version script assignment of 'V1.0' to symbol 'BrotliDecoderCreateInstance' failed: symbol not defined [runtime/src/native/libs/build-native.proj]
	ld.lld : error : version script assignment of 'V1.0' to symbol 'BrotliDecoderDecompress' failed: symbol not defined [runtime/src/native/libs/build-native.proj]
	ld.lld : error : version script assignment of 'V1.0' to symbol 'BrotliDecoderDecompressStream' failed: symbol not defined [runtime/src/native/libs/build-native.proj]
	ld.lld : error : version script assignment of 'V1.0' to symbol 'BrotliDecoderDestroyInstance' failed: symbol not defined [runtime/src/native/libs/build-native.proj]
	ld.lld : error : version script assignment of 'V1.0' to symbol 'BrotliDecoderIsFinished' failed: symbol not defined [runtime/src/native/libs/build-native.proj]
	ld.lld : error : version script assignment of 'V1.0' to symbol 'BrotliEncoderCompress' failed: symbol not defined [runtime/src/native/libs/build-native.proj]
	ld.lld : error : version script assignment of 'V1.0' to symbol 'BrotliEncoderCompressStream' failed: symbol not defined [runtime/src/native/libs/build-native.proj]
	ld.lld : error : version script assignment of 'V1.0' to symbol 'BrotliEncoderCreateInstance' failed: symbol not defined [runtime/src/native/libs/build-native.proj]
	ld.lld : error : version script assignment of 'V1.0' to symbol 'BrotliEncoderDestroyInstance' failed: symbol not defined [runtime/src/native/libs/build-native.proj]
	ld.lld : error : version script assignment of 'V1.0' to symbol 'BrotliEncoderHasMoreOutput' failed: symbol not defined [runtime/src/native/libs/build-native.proj]
	ld.lld : error : version script assignment of 'V1.0' to symbol 'BrotliEncoderSetParameter' failed: symbol not defined [runtime/src/native/libs/build-native.proj]
	clang-18 : error : linker command failed with exit code 1 (use -v to see invocation) [runtime/src/native/libs/build-native.proj]
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Apr 25, 2024
Copy link
Contributor

Tagging subscribers to this area: @dotnet/area-system-io-compression
See info in area-owners.md if you want to be subscribed.

@jkotas
Copy link
Member

jkotas commented Apr 26, 2024

Have you verified that the binaries produced with this fix and -DCLR_CMAKE_USE_SYSTEM_BROTLI=true option are functional?

@omajid
Copy link
Member Author

omajid commented Apr 26, 2024

Unit tests passed.

$ cd ./src/libraries/System.IO.Compression.Brotli/tests/
$ ../../../../.dotnet/dotnet dotnet build /t:Test

  Determining projects to restore...
  Restored /home/omajid/devel/dotnet/runtime/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/Microsoft.Interop.SourceGeneration.csproj (in 140 ms).
  Restored /home/omajid/devel/dotnet/runtime/src/libraries/Common/tests/StreamConformanceTests/StreamConformanceTests.csproj (in 133 ms).
  Restored /home/omajid/devel/dotnet/runtime/src/libraries/Common/tests/TestUtilities/TestUtilities.csproj (in 133 ms).
  Restored /home/omajid/devel/dotnet/runtime/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/LibraryImportGenerator.csproj (in 139 ms).
  Restored /home/omajid/devel/dotnet/runtime/src/libraries/System.IO.Compression.Brotli/tests/System.IO.Compression.Brotli.Tests.csproj (in 1.43 sec).
  TestUtilities -> /home/omajid/devel/dotnet/runtime/artifacts/bin/TestUtilities/Debug/net8.0/TestUtilities.dll
  StreamConformanceTests -> /home/omajid/devel/dotnet/runtime/artifacts/bin/StreamConformanceTests/Debug/net9.0/StreamConformanceTests.dll
  System.IO.Compression.Brotli.Tests -> /home/omajid/devel/dotnet/runtime/artifacts/bin/System.IO.Compression.Brotli.Tests/Debug/net9.0-unix/System.IO.Compression.Brotli.Tests.dll
  ========================= Begin custom configuration settings ==============================
  export __IsXUnitLogCheckerSupported=1
  ========================== End custom configuration settings ===============================
  ----- start Fri 26 Apr 2024 06:08:26 PM EDT =============== To repro directly: =====================================================
  pushd /home/omajid/devel/dotnet/runtime/artifacts/bin/System.IO.Compression.Brotli.Tests/Debug/net9.0-unix
  /home/omajid/devel/dotnet/runtime/artifacts/bin/testhost/net9.0-linux-Debug-x64/dotnet exec --runtimeconfig System.IO.Compression.Brotli.Tests.runtimeconfig.json --depsfile System.IO.Compression.Brotli.Tests.deps.json xunit.console.dll System.IO.Compression.Brotli.Tests.dll -xml testResults.xml -nologo -notrait category=OuterLoop -notrait category=failing
  popd
  ===========================================================================================================
  ~/devel/dotnet/runtime/artifacts/bin/System.IO.Compression.Brotli.Tests/Debug/net9.0-unix ~/devel/dotnet/runtime/src/libraries/System.IO.Compression.Brotli/tests
    Discovering: System.IO.Compression.Brotli.Tests (method display = ClassAndMethod, method display options = None)
    Discovered:  System.IO.Compression.Brotli.Tests (found 71 of 82 test cases)
    Starting:    System.IO.Compression.Brotli.Tests (parallel test collections = on [20 threads], stop on fail = off)
    Finished:    System.IO.Compression.Brotli.Tests
  === TEST EXECUTION SUMMARY ===
     System.IO.Compression.Brotli.Tests  Total: 247, Errors: 0, Failed: 0, Skipped: 0, Time: 10.165s
  ~/devel/dotnet/runtime/src/libraries/System.IO.Compression.Brotli/tests
  ----- end Fri 26 Apr 2024 06:08:46 PM EDT ----- exit code 0 ----------------------------------------------------------
  exit code 0 means Exited Successfully

Build succeeded.
    0 Warning(s)
    0 Error(s)

Time Elapsed 00:00:23.76

I am trying to use a (VMR) .NET built with change to build itself (in progress at the moment). Any other suggestions on how to test it?

@jkotas
Copy link
Member

jkotas commented Apr 27, 2024

Unit tests passed.

Hmm, I expected that the tests are going to fail with CLR_CMAKE_USE_SYSTEM_BROTLI after this change. I do not see how the Brotli symbols get exported from System.IO.Compression.Native after this change. How does dlsym(dlopen("...libSystem.IO.Compression.Native.so", RTLD_LAZY), "BrotliDecoderCreateInstance") work after this change with CLR_CMAKE_USE_SYSTEM_BROTLI?

@omajid
Copy link
Member Author

omajid commented Apr 29, 2024

I do not see how the Brotli symbols get exported from System.IO.Compression.Native after this change

$ ldd ./artifacts/bin/runtime/net9.0-linux-Debug-x64/libSystem.IO.Compression.Native.so
        linux-vdso.so.1 (0x00007ffecaffd000)
        libz.so.1 => /lib64/libz.so.1 (0x00007f08a63f3000)
        libm.so.6 => /lib64/libm.so.6 (0x00007f08a6310000)
        libbrotlidec.so.1 => /lib64/libbrotlidec.so.1 (0x00007f08a6302000)
        libbrotlienc.so.1 => /lib64/libbrotlienc.so.1 (0x00007f08a6262000)
        libc.so.6 => /lib64/libc.so.6 (0x00007f08a6075000)
        /lib64/ld-linux-x86-64.so.2 (0x00007f08a6433000)
        libbrotlicommon.so.1 => /lib64/libbrotlicommon.so.1 (0x00007f08a6050000)

The linking is ldd-visible. I wrote a program to confirm that the symbols can be found:

$ cat test.c
#include <dlfcn.h>
#include <stddef.h>
#include <stdio.h>

int main(int argc, char* argv[])
{

    char* library_name = NULL;
    if (argc == 1)
    {
        library_name = "./artifacts/bin/runtime/net9.0-linux-Debug-x64/libSystem.IO.Compression.Native.so";
    }
    else
    {
        library_name = argv[1];
    }

    void* library = dlopen(library_name, RTLD_LAZY);
    if (library == NULL)
    {
        dlerror();
        perror("error:");
        return 1;
    }

    printf("opened %s at %x\n", library_name, library);

    char* function_name = "BrotliDecoderCreateInstance";
    void* function = dlsym(library, function_name);
    if (function == NULL)
    {
        dlerror();
        perror("error:");
        return 2;
    }

    printf("%s is at %x\n", function_name, function);

    return 0;
}
$ gcc -o ./test ./test.c
$ ./test
opened ./artifacts/bin/runtime/net9.0-linux-Debug-x64/libSystem.IO.Compression.Native.so at 43b300
BrotliDecoderCreateInstance is at b4092b10

Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

LGTM

@jkotas jkotas merged commit bb61fb5 into dotnet:main Apr 30, 2024
155 of 160 checks passed
michaelgsharp pushed a commit to michaelgsharp/runtime that referenced this pull request May 9, 2024
The system version of libbrotli has symbols without the `V1.0` version.
Support linking against that by removing the steps to use the versioned
exports file.

Without this fix, the build only works with bfd linker (I am not sure
how). When lld is used, the build errors out:

	ld.lld : error : version script assignment of 'V1.0' to symbol 'BrotliDecoderCreateInstance' failed: symbol not defined [runtime/src/native/libs/build-native.proj]
	ld.lld : error : version script assignment of 'V1.0' to symbol 'BrotliDecoderDecompress' failed: symbol not defined [runtime/src/native/libs/build-native.proj]
	ld.lld : error : version script assignment of 'V1.0' to symbol 'BrotliDecoderDecompressStream' failed: symbol not defined [runtime/src/native/libs/build-native.proj]
	ld.lld : error : version script assignment of 'V1.0' to symbol 'BrotliDecoderDestroyInstance' failed: symbol not defined [runtime/src/native/libs/build-native.proj]
	ld.lld : error : version script assignment of 'V1.0' to symbol 'BrotliDecoderIsFinished' failed: symbol not defined [runtime/src/native/libs/build-native.proj]
	ld.lld : error : version script assignment of 'V1.0' to symbol 'BrotliEncoderCompress' failed: symbol not defined [runtime/src/native/libs/build-native.proj]
	ld.lld : error : version script assignment of 'V1.0' to symbol 'BrotliEncoderCompressStream' failed: symbol not defined [runtime/src/native/libs/build-native.proj]
	ld.lld : error : version script assignment of 'V1.0' to symbol 'BrotliEncoderCreateInstance' failed: symbol not defined [runtime/src/native/libs/build-native.proj]
	ld.lld : error : version script assignment of 'V1.0' to symbol 'BrotliEncoderDestroyInstance' failed: symbol not defined [runtime/src/native/libs/build-native.proj]
	ld.lld : error : version script assignment of 'V1.0' to symbol 'BrotliEncoderHasMoreOutput' failed: symbol not defined [runtime/src/native/libs/build-native.proj]
	ld.lld : error : version script assignment of 'V1.0' to symbol 'BrotliEncoderSetParameter' failed: symbol not defined [runtime/src/native/libs/build-native.proj]
	clang-18 : error : linker command failed with exit code 1 (use -v to see invocation) [runtime/src/native/libs/build-native.proj]
Ruihan-Yin pushed a commit to Ruihan-Yin/runtime that referenced this pull request May 30, 2024
The system version of libbrotli has symbols without the `V1.0` version.
Support linking against that by removing the steps to use the versioned
exports file.

Without this fix, the build only works with bfd linker (I am not sure
how). When lld is used, the build errors out:

	ld.lld : error : version script assignment of 'V1.0' to symbol 'BrotliDecoderCreateInstance' failed: symbol not defined [runtime/src/native/libs/build-native.proj]
	ld.lld : error : version script assignment of 'V1.0' to symbol 'BrotliDecoderDecompress' failed: symbol not defined [runtime/src/native/libs/build-native.proj]
	ld.lld : error : version script assignment of 'V1.0' to symbol 'BrotliDecoderDecompressStream' failed: symbol not defined [runtime/src/native/libs/build-native.proj]
	ld.lld : error : version script assignment of 'V1.0' to symbol 'BrotliDecoderDestroyInstance' failed: symbol not defined [runtime/src/native/libs/build-native.proj]
	ld.lld : error : version script assignment of 'V1.0' to symbol 'BrotliDecoderIsFinished' failed: symbol not defined [runtime/src/native/libs/build-native.proj]
	ld.lld : error : version script assignment of 'V1.0' to symbol 'BrotliEncoderCompress' failed: symbol not defined [runtime/src/native/libs/build-native.proj]
	ld.lld : error : version script assignment of 'V1.0' to symbol 'BrotliEncoderCompressStream' failed: symbol not defined [runtime/src/native/libs/build-native.proj]
	ld.lld : error : version script assignment of 'V1.0' to symbol 'BrotliEncoderCreateInstance' failed: symbol not defined [runtime/src/native/libs/build-native.proj]
	ld.lld : error : version script assignment of 'V1.0' to symbol 'BrotliEncoderDestroyInstance' failed: symbol not defined [runtime/src/native/libs/build-native.proj]
	ld.lld : error : version script assignment of 'V1.0' to symbol 'BrotliEncoderHasMoreOutput' failed: symbol not defined [runtime/src/native/libs/build-native.proj]
	ld.lld : error : version script assignment of 'V1.0' to symbol 'BrotliEncoderSetParameter' failed: symbol not defined [runtime/src/native/libs/build-native.proj]
	clang-18 : error : linker command failed with exit code 1 (use -v to see invocation) [runtime/src/native/libs/build-native.proj]
@github-actions github-actions bot locked and limited conversation to collaborators May 31, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.IO.Compression community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants