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

Building Julia and LLVM from source is broken on master #55337

Closed
giordano opened this issue Aug 1, 2024 · 9 comments · Fixed by #55354
Closed

Building Julia and LLVM from source is broken on master #55337

giordano opened this issue Aug 1, 2024 · 9 comments · Fixed by #55354
Labels
building Build system, or building Julia or its dependencies
Milestone

Comments

@giordano
Copy link
Contributor

giordano commented Aug 1, 2024

Building Julia on 0ef8a91 with the following Make.user

USE_BINARYBUILDER_LLVM = 0
DEPS_GIT = llvm

crashes for me on aarch64-linux-gnu with

[...]
    LINK usr/lib/libjulia-codegen.so.1.12.0
    LINK usr/lib/libjulia-codegen.so
    LINK usr/lib/libjulia-codegen.so.1.12
    JULIA usr/lib/julia/corecompiler.ji

[1217093] signal 6 (-6): Aborted
in expression starting at boot.jl:264
__pthread_kill_implementation at /lib64/libc.so.6 (unknown line)
raise at /lib64/libc.so.6 (unknown line)
abort at /lib64/libc.so.6 (unknown line)
_ZNK4llvm25AMDGPUInstructionSelector15runCustomActionEjRKNS_20GIMatchTableExecutor12MatcherStateE at /data/cceamgi/repo/julia/usr/bin/../lib/libLLVM-17jl.so (unknown line)
registerJITObject at /data/cceamgi/repo/julia/src/debuginfo.cpp:340
unknown function (ip: 0xffffabf83cbf)
unknown function (ip: 0xffffabf83cbf)
Allocations: 0 (Pool: 0; Big: 0); GC: 0
make[1]: *** [sysimage.mk:64: /data/cceamgi/repo/julia/usr/lib/julia/corecompiler.ji] Error 134
make: *** [Makefile:107: julia-sysimg-ji] Error 2

Instead, compilation with the same configuration is successful on d68befd, which is the parent of the merge commit of #55180, which @gbaraldi identified as possible culprit. I don't have the time to do a full git bisect right now, perhaps I'll be able to do it at a later point. Side note, JuliaCI/julia-buildkite#373 also identified #55180 as possible culprit (or at least related).

@giordano giordano added the building Build system, or building Julia or its dependencies label Aug 1, 2024
@giordano
Copy link
Contributor Author

giordano commented Aug 1, 2024

Confirmed with git bisect

$ cat bisect.sh
#!/bin/bash
make cleanall || true
make -j60 USE_BINARYBUILDER_LLVM=0 DEPS_GIT=llvm
$ git bisect start
$ git bisect good d68befdc0d3479cc3cb8cce64aa8c380253d2f5d
$ git bisect bad 0ef8a91e490dfaa90cb6e4781ca0445c7f991933
$ git bisect start ./bisect.sh

