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

[build] Install mingw32 *.a files in private_libdir #51698

Merged
merged 8 commits into from
Oct 19, 2023

Conversation

giordano
Copy link
Contributor

@giordano giordano commented Oct 13, 2023

This avoid that these files are picked up during julia's build process, and are instead only used to link pkgimages, as intended.

I can't test this PR myself, so someone else would have to tell me if it works to make building julia work again.

Do I understand correctly this would address #48081? Edit: yes, according to #51698 (comment) this PR fixes #48081.

@giordano giordano added building Build system, or building Julia or its dependencies system:windows Affects only Windows backport 1.9 Change should be backported to release-1.9 backport 1.10 Change should be backported to the 1.10 release labels Oct 13, 2023
Copy link
Member

@vtjnash vtjnash left a comment

Choose a reason for hiding this comment

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

This is too late to move them, as the damage is already been done on the compiler side. After this step the compiler is no longer involved anyways

@gbaraldi
Copy link
Member

Why not install them in libexec? Only lld uses them anyway?

@giordano giordano removed backport 1.9 Change should be backported to release-1.9 backport 1.10 Change should be backported to the 1.10 release labels Oct 15, 2023
@giordano
Copy link
Contributor Author

I updated CSL to v1.1.0 from JuliaPackaging/Yggdrasil#7535 which changes the tree structure of the *.a files in the tarball itself, with the hope that now they aren't at any point in the compiler search path during julia's build process.

@maleadt
Copy link
Member

maleadt commented Oct 16, 2023

With this PR, I don't get undefined references to strtoull anymore, so I guess this fixes #48081!

Compilation still fails when using GCC 13, but that's probably unrelated (and compiling with the mingw mibmsvcrt.a in place doesn't change this error):

C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/13.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: ./codegen.o: in function `add_named_global':
C:/Users/Tim/Julia/src/julia/src/codegen.cpp:549:(.text+0x1e413): undefined reference to `__imp___stack_chk_guard'
collect2.exe: error: ld returned 1 exit status

@giordano
Copy link
Contributor Author

giordano commented Oct 16, 2023

I don't know why build in CI is failing with

lld: error: unable to find library -lssp
lld: error: unable to find library -lgcc_s
lld: error: unable to find library -lgcc
lld: error: unable to find library -lmsvcrt

during precompilation of stdlibs: are the files not moved to private_libdir yet at this point? How did it work before?

@vtjnash
Copy link
Member

vtjnash commented Oct 16, 2023

The stack_chk_guard variable should be from libjulia-internal (rtutils.c), so maybe check why that is missing the export for it? We might be accidentally filtering it out. Most compiler builds also have -lssp though some don't, so that might have been hiding the issue before (we don't really care which one gets picked up by the linker).

@maleadt
Copy link
Member

maleadt commented Oct 17, 2023

And FWIW, these libraries are needed. If I remove them from base/linking.jl, I get:

    JULIA stdlib/ArgTools.release.image
failed process: Process(lld: error: undefined symbol: ___chkstk_ms
>>> referenced by array.jl:1019
>>>               jl_C5DB.tmp(text#0.o):(julia_collect_1385)
>>> referenced by array.jl:1019
>>>               jl_C5DB.tmp(text#0.o):(julia_collect_1385)
>>> referenced by array.jl:1024
>>>               jl_C5DB.tmp(text#0.o):(julia_maxstats_1157)
>>> referenced 554 more times

lld: error: undefined symbol: memset
>>> referenced by cmem.jl:39
>>>               jl_C5DB.tmp(text#1.o):(julia_Dict_1786)
>>> referenced by cmem.jl:39
>>>               jl_C5DB.tmp(text#1.o):(julia_rehashNOT._1363)
>>> referenced by cmem.jl:39
>>>               jl_C5DB.tmp(text#1.o):(julia_rehashNOT._1363)
>>> referenced 22 more times

lld: error: undefined symbol: memmove
>>> referenced by cmem.jl:26
>>>               jl_C5DB.tmp(text#1.o):(japi1_YY.sortYY.30_1575)
>>> referenced by cmem.jl:26
>>>               jl_C5DB.tmp(text#1.o):(japi1__string_1826)
>>> referenced by cmem.jl:26
>>>               jl_C5DB.tmp(text#1.o):(japi1__string_1826)
>>> referenced 22 more times

lld: error: undefined symbol: memcpy
>>> referenced by jl_C5DB.tmp(text#6.o):(jfptr_parse_flat_932)
>>> referenced by jl_C5DB.tmp(text#7.o):(jfptr_decode_alloc_587)
>>> referenced by jl_C5DB.tmp(text#10.o):(jfptr_parse_flat_948)
setenv(`'C:\Users\Tim\Julia\src\julia\usr\tools\lld.exe' -flavor gnu -m i386pep -Bdynamic --enable-auto-image-base --allow-multiple-definition '' -shared -o 'C:/Users/Tim/Julia/src/julia/usr/share/julia\compiled\v1.11\ArgTools\jl_C2DF.tmp' --whole-archive 'C:/Users/Tim/Julia/src/julia/usr/share/julia\compiled\v1.11\ArgTools\jl_C2DE.tmp' --no-whole-archive '-LC:\Users\Tim\Julia\src\julia\usr\lib' '-LC:\Users\Tim\Julia\src\julia\usr\lib\julia' '-LC:\Users\Tim\Julia\src\julia\usr\bin' -ljulia -ljulia-internal -lopenlibm`,["MFLAGS=-s -j", "WINDIR=C:\\Windows", "PATH=C:\\Users\\Tim\\Julia\\src\\julia\\usr\\lib\\julia;C:\\Users\\Tim\\Julia\\src\\julia\\usr\\bin;C:\\buildkite-agent\\bin;C:\\msys64\\mingw64\\bin;C:\\msys64\\usr\\bin;C:\\Windows\\system32;C:\\Windows;C:\\Windows\\System32\\Wbem;C:\\Windows\\System32\\WindowsPowerShell\\v1.0;C:\\Windows\\System32\\OpenSSH;C:\\Users\\ContainerAdministrator\\AppData\\Local\\Microsoft\\WindowsApps;C:\\Program Files (x86)\\Windows Kits\\10\\bin\\10.0.22621.0\\x64;C:\\Program Files\\Amazon\\AWSCLIV2", "LOCALAPPDATA=C:\\Users\\ContainerAdministrator\\AppData\\Local", "BUILDROOT=C:/Users/Tim/Julia/src/julia", "MAKEFLAGS=s -j", "PROCESSOR_IDENTIFIER=AMD64 Family 25 Model 33 Stepping 0, AuthenticAMD", "NUMBER_OF_PROCESSORS=32", "LC_ALL=C", "PATHEXT=.COM;.EXE;.BAT;.CMD;.VBS;.VBE;.JS;.JSE;.WSF;.WSH;.MSC"  …  "JULIA_LOAD_PATH=@stdlib", "PROCESSOR_LEVEL=25", "SYSTEMDRIVE=C:", "MAKE_TERMOUT=/dev/cons0", "PROGRAMW6432=C:\\Program Files", "HOME=C:\\msys64\\home\\ContainerUser", "TEMP=C:\\Users\\ContainerAdministrator\\AppData\\Local\\Temp", "PROCESSOR_ARCHITECTURE=AMD64", "OPENBLAS_MAIN_FREE=1", "TERM=xterm-256color"]), ProcessExited(1)) [1]

... and a bunch of others as well.

@giordano
Copy link
Contributor Author

And FWIW, these libraries are needed:

Needed for what? Only linking pkgimages, no? We didn't have them before pkgimages, and everybody was happy.

@maleadt
Copy link
Member

maleadt commented Oct 17, 2023

Only linking pkgimages, no?

Yes. I've updated my comment above.

@gbaraldi
Copy link
Member

If only building compiler-rt builtins wasn't the hardest thing in the world. I have no clue how zig got it working and I'm slightly scared to find out

@giordano
Copy link
Contributor Author

@maleadt where have been the files installed? I was expecting to find them in private_libdir (==usr/lib/julia), which is in the pkgimages linker path:

PRIVATE_LIBDIR = "-L$(private_libdir())"
but the build failure suggests they haven't been moved there (yet?) and I'm lost.

@maleadt
Copy link
Member

maleadt commented Oct 17, 2023

# make cleanall && make -C deps install-csl
# find usr
usr
usr/bin
usr/bin/libatomic-1.dll
usr/bin/libgcc_s_seh-1.dll
usr/bin/libgfortran-5.dll
usr/bin/libgomp-1.dll
usr/bin/libquadmath-0.dll
usr/bin/libssp-0.dll
usr/bin/libstdc++-6.dll
usr/bin/libwinpthread-1.dll
usr/lib
usr/lib/gcc
usr/lib/gcc/x86_64-w64-mingw32
usr/lib/gcc/x86_64-w64-mingw32/13
usr/lib/gcc/x86_64-w64-mingw32/13/libgcc.a
usr/lib/gcc/x86_64-w64-mingw32/13/libgcc_s.a
usr/lib/gcc/x86_64-w64-mingw32/13/libmsvcrt.a
usr/lib/gcc/x86_64-w64-mingw32/13/libssp.dll.a
...

@giordano
Copy link
Contributor Author

Cool, so these lines

julia/Makefile

Lines 296 to 301 in 4ff66ed

# The rest are libraries from mingw32 needed to link pkgimages, as an example memcpy is exported by msvcrt.
# These files must be outside of the search path of the compiler used to build julia itself,
# but should be picked up by the pkgimages linker.
-$(INSTALL_M) $(build_libdir)/gcc/$(BB_TRIPLET)/13/libgcc_s.a $(DESTDIR)$(private_libdir)/
-$(INSTALL_M) $(build_libdir)/gcc/$(BB_TRIPLET)/13/libgcc.a $(DESTDIR)$(private_libdir)/
-$(INSTALL_M) $(build_libdir)/gcc/$(BB_TRIPLET)/13/libmsvcrt.a $(DESTDIR)$(private_libdir)/
are never executed?

@maleadt
Copy link
Member

maleadt commented Oct 17, 2023

AFAICT those are in the install target, which isn't typically executed when building Julia, right?
Furthermore, they happen after building Julia, which is what fails here in the first place.

@giordano
Copy link
Contributor Author

@vtjnash can you suggest any other rule where we can do the move? Otherwise we never get the files where we need them.

@gbaraldi
Copy link
Member

We can move them during the deps part?

@maleadt
Copy link
Member

maleadt commented Oct 17, 2023

Yeah that seems to work. I did a quick test with:

diff --git a/deps/csl.mk b/deps/csl.mk
index 37956ba5c3..81c88e0da7 100644
--- a/deps/csl.mk
+++ b/deps/csl.mk
@@ -104,4 +104,10 @@ distclean-csl: clean-csl

 else
 $(eval $(call bb-install,csl,CSL,true))
+install-csl:
+       mkdir -p $(build_private_libdir)/
+       cp -a $(build_libdir)/gcc/$(BB_TRIPLET)/13/libgcc_s.a $(build_private_libdir)/
+       cp -a $(build_libdir)/gcc/$(BB_TRIPLET)/13/libgcc.a $(build_private_libdir)/
+       cp -a $(build_libdir)/gcc/$(BB_TRIPLET)/13/libmsvcrt.a $(build_private_libdir)/
+       cp -a $(build_libdir)/gcc/$(BB_TRIPLET)/13/libssp.dll.a $(build_private_libdir)/
 endif

@vtjnash
Copy link
Member

vtjnash commented Oct 17, 2023

You will muck up the make uninstall step if you move them, which means any later upgrades to CSL will run into problems. Files should be left where they are found.

@giordano giordano force-pushed the mg/win-a-libdir branch 2 times, most recently from d7775e6 to fdf084e Compare October 17, 2023 22:03
deps/csl.mk Show resolved Hide resolved
@maleadt
Copy link
Member

maleadt commented Oct 18, 2023

@giordano Is there a reason you didn't put those files in the correct spot already for the JLL?

@giordano
Copy link
Contributor Author

What's a correct spot? Point is, they have to be in a place which isn't picked up by the linker during Julia's own build process, but can be used by pkgimages linker (but we have some more control for this). I'm not aware of any directory which would magically work this way.

@maleadt maleadt force-pushed the mg/win-a-libdir branch 2 times, most recently from 3d7bce2 to 476262c Compare October 18, 2023 11:08
giordano and others added 8 commits October 18, 2023 16:59
This avoid that these files are picked up during julia's build process, and are
instead only used to link pkgimages, as intended.
This release

* upgrades GCC libraries to GCC 13
* has a new tree structure for GCC `*.a` libraries for Windows
Co-authored-by: Tim Besard <tim.besard@gmail.com>
@maleadt maleadt merged commit 4ef353c into JuliaLang:master Oct 19, 2023
6 checks passed
@giordano giordano deleted the mg/win-a-libdir branch October 19, 2023 08:38
@jaakkor2 jaakkor2 mentioned this pull request Feb 3, 2024
33 tasks
KristofferC pushed a commit that referenced this pull request Feb 5, 2024
This avoid that these files are picked up during julia's build process,
and are instead only used to link pkgimages, as intended.

Co-authored-by: Tim Besard <tim.besard@gmail.com>
(cherry picked from commit 4ef353c)
Drvi pushed a commit to RelationalAI/julia that referenced this pull request Jun 7, 2024
This avoid that these files are picked up during julia's build process,
and are instead only used to link pkgimages, as intended.

Co-authored-by: Tim Besard <tim.besard@gmail.com>
(cherry picked from commit 4ef353c)
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 system:windows Affects only Windows
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CompilerSupportLibraries' libmsvcrt.a and msys2+mingw64 build failed
4 participants