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

Update Rust to 1.48 #6932

Merged
merged 4 commits into from
Nov 23, 2020
Merged

Update Rust to 1.48 #6932

merged 4 commits into from
Nov 23, 2020

Conversation

mati865
Copy link
Collaborator

@mati865 mati865 commented Sep 4, 2020

Closes #6758

@mati865
Copy link
Collaborator Author

mati865 commented Sep 4, 2020

I can reproduce 64-bit (exit code: 0xc0000005, STATUS_ACCESS_VIOLATION) locally with latest packages, trying to fix it.

Ouch:

(lldb) bt
* thread #1, stop reason = Exception 0xc0000005 encountered at address 0x2e74f7720: User-mode data execution prevention
(DEP) violation at location 0x2e74f7720
  * frame #0: 0x00000002e74f7720
    frame #1: 0x0000000277a9fe0b rustc_driver-dd6622bf73e2eb5a.dll`_$LT$rustc_driver..DEFAULT_HOOK$u20$as$u20$lazy_stati
c..LazyStatic$GT$::initialize::hd3a4f38e7904f614 + 231995
    frame #2: 0x0000000277ac17e0 rustc_driver-dd6622bf73e2eb5a.dll`rustc_driver::pretty::print_after_hir_lowering::hc6f4
c36ae1e9b344 + 4832
    frame #3: 0x0000000277a6614c rustc_driver-dd6622bf73e2eb5a.dll`rustc_driver::handle_options::he701fa99488a750e + 554
8
    frame #4: 0x0000000277a6720d rustc_driver-dd6622bf73e2eb5a.dll`rustc_driver::main::hd7e434be262eae6a + 45
    frame #5: 0x000000014000158e rustc.exe`rustc_binary::main::hb57cdd5e7952eb06 + 14
    frame #6: 0x0000000140001556 rustc.exe`std::rt::lang_start::_$u7b$$u7b$closure$u7d$$u7d$::he1d78f95d5162d84 + 6
    frame #7: 0x00000001e74f7d6d std-fc47a2c9bc2884a5.dll`std::rt::lang_start_internal::hbe42a3bca6615642 + 285
    frame #8: 0x00000001400015c8 rustc.exe`main + 56
    frame #9: 0x00000001400013c1 rustc.exe`__tmainCRTStartup at crtexe.c:335:15
    frame #10: 0x00000001400014f6 rustc.exe`mainCRTStartup at crtexe.c:219:9
    frame #11: 0x00007ffadeb86fd4 kernel32.dll`BaseThreadInitThunk + 20
    frame #12: 0x00007ffadfefcec1 ntdll.dll`RtlUserThreadStart + 33

@mati865 mati865 marked this pull request as draft September 4, 2020 13:35
@abcfy2
Copy link

abcfy2 commented Sep 6, 2020

Hope this build failing would be fixed.

@mati865
Copy link
Collaborator Author

mati865 commented Sep 7, 2020

I believe I've got the fix (still testing it to be sure).
Before rust-lang/rust#72049 Rust was relying on the linker "to make things work". That means linker to guess dllimport/dllexport and in case it fails the linker was using runtime pseudo relocs as backup.
I haven't full investigated it but I think the recent changes to Binutils (namely increasing base address over 4G for 64-bit binaries) made runtime pseudo relocs that have 32-bit offsets to overflow and hence trigger the DEP.
That would also explain why i686 build had no issues.

@lazka I believe that would explain some of those cases:

So far all software that also supports building with MSVC works fine (glib,gtk3,pygobject for example) so I'm wondering if this isn't just exposes some latent bugs for things only built with mingw so far. Agreed on the example search..

@lazka
Copy link
Member

lazka commented Sep 7, 2020

So we should pass -Wl,--no-dynamicbase,--image-base=0x400000 to revert this for rust for now?

@mati865
Copy link
Collaborator Author

mati865 commented Sep 7, 2020

@lazka that would be complicated since executables and DLLs use different base addresses.
I'm backporting rust-lang/rust#72049 and it already passed with export LDFLAGS="-pipe". I'm testing default LDFLAGS now but since rust-lang/rust#75406 works for me locally I believe this will also succeed.

@jeremyd2019
Copy link
Member

@lazka that would be complicated since executables and DLLs use different base addresses.

Yes! I was considering sending another patch for binutils to allow selecting whether the default bases should be "high" or "low" via command line option. But I am not sold on that being a good idea. It would make hacking around buggy programs a lot easier though. Thoughts?

@mati865
Copy link
Collaborator Author

mati865 commented Sep 7, 2020

It would make hacking around buggy programs a lot easier though. Thoughts?

It would definitely help with the transition.

jeremyd2019 added a commit to jeremyd2019/MINGW-packages that referenced this pull request Sep 7, 2020
msys2#6932 and msys2#6907 show packages
have issues with the new default base addresses, so add an option to
change back to the old defaults during the transition.
@jeremyd2019
Copy link
Member

It would definitely help with the transition.

I made a patch and am testing it now (simple tests worked as intended, going to attempt building GCC with it). I don't know if a 'transitional' option warrants upstreaming though.

@lazka
Copy link
Member

lazka commented Sep 12, 2020

New binutils with --default-image-base-low option is now in the repo.

@mati865
Copy link
Collaborator Author

mati865 commented Sep 12, 2020

I think using rust-lang/rust#72049 instead of --default-image-base-low is better but it's up to you.

@lazka
Copy link
Member

lazka commented Sep 12, 2020

a real upstream fix/backport is obviously better

@mati865
Copy link
Collaborator Author

mati865 commented Sep 28, 2020

I have no idea why CI fails on x86_64 build, lets try again.

@mati865 mati865 force-pushed the update-rust branch 2 times, most recently from b5f0661 to 2b0d14f Compare October 14, 2020 16:36
@mati865 mati865 marked this pull request as draft October 14, 2020 16:36
@mati865 mati865 changed the title Update Rust to 1.46 Update Rust to 1.47 Oct 14, 2020
@lazka
Copy link
Member

lazka commented Oct 14, 2020

Not sure if relevant, but it finds the wrong Python: -- Found Python3: C:/hostedtoolcache/windows/Python/3.8.6/x86/python.exe (found version "3.8.6") found components: Interpreter

@mati865
Copy link
Collaborator Author

mati865 commented Oct 20, 2020

Blocked on #7116

@mati865
Copy link
Collaborator Author

mati865 commented Nov 5, 2020

CI now appears to be failing due to path length but it's kinda hard to tell.

@jeremyd2019
Copy link
Member

jeremyd2019 commented Nov 5, 2020

Or command line length, by the look of that ar command... I copy/pasted it and saw 36618 characters there!

@mati865
Copy link
Collaborator Author

mati865 commented Nov 5, 2020

I think it should have used response file but yeah sounds plausible.

@filnet
Copy link
Contributor

filnet commented Nov 17, 2020

Hi,

What's holding up the rust upgrade ? A hard to solve technical problem or just lack of time from the maintainers ?
Can I help in any way ?

@mati865
Copy link
Collaborator Author

mati865 commented Nov 17, 2020

Local build succeeds every time for me.
I haven't got to dive deeply into the issue yet and since the next Rust version will be released this week I just decided to wait.

@Zapeth
Copy link
Contributor

Zapeth commented Nov 20, 2020

Looking at the build log it seems the failed ar command originates from compiling libgit2-sys for cargo.

I haven't heard yet of command line length being an issue when compiling any kind of software, but path length could certainly be an issue (some of those paths get pretty close to the Windows MAX_PATH length, and i686 config might just fall under that limit), though I'd be curious why this wasn't an issue with earlier package releases.

In any case, 1.48 was released yesterday, so it might be worth trying a new ci build with that.

@mati865 mati865 changed the title Update Rust to 1.47 Update Rust to 1.48 Nov 21, 2020
@mati865
Copy link
Collaborator Author

mati865 commented Nov 21, 2020

I haven't heard yet of command line length being an issue when compiling any kind of software, but path length could certainly be an issue

IIRC I have inspected the longest paths and they were under the limit. Command line limit is definitely a thing on Windows but it depends on how you run the executable so I won't even try to guess what is the limit in this case.

@mati865
Copy link
Collaborator Author

mati865 commented Nov 22, 2020

I have no idea why it fails with rustc-1.48.0-src.tar.gz ... FAILED (unknown public key 85AB96E6FA1BE5FE) on CI.
The build passed for me locally before pushing.

@lazka
Copy link
Member

lazka commented Nov 22, 2020

It means the release was signed with an ancient gnupg which doesn't include the full key ID in the signature, so auto key retrieval doesn't work. You can remove the signature to avoid it.

@Zapeth
Copy link
Contributor

Zapeth commented Nov 22, 2020

Looks like the same issue. I did some searching online and found this -> rust-lang/cc-rs#496

To add some more context: The build.rs of libgit2-sys issues a cc::Build compile call which fails with the aforementioned error message, hopefully the problem will be fixed with that PR.

@lazka
Copy link
Member

lazka commented Nov 23, 2020

nice :) should I merge?

@mati865
Copy link
Collaborator Author

mati865 commented Nov 23, 2020

I have made basic tests with x86_64 before pushing and it worked so I guess it's good to go.

@lazka
Copy link
Member

lazka commented Nov 23, 2020

thanks, I also built librsvg with it and all looks good.

@lazka lazka merged commit 6227cc8 into msys2:master Nov 23, 2020
@mati865 mati865 deleted the update-rust branch November 23, 2020 08:56
@Biswa96
Copy link
Member

Biswa96 commented Nov 25, 2020

Just wait and watch.

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

Successfully merging this pull request may close these issues.

Please upgrade rust to 1.45.2
7 participants