rust: update to 1.70.0

[filnet]

No description provided.

[filnet]

clang64 fails with:

error: Dlltool could not create import library: OVERVIEW: llvm-dlltool

       USAGE: llvm-dlltool [options] file...

       OPTIONS:
         -D <value> Specify the input DLL Name
         -d <value> Input .def File
         -f <value> Assembler Flags
         -k         Kill @n Symbol from export
         -l <value> Generate an import lib
         -m <value> Set target machine
         -S <value> Assembler

       TARGETS: i386, i386:x86-64, arm, arm64
       \n

error: could not compile `windows` due to previous error

On clang, dlltool.exe is actually llvm-dlltool.exe.

$ dlltool -V
OVERVIEW: llvm-dlltool

USAGE: llvm-dlltool [options] file...

OPTIONS:
  -D <value> Specify the input DLL Name
  -d <value> Input .def File
  -f <value> Assembler Flags
  -k         Kill @n Symbol from export
  -l <value> Generate an import lib
  -m <value> Set target machine
  -S <value> Assembler

TARGETS: i386, i386:x86-64, arm, arm64

While on mingw dlltool.exe is itself.

$ dlltool -V
GNU C:\msys64\mingw64\bin\dlltool.exe (GNU Binutils) 2.40
Copyright (C) 2023 Free Software Foundation, Inc.
This program is free software; you may redistribute it under the terms of
the GNU General Public License version 3 or (at your option) any later version.
This program has absolutely no warranty.

Is the clang behavior new ?

[filnet]

Note that the build failure comes from the bootstrap compiler (the stage 0 compiler).
So the issue might not come from a code change in Rust 1.70.0 but from elsewhere...

[filnet]

$ pkgfile dlltool.exe
mingw64/mingw-w64-x86_64-binutils
clang64/mingw-w64-clang-x86_64-llvm
msys/binutils
msys/mingw-w64-cross-binutils

[ognevny]

maybe #17163 and #17333 influenced the build (there were some changes in binutils packages)

[ognevny]

also it would be better to have llvm-libs as dependency like arch does

[ognevny]

maybe #17163 and #17333 influenced the build (there were some changes in binutils packages)

oops, these was about binutils for gcc toolchain... I think that this commit has really changed things

[jeremyd2019]

On clang, dlltool.exe is actually llvm-dlltool.exe.
Is the clang behavior new ?

No, CLANG* dlltool has always been llvm-dlltool, GNU binutils is not used there. I'm guessing something in rust (or a crate pulled in) is now trying to use a dlltool option that llvm-dlltool doesn't implement. As you can see GNU dlltool has a lot more options, most of which are not all that useful anymore. Wish llvm-dlltool would include what unknown options it was given in the error, rather than just the generic usage information. CC @mstorsjo would probably be interested too

[jeremyd2019]

I noticed error: could not compile windows due to previous error and sure enough, there is a usage of dlltool in windows-rs. But it seems to only be used to make the import libraries, which are shipped in crates. I don't know why that code would be invoked during rust build...

If it were, I could see where it could get confused, because of the hacks that are still present to make CLANG64 use x86_64-pc-windows-gnu instead of -gnullvm. Also, the lack of any i686-pc-windows-gnullvm handling means that CLANG32 must still use i686-pc-windows-gnu. @mati865

I'm guessing around https://github.com/microsoft/windows-rs/blob/0b333d3d9b15bbf8193ded2d776a666137a372f5/crates/tools/gnu/src/main.rs#L110 is where it would start passing unknown args to dlltool.

[filnet]

No, CLANG* dlltool has always been llvm-dlltool, GNU binutils is not used there. I'm guessing something in rust (or a crate pulled in) is now trying to use a dlltool option that llvm-dlltool doesn't implement. As you can see GNU dlltool has a lot more options, most of which are not all that useful anymore. Wish llvm-dlltool would include what unknown options it was given in the error, rather than just the generic usage information. CC @mstorsjo would probably be interested too

Rust does use options that are not advertised by llvm-dlltool. For example here : https://github.com/rust-lang/rust/blob/794249d768a4f112519f22502ade032dc03b9fde/compiler/rustc_codegen_llvm/src/back/archive.rs#L201.

But it is not new, so, yes, maybe the windows crate has started to use it.

[jeremyd2019]

That probably makes more sense... The is_mingw_gnu_toolchain function looks specifically for abi being empty (ie, not -gnullvm), so maybe the hack patch that makes CLANG32/64 work as -gnu needs to patch that function to be false there? Or it's about time to switch to -gnullvm

[jeremyd2019]

happily CLANGARM64 build succeeded on this version

[jeremyd2019]

It seems to be a little of both: it is trying to produce an import library for kernel32 while building the windows crate, but the windows crate itself doesn't seem to be calling dlltool. I'm thinking it must be the raw-dylib thing causing that rust code you linked to to be invoked to create an import library?

Unfortunately, the bootstrap stage0 compiler downloaded is trying to call dlltool, so just patching that code does not work (the compiler with that patch doesn't exist yet). I made a python wrapper script that filtered out the "unknown" options --temp-prefix and --no-leading-underscore (utilizing the old setuptools wrapper exe so that rust can execute it, as opposed to my first attempt of a shell script) and have the compile going along. I logged the arguments to dlltool too so I could see what was going on.

I just tried not filtering out --no-leading-underscore and that was ok, so it must just be --temp-prefix whatever. I don't see why though, because --no-leading-underscore is not in https://github.com/llvm/llvm-project/blob/0a168131b4f420d9f561926c643c143c84c97535/llvm/lib/ToolDrivers/llvm-dlltool/Options.td

[jeremyd2019]

I think something like this patch needs to be added to 0007-clang-subsystem.patch:

--- rustc-1.70.0-src/compiler/rustc_codegen_llvm/src/common.rs.orig     2023-06-02 17:34:23.626069500 -0700
+++ rustc-1.70.0-src/compiler/rustc_codegen_llvm/src/common.rs  2023-06-02 17:35:00.023376400 -0700
@@ -382,7 +382,7 @@
 }

 pub(crate) fn is_mingw_gnu_toolchain(target: &Target) -> bool {
-    target.vendor == "pc" && target.os == "windows" && target.env == "gnu" && target.abi.is_empty()
+    false && target.vendor == "pc" && target.os == "windows" && target.env == "gnu" && target.abi.is_empty()
 }

 pub(crate) fn i686_decorated_name(

Unfortunately due to bootstrapping, this is not sufficient. I see 3 ways around this:

1. patch mingw-w64-clang to add --temp-prefix whatever arg to llvm/lib/ToolDrivers/llvm-dlltool/Options.td, probably just ignoring it. maybe that can be done upstream too
2. add a dlltool wrapper like I made to test, that drops --temp-prefix and the next arg after that, and prepend that to the path during the rust build on CLANG32/64
3. rebuild rust 1.69 with that patch, then update to 1.70 with _bootstrapping=no (a la [WIP] rust: set _bootstrapping=no for all architectures #16207)

[filnet]

Some related info:

• https://kennykerr.ca/rust-getting-started/understanding-windows-targets.html
• https://rust-lang.github.io/rfcs/2627-raw-dylib-kind.html
• Tracking issue for RFC 2627: #[link(kind="raw-dylib")] rust-lang/rust#58713

[filnet]

3. rebuild rust 1.69 with that patch, then update to 1.70 with _bootstrapping=no

I'll give it a shot next week.

[filnet]

1. patch mingw-w64-clang to add `--temp-prefix whatever` arg to `llvm/lib/ToolDrivers/llvm-dlltool/Options.td`, probably just ignoring it.  maybe that can be done upstream too

llvm-dlltool already ignores a couple of options: https://github.com/llvm/llvm-project/blob/0a168131b4f420d9f561926c643c143c84c97535/llvm/lib/ToolDrivers/llvm-dlltool/Options.td#L19

But ignoring more is kind of scary... as they do things.

[jeremyd2019]

I've got a build running now of 1.70.0 with bootstrapping=no with a patched 1.69.0 installed, on CLANG64.

llvm-dlltool already ignores a couple of options: https://github.com/llvm/llvm-project/blob/0a168131b4f420d9f561926c643c143c84c97535/llvm/lib/ToolDrivers/llvm-dlltool/Options.td#L19

But ignoring more is kind of scary... as they do things.

This is why I mentioned @mstorsjo on this... he'd know best what needs to happen with llvm-dlltool. I think --temp-prefix doesn't really apply to how llvm's import libraries work, but it is kind of scary to me that --no-leading-underscore isn't causing an error, even though I don't see it being handled.

For reference, here's an example of args to dlltool logged by my python script:

['-d', 'C:\\msys64\\tmp\\rustc4pTSek\\kernel32.def', '-D', 'kernel32.dll', '-l', 'C:\\msys64\\tmp\\rustc4pTSek\\kernel32.lib', '--no-leading-underscore', '--temp-prefix', 'C:\\msys64\\tmp\\rustc4pTSek\\kernel32.dll']

And my python script:

$ ls /foo
dlltool.exe  dlltool-script.py

$ cat /foo/dlltool-script.py

#!c:/msys64/clang64/bin/python
import subprocess
import sys

with open("C:/msys64/tmp/dlltool.log", "a") as f:
    print(repr(sys.argv[1:]), file=f)

i = 1
args = ["C:/msys64/clang64/bin/dlltool.exe"]
while i < len(sys.argv):
    if sys.argv[i] == "--temp-prefix":
        i += 1
    #elif False and sys.argv[i] == "--no-leading-underscore":
    #    pass
    else:
        args.append(sys.argv[i])
    i += 1

sys.exit(subprocess.run(args=args).returncode)

[filnet]

I am also in the process of building 1.70.0 with a patched 1.69.0. Seems good...

I have branches ready that I'll push tomorrow.

[jeremyd2019]

Another cleanup that can be done for _bootstrapping=no:

   mkdir -p "${srcdir}/${MSYSTEM}" && cd "${srcdir}/${MSYSTEM}"

   # The ultimate hack to let the bootstrap compiler use libgcc* libs
-  if [[ $MINGW_PACKAGE_PREFIX == *-clang-i686 || $MINGW_PACKAGE_PREFIX == *-clang-x86_64 ]]; then
+  if [[ $_bootstrapping != "no" ]] && [[ $MINGW_PACKAGE_PREFIX == *-clang-i686 || $MINGW_PACKAGE_PREFIX == *-clang-x86_64 ]]; then
     export GCC_LIBS_HACK="$(cygpath -am build/missing-libs-hack)"
     mkdir -p "${GCC_LIBS_HACK}"
     cp "$(cygpath -u $(clang -print-libgcc-file-name))" "${GCC_LIBS_HACK}/libgcc.a"

[mstorsjo]

This is why I mentioned @mstorsjo on this... he'd know best what needs to happen with llvm-dlltool. I think --temp-prefix doesn't really apply to how llvm's import libraries work,

Yes, that's true, --temp-prefix would be safe to ignore in llvm-dlltool.

but it is kind of scary to me that --no-leading-underscore isn't causing an error, even though I don't see it being handled.

Hmm, if I try to call llvm-dlltool with that option, it does error out similarly to --temp-prefix?

[jeremyd2019]

$ llvm-dlltool.exe -d foo.def -D foo.dll -l foo.lib --no-leading-underscore
ignoring unknown argument: --no-leading-underscore

$ echo $?
0

$ llvm-dlltool.exe -d foo.def -D foo.dll -l foo.lib --no-leading-underscore --temp-prefix
ignoring unknown argument: --no-leading-underscore
ignoring unknown argument: --temp-prefix

$ llvm-dlltool.exe -d foo.def -D foo.dll -l foo.lib --no-leading-underscore --temp-prefix C:\\msys64\\tmp\\blahblah
OVERVIEW: llvm-dlltool
...

It seems that it just doesn't like the fact that the unknown --temp-prefix is supposed to take a parameter

[mstorsjo]

It seems that it just doesn't like the fact that the unknown --temp-prefix is supposed to take a parameter

Oh, right, that explains it!

[filnet]

I confirm that approach 3. works:

$ rustc --version
rustc 1.70.0 (90c541806 2023-05-31) (Rev1, Built by MSYS2 project)

[jeremyd2019]

3. rebuild rust 1.69 with that patch, then update to 1.70 with _bootstrapping=no (a la [WIP] rust: set _bootstrapping=no for all architectures #16207)

I have done this for CLANG64 and CLANG32.

2. add a dlltool wrapper like I made to test, that drops --temp-prefix and the next arg after that, and prepend that to the path during the rust build on CLANG32/64

Next, I intend to try this with CLANG32, because I am curious if the ignoring of the --no-leading-underscore option will be a problem (iirc, i686 defaults to a leading underscore while the other arches do not).

[filnet]

I have done this for CLANG64 and CLANG32.

I only tested CLANG64, so good to know. The stuff I pushed is for both CLANG64 and CLANG32 though.

[jeremyd2019]

2. add a dlltool wrapper like I made to test, that drops --temp-prefix and the next arg after that, and prepend that to the path during the rust build on CLANG32/64

Next, I intend to try this with CLANG32, because I am curious if the ignoring of the --no-leading-underscore option will be a problem (iirc, i686 defaults to a leading underscore while the other arches do not).

This failed with a bunch of undefined dllimport symbol linker errors, such as:

          ld.lld: error: undefined symbol: __declspec(dllimport) GetLastError
          >>> referenced by libwindows-62de4ad892e4ceab.rlib(windows-62de4ad892e4ceab.windows.b0545aed-cgu.15.rcgu.o):(windows::imp::factory_cache::get_activation_factory)
          >>> referenced by libwindows-62de4ad892e4ceab.rlib(windows-62de4ad892e4ceab.windows.b0545aed-cgu.15.rcgu.o):(<windows::imp::waiter::Waiter>::new)
          >>> referenced by libwindows-62de4ad892e4ceab.rlib(windows-62de4ad892e4ceab.windows.b0545aed-cgu.9.rcgu.o):(<windows::core::error::Error>::from_win32)

          ld.lld: error: undefined symbol: __declspec(dllimport) CreateEventW
          >>> referenced by libwindows-62de4ad892e4ceab.rlib(windows-62de4ad892e4ceab.windows.b0545aed-cgu.15.rcgu.o):(<windows::imp::waiter::Waiter>::new)

          ld.lld: error: too many errors emitted, stopping now (use --error-limit=0 to see all errors)
          clang: error: linker command failed with exit code 1 (use -v to see invocation)

So I guess the --no-leading-underscore option is significant to i686.

[filnet]

Btw, Rust 1.69.0 does not use the windows crate...

[mati865]

Sorry for radio silence, I've had company trip so the timing was a bit unfortunate.

There are 2 ways Rust can create import libraries: using LLVM API or dlltool.
Initially it was attempted to use LLVM API only but due to many bugs in ld.bfd (I think they could be fixed already but you gotta support older toolchains...) it was decided to create import libraries for mingw using dlltool.
Since CLANG* subsystems don't have to worry about BFD compatibility I'd suggest to override the check and use LLVM to generate import libs. I think it'd be as replacing this line with false: https://github.com/rust-lang/rust/blob/9eee230cd0a56bfba3ce65121798d9f9f4341cdd/compiler/rustc_codegen_llvm/src/common.rs#L370

I can look at it and do some testing tomorrow.

[mati865]

Oh, jeremyd2019 has already found it.

I think all 3 options would be viable with 1. slightly risky and 3. being possibly hard.
Ideally we would swtich CLANG* targets to -gnullvm triple but first I need to figure out why it crashes one of optimization passes in LLVM..

[filnet]

@mati865 Option 3. is in the work.

@jeremyd2019 has validated that it works for CLANG64 and CLANG32. I validated that it works for CLANG64.

There is a in progress PR for patching 1.69.0 but the MINGW32 build fails/times out (see #17433).

[jeremyd2019]

Oh, jeremyd2019 has already found it.

I think all 3 options would be viable with 1. slightly risky and 3. being possibly hard.

Based on subsequest testing I think option 3 is probably the only one that would work for CLANG32, option 1 would also require implementing the --no-leading-underscore dlltool option in llvm (more than just ignoring an unknown option).

Ideally we would swtich CLANG* targets to -gnullvm triple but first I need to figure out why it crashes one of optimization passes in LLVM..

Also, there is no i686-pc-windows-gnullvm, and I noticed in some crates when they added support for -gnullvm that they didn't seem to make the logical conclusion that there could be in the future ☹️

[filnet]

Option 3. implies that we have to keep bootstrapping=no for clang and can't revert to yes anymore (unless something is fixed upstream).

[mstorsjo]

Oh, jeremyd2019 has already found it.
I think all 3 options would be viable with 1. slightly risky and 3. being possibly hard.

Based on subsequest testing I think option 3 is probably the only one that would work for CLANG32, option 1 would also require implementing the --no-leading-underscore dlltool option in llvm (more than just ignoring an unknown option).

Implementing that option should be fairly straightforward, I think. I can give it a shot hopefully within a few days, if not sooner.

[mati865]

Also, there is no i686-pc-windows-gnullvm, and I noticed in some crates when they added support for -gnullvm that they didn't seem to make the logical conclusion that there could be in the future ☹️

That would be my fault, it was failing back when Rust was using LLVM 15 and I haven't submitted it after LLVM 16 was released.

[jeremyd2019]

As a reminder, though this PR may pass CI once 1.69.0-2 is in staging, it should not be merged until that version is uploaded to the repos, because autobuild will remove the staging version before trying to build a new version.

[filnet]

Once 1.69.0-2 is officially available, I'll rebase and push this branch again.

[jeremyd2019]

I re-ran the CI on afde6f4 once 1.69.0-2 was in staging, and all MSYSTEMs passed.

[filnet]

CI is green...

[filnet]

Implementing that option should be fairly straightforward, I think. I can give it a shot hopefully within a few days, if not sooner.

That would help us in the future.

[jeremyd2019]

D152359 D152360 D152361 and D152363. (a couple of those are basically just comment changes, but may be necessary for the others to apply cleanly).

[jeremyd2019]

Those patches were applied upstream, and are now backported in our llvm 16.0.5-1 packages

[filnet]

Those patches were applied upstream, and are now backported in our llvm 16.0.5-1 packages

I'll make sure to revert to bootstrap=yes when Rust 1.71.0 comes out,

[ognevny]

I'll make sure to revert to bootstrap=yes when Rust 1.71.0 comes out,

but why can't we do this now?

[filnet]

also it would be better to have llvm-libs as dependency like arch does

Just replace llvm by llvm-libs ?

[filnet]

I'll make sure to revert to bootstrap=yes when Rust 1.71.0 comes out,

but why can't we do this now?

What would be the point ?

[ognevny]

also it would be better to have llvm-libs as dependency like arch does

Just replace llvm by llvm-libs ?

yes, let's give it a try