that the first broken commit is fe597c1 is the first bad commit (#55180). CC: @vtjnash.

@vtjnash
Copy link
Member

vtjnash commented Aug 1, 2024

Did you mistakenly build without zlib support? I didn't handle that case, as I didn't think anyone would do that

@giordano
Copy link
Contributor Author

giordano commented Aug 1, 2024

Did you mistakenly build without zlib support?

I didn't change anything in the build system of LLVM or Julia, I showed above all the options I used.

@giordano
Copy link
Contributor Author

giordano commented Aug 1, 2024

And also JuliaCI/julia-buildkite#373 shows that a from build source build of julia and llvm has similar issues. It's probably the same issue as this one (all errors are in the same places), but it shows that the standard configuration of the llvm build system within julia is not doing something correctly and this is independently reproducible by multiple users..

@vtjnash
Copy link
Member

vtjnash commented Aug 1, 2024

#47727 ?

@giordano
Copy link
Contributor Author

giordano commented Aug 1, 2024

What I'm trying to convey is that the default build of llvm from the build system we have here produces a non functional llvm, and now it does so in a dramatic way by breaking Julia build itself. I'm not contesting that a non functional llvm isn't good.

@giordano
Copy link
Contributor Author

giordano commented Aug 2, 2024

It turned out this was broken by #42622: setting ZLIB_LIBRARY to a directory path is wrong, it should have been the path to the library file itself. A better solution is to use ZLIB_ROOT, which is the CMake-blessed way to suggest where Zlib is installed.

@vtjnash
Copy link
Member

vtjnash commented Aug 2, 2024

I don't have a build for testing this, but this should also work to disable zlib compression when not available:

diff --git a/src/debuginfo.cpp b/src/debuginfo.cpp
index 8455081107..53081bd77a 100644
--- a/src/debuginfo.cpp
+++ b/src/debuginfo.cpp
@@ -337,9 +337,16 @@ void JITDebugInfoRegistry::registerJITObject(const object::ObjectFile &Object,
 #endif // defined(_OS_WINDOWS_)
 
     SmallVector<uint8_t, 0> packed;
-    compression::zlib::compress(ArrayRef<uint8_t>((uint8_t*)Object.getData().data(), Object.getData().size()), packed, compression::zlib::DefaultCompression);
+    auto uncompressedsize = Object.getData().size();
+    if (compression::zlib::isAvailable())
+        compression::zlib::compress(ArrayRef<uint8_t>((uint8_t*)Object.getData().data(), uncompressedsize), packed, compression::zlib::DefaultCompression);
+    else {
+        packed.append(Object.getData().begin(), Object.getData().end());
+        uncompressedsize = 0;
+    }
     jl_jit_add_bytes(packed.size());
-    auto ObjectCopy = new LazyObjectInfo{packed, Object.getData().size()}; // intentionally leaked so that we don't need to ref-count it, intentionally copied so that we exact-size the allocation (since no shrink_to_fit function)
+    auto ObjectCopy = new LazyObjectInfo{packed, uncompressedsize}; // intentionally leaked so that we don't need to ref-count it, intentionally copied so that we exact-size the allocation (since no shrink_to_fit function)
+
     auto symbols = object::computeSymbolSizes(Object);
     bool hassection = false;
     for (const auto &sym_size : symbols) {

@giordano
Copy link
Contributor Author

giordano commented Aug 2, 2024

I don't have a build for testing this

That's easy to resolve: just run make -j USE_BINARYBUILDER_LLVM=0 🙂

but this should also work to disable zlib compression when not available:

I can confirm this doesn't make julia compilation fail with USE_BINARYBUILDER_LLVM=0. I think #55354 is still worthwhile to get in because we're passing wrong options around, but checking if zlib is actually available sounds good to do too.

giordano added a commit that referenced this issue Aug 6, 2024
After #55180 we implicitly require an LLVM built with Zlib support, but
compiling Julia with `make USE_BINARYBUILDER_LLVM=0` builds an LLVM
without Zlib support, despite the fact we attempt to request it at

https://github.com/JuliaLang/julia/blob/996351f5f6651d1508aef3c35c7d37eb22a0fb1e/deps/llvm.mk#L97
This was first identified in #55337.

`ZLIB_LIBRARY` must be the path to the zlib library, but we currently
set it to the libdir where the library is installed (introduced in

https://github.com/JuliaLang/julia/blob/996351f5f6651d1508aef3c35c7d37eb22a0fb1e/deps/llvm.mk#L97
which is wrong. However, CMake is actually able to find Zlib correctly,
but then the check at
https://github.com/llvm/llvm-project/blob/46425b8d0fac3c529aa4a716d19abd7032e452f3/llvm/cmake/config-ix.cmake#L139-L141
uses the value of `ZLIB_LIBRARY` to list the Zlib to link for the test,
but being `ZLIB_LIBRARY` a directory, CMake doesn't see any valid Zlib
and thus tries to run the test without linking any Zlib, and the test
silently fails (they're silent only when `LLVM_ENABLE_ZLIB=ON`),
resulting in no usable Zlib available, even if found.

`ZLIB_ROOT` is the only [hint recommended by the CMake module
`FindZLIB`](https://cmake.org/cmake/help/latest/module/FindZLIB.html#hints).
This PR replaces a broken `ZLIB_LIBRARY` with an appropriate
`ZLIB_ROOT`. Also, we set `LLVM_ENABLE_ZLIB=FORCE_ON` which is the only
way to make CMake fail loudly if no usable Zlib is available, and avoid
going on with a non-usable build.

I confirm this fixes #55337 for me, it should likely address
JuliaCI/julia-buildkite#373 as well.

Also, options `COMPILER_RT_ENABLE_IOS`, `COMPILER_RT_ENABLE_WATCHOS`,
`COMPILER_RT_ENABLE_TVOS`, and `HAVE_HISTEDIT_H` don't exist anymore,
and they are removed.
giordano added a commit that referenced this issue Aug 6, 2024
After #55180 we implicitly require an LLVM built with Zlib support, but
compiling Julia with `make USE_BINARYBUILDER_LLVM=0` builds an LLVM
without Zlib support, despite the fact we attempt to request it at

https://github.com/JuliaLang/julia/blob/996351f5f6651d1508aef3c35c7d37eb22a0fb1e/deps/llvm.mk#L97
This was first identified in #55337.

`ZLIB_LIBRARY` must be the path to the zlib library, but we currently
set it to the libdir where the library is installed (introduced in

https://github.com/JuliaLang/julia/blob/996351f5f6651d1508aef3c35c7d37eb22a0fb1e/deps/llvm.mk#L97
which is wrong. However, CMake is actually able to find Zlib correctly,
but then the check at
https://github.com/llvm/llvm-project/blob/46425b8d0fac3c529aa4a716d19abd7032e452f3/llvm/cmake/config-ix.cmake#L139-L141
uses the value of `ZLIB_LIBRARY` to list the Zlib to link for the test,
but being `ZLIB_LIBRARY` a directory, CMake doesn't see any valid Zlib
and thus tries to run the test without linking any Zlib, and the test
silently fails (they're silent only when `LLVM_ENABLE_ZLIB=ON`),
resulting in no usable Zlib available, even if found.

`ZLIB_ROOT` is the only [hint recommended by the CMake module
`FindZLIB`](https://cmake.org/cmake/help/latest/module/FindZLIB.html#hints).
This PR replaces a broken `ZLIB_LIBRARY` with an appropriate
`ZLIB_ROOT`. Also, we set `LLVM_ENABLE_ZLIB=FORCE_ON` which is the only
way to make CMake fail loudly if no usable Zlib is available, and avoid
going on with a non-usable build.

I confirm this fixes #55337 for me, it should likely address
JuliaCI/julia-buildkite#373 as well.

Also, options `COMPILER_RT_ENABLE_IOS`, `COMPILER_RT_ENABLE_WATCHOS`,
`COMPILER_RT_ENABLE_TVOS`, and `HAVE_HISTEDIT_H` don't exist anymore,
and they are removed.
lazarusA pushed a commit to lazarusA/julia that referenced this issue Aug 17, 2024
### Elaboration of the issue

After JuliaLang#55180 we implicitly require an LLVM built with Zlib support, but
compiling Julia with `make USE_BINARYBUILDER_LLVM=0` builds an LLVM
without Zlib support, despite the fact we attempt to request it at

https://github.com/JuliaLang/julia/blob/996351f5f6651d1508aef3c35c7d37eb22a0fb1e/deps/llvm.mk#L97
This was first identified in JuliaLang#55337.

### Explanation of how configuration of LLVM failed

`ZLIB_LIBRARY` must be the path to the zlib library, but we currently
set it to the libdir where the library is installed (introduced in
JuliaLang#42622):

https://github.com/JuliaLang/julia/blob/996351f5f6651d1508aef3c35c7d37eb22a0fb1e/deps/llvm.mk#L97
which is wrong. However, CMake is actually able to find Zlib correctly,
but then the check at
https://github.com/llvm/llvm-project/blob/46425b8d0fac3c529aa4a716d19abd7032e452f3/llvm/cmake/config-ix.cmake#L139-L141
uses the value of `ZLIB_LIBRARY` to list the Zlib to link for the test,
but being `ZLIB_LIBRARY` a directory, CMake doesn't see any valid Zlib
and thus tries to run the test without linking any Zlib, and the test
silently fails (they're silent only when `LLVM_ENABLE_ZLIB=ON`),
resulting in no usable Zlib available, even if found.

### Proposed solution

`ZLIB_ROOT` is the only [hint recommended by the CMake module
`FindZLIB`](https://cmake.org/cmake/help/latest/module/FindZLIB.html#hints).
This PR replaces a broken `ZLIB_LIBRARY` with an appropriate
`ZLIB_ROOT`. Also, we set `LLVM_ENABLE_ZLIB=FORCE_ON` which is the only
way to make CMake fail loudly if no usable Zlib is available, and avoid
going on with a non-usable build.

### Other comments

I confirm this fixes JuliaLang#55337 for me, it should likely address
JuliaCI/julia-buildkite#373 as well.

Also, options `COMPILER_RT_ENABLE_IOS`, `COMPILER_RT_ENABLE_WATCHOS`,
`COMPILER_RT_ENABLE_TVOS`, and `HAVE_HISTEDIT_H` don't exist anymore,
and they are removed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
building Build system, or building Julia or its dependencies
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants