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

Broken nightly toolchain for windows-gnu since 2018-03-17 #49438

Closed
rkarp opened this issue Mar 28, 2018 · 25 comments
Closed

Broken nightly toolchain for windows-gnu since 2018-03-17 #49438

rkarp opened this issue Mar 28, 2018 · 25 comments
Labels
C-bug Category: This is a bug. O-windows-gnu Toolchain: GNU, Operating system: Windows P-high High priority regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.
Milestone

Comments

@rkarp
Copy link
Contributor

rkarp commented Mar 28, 2018

This is the last working version:

$ rustup toolchain install nightly-2018-03-16-gnu
info: syncing channel updates for 'nightly-2018-03-16-x86_64-pc-windows-gnu'
info: latest update on 2018-03-16, rust version 1.26.0-nightly (392645394 2018-03-15)
info: downloading component 'rustc'
info: downloading component 'rust-std'
info: downloading component 'cargo'
info: downloading component 'rust-docs'
info: downloading component 'rust-mingw'
info: installing component 'rustc'
info: installing component 'rust-std'
info: installing component 'cargo'
info: installing component 'rust-docs'
info: installing component 'rust-mingw'

  nightly-2018-03-16-x86_64-pc-windows-gnu installed - rustc 1.26.0-nightly (392645394 2018-03-15)

For nightly-2018-03-17 rustc doesn't work at all:

$ rustup toolchain install nightly-2018-03-17-gnu
info: syncing channel updates for 'nightly-2018-03-17-x86_64-pc-windows-gnu'
info: latest update on 2018-03-17, rust version 1.26.0-nightly (55c984ee5 2018-03-16)
info: downloading component 'rustc'
info: downloading component 'rust-std'
info: downloading component 'cargo'
info: downloading component 'rust-docs'
info: downloading component 'rust-mingw'
info: installing component 'rustc'
info: installing component 'rust-std'
info: installing component 'cargo'
info: installing component 'rust-docs'
info: installing component 'rust-mingw'

  nightly-2018-03-17-x86_64-pc-windows-gnu installed - (error reading rustc version)

When trying to run rustc from that nightly, there's an error about not being able to find an entry point for CreateMutexA in api-ms-core-synch-l1-2-0.dll. This is on 2 different computers running Windows 7 Pro x64.

@leonardo-m
Copy link

For me the last working Nightly version is:

rustc 1.26.0-nightly (9c9424de5 2018-03-27)
binary: rustc
commit-hash: 9c9424de51da41fd3d1077ac7810276f8dc746fa
commit-date: 2018-03-27
host: x86_64-pc-windows-gnu
release: 1.26.0-nightly
LLVM version: 6.0

@rkarp
Copy link
Contributor Author

rkarp commented Mar 28, 2018

The problem seems to be that rustc-errors.dll has gained a dependency on api-ms-core-synch-l1-2-0.dll. On nightly-2018-03-16 CreateMutexA is instead imported from kernel32.dll.

@kennytm kennytm added regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. O-windows-gnu Toolchain: GNU, Operating system: Windows C-bug Category: This is a bug. labels Mar 28, 2018
@retep998
Copy link
Member

retep998 commented Mar 28, 2018

Ah the joys of relying on import libraries from MinGW and hoping everything resolves where we want it...

This was likely caused by something depending on synchronization.lib/libsynchronization.a through winapi recently. Fixing it would require either reverting that dependency change or doing some complicated shenanigans involving import libraries and winapi.

@ghost
Copy link

ghost commented Apr 2, 2018

This error seems stopped with version 1.26.0-nightly (517f24025 2018-03-31) on my Windows 7 Pro x64,

Does anyone know what commit fixed it?

I'm not able to locate it at Commits merged on Mar 31, 2018 at Master.

rustup install nightly-2018-03-16-gnu
info: syncing channel updates for 'nightly-2018-03-16-x86_64-pc-windows-gnu'
info: latest update on 2018-03-16, rust version 1.26.0-nightly (392645394 2018-03-15)
[...]

** Ok **
rustup install nightly-2018-03-17-gnu
info: syncing channel updates for 'nightly-2018-03-17-x86_64-pc-windows-gnu'
info: latest update on 2018-03-17, rust version 1.26.0-nightly (55c984ee5 2018-03-16)
[...]

** Error **  rustc.exe : 
 The procedure entry point CreateMutexA could not be located 
 in the dynamic library api-ms-win-core-synch-l1-2-0.dll
[...]
rustup install nightly-2018-03-31-gnu
info: syncing channel updates for 'nightly-2018-03-31-x86_64-pc-windows-gnu'
info: latest update on 2018-03-31, rust version 1.26.0-nightly (80785a547 2018-03-30)
[...]

** Error **  rustc.exe : 
 The procedure entry point CreateMutexA could not be located 
 in the dynamic library api-ms-win-core-synch-l1-2-0.dll
rustup install nightly-2018-04-01-gnu
info: syncing channel updates for 'nightly-2018-04-01-x86_64-pc-windows-gnu'
info: latest update on 2018-04-01, rust version 1.26.0-nightly (517f24025 2018-03-31)
[...]

** Ok **
rustup install nightly
info: syncing channel updates for 'nightly-x86_64-pc-windows-gnu'
info: latest update on 2018-04-02, rust version 1.26.0-nightly (06fa27d7c 2018-04-01)
[...]

** Ok **

@rkarp
Copy link
Contributor Author

rkarp commented Apr 8, 2018

It's not yet fixed for me on version 1.27.0-nightly (056f589fb 2018-04-07).

@ghost
Copy link

ghost commented Apr 9, 2018

Idem

rustup install nightly-2018-04-04-gnu
info: syncing channel updates for 'nightly-2018-04-04-x86_64-pc-windows-gnu'
info: latest update on 2018-04-04, rust version 1.27.0-nightly (637ac17c5 2018-04-03)

** Error **  rustc.exe : 
 The procedure entry point CreateMutexA could not be located 
 in the dynamic library api-ms-win-core-synch-l1-2-0.dll
 [...]
rustup install nightly-2018-04-08-gnu
info: syncing channel updates for 'nightly-2018-04-08-x86_64-pc-windows-gnu'
info: latest update on 2018-04-08, rust version 1.27.0-nightly (056f589fb 2018-04-07)

** Error **  rustc.exe : 
 The procedure entry point CreateMutexA could not be located 
 in the dynamic library api-ms-win-core-synch-l1-2-0.dll

@pnkfelix
Copy link
Member

Is one of the participants here interested in attempting to bisect down to a particular commit (rather than just a date) via https://github.com/rust-lang-nursery/cargo-bisect-rustc ?

@nikomatsakis nikomatsakis added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Apr 12, 2018
@retep998
Copy link
Member

retep998 commented Apr 12, 2018

After further investigation I have found that MinGW's libsynchronization.a is actually quite different from MSVC's synchronization.lib.

synchronization.lib has just these symbols:

WaitOnAddress
WakeByAddressAll
WakeByAddressSingle

libsynchronization.a meanwhile has all of these symbols:

AcquireSRWLockExclusive
AcquireSRWLockShared
CreateEventA
CreateEventExA
CreateEventExW
CreateEventW
CreateMutexA
CreateMutexExA
CreateMutexExW
CreateMutexW
CreateSemaphoreExW
DeleteCriticalSection
EnterCriticalSection
InitOnceBeginInitialize
InitOnceComplete
InitOnceExecuteOnce
InitOnceInitialize
InitializeConditionVariable
InitializeCriticalSection
InitializeCriticalSectionAndSpinCount
InitializeCriticalSectionEx
InitializeSRWLock
LeaveCriticalSection
OpenEventA
OpenEventW
OpenMutexW
OpenSemaphoreW
ReleaseMutex
ReleaseSRWLockExclusive
ReleaseSRWLockShared
ReleaseSemaphore
ResetEvent
SetCriticalSectionSpinCount
SetEvent
Sleep
SleepConditionVariableCS
SleepConditionVariableSRW
SleepEx
TryAcquireSRWLockExclusive
TryAcquireSRWLockShared
TryEnterCriticalSection
WaitForMultipleObjectsEx
WaitForSingleObject
WaitForSingleObjectEx
WaitOnAddress
WakeAllConditionVariable
WakeByAddressAll
WakeByAddressSingle
WakeConditionVariable

These exports lead to DLLs that only exist on newer versions of Windows, and are overriding the exports from libkernel32.a. Therefore this is entirely MinGW's fault for providing such shoddy import libraries that don't match their msvc equivalents. Also do note that this issue does not manifest when using winapi normally, only when building Rust because Rust does not use winapi's bundled import libraries. So as far as I can see there are a few ways we can deal with this.

  1. Get rid of or modify whatever dependency is enabling the synchapi feature of winapi which causes libsynchronization.a to be linked in.
  2. Get this fixed upstream in MinGW.
  3. Replace libsynchronization.a from MinGW with our own version (borrow the one from winapi perhaps).
  4. Somehow figure out how to use winapi's bundled import libraries. Would be a lot easier if Rust wasn't a massive pile of dylibs to support plugins.

As a workaround for users in the meantime, the -msvc toolchain still works fine on Windows 7 and you can cross to -gnu if you really need to work with MinGW.

@bricky149
Copy link

This issue now affects the beta channel after 1.26 beta 9 got the dodgy commit. MSVC works fine on Windows 7 x64.

@retep998 retep998 added regression-from-stable-to-beta Performance or correctness regression from stable to beta. and removed regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. labels Apr 22, 2018
@pietroalbini pietroalbini added this to the 1.26 milestone Apr 26, 2018
@pietroalbini pietroalbini added this to Triaged in 1.26 regressions Apr 26, 2018
@nikomatsakis
Copy link
Contributor

@retep007 which of those options do you think would be the best choice? Maybe we should file an issue with MinGW just to get their opinion? I guess my preference, in order, would be to (a) file an issue, then (b) remove the dependency if we can, else (c) use an alternate version of the library.

@nikomatsakis
Copy link
Contributor

cc @rust-lang/infra -- using an alternate version of the library seems more to fall in your domain =) at least, I don't really know what's involved

@nikomatsakis nikomatsakis added T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. P-high High priority and removed I-nominated labels Apr 26, 2018
@pietroalbini pietroalbini removed the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Apr 26, 2018
@alexcrichton
Copy link
Member

@rkarp and @leonardo-m, is this still happening for you on nightly? Looking at the dependencies on today's nightly it looks like the DLL dependency has disappeared so I'd just want to confirm that works so we can figure out what to backport.

@alexcrichton
Copy link
Member

I also can't see the dependency in beta.12 (today's beta release). Was this perhaps accidentally fixed? Could y'all who have been reproducing this confirm whether ornot it's been fixed?

@alexcrichton
Copy link
Member

The difference between beta.9 and beta.12 doesn't really show much why this would be fixed one way or the other...

@retep998
Copy link
Member

retep998 commented Apr 26, 2018

https://github.com/retep998/winapi-rs/blob/c54b3e8d887644316654f8fb4ab8dd2aabad70bd/build.rs#L388

The order in which winapi emits libraries is random so it occurs in some versions but not others, which means reproducing is like rolling a die. Whether libsynchronization.a or libkernel32.a is first can determine which dll a symbol will end up resolving to.

@alexcrichton
Copy link
Member

@retep998 can the nondeterminism be fixed? Sounds like it's at least problematic here!

alexcrichton added a commit to alexcrichton/rust that referenced this issue Apr 26, 2018
Unfortunately the `parking_lot` crate enables the `synchapi` feature of the
`winapi` crate which activates a dependency on `libsynchronization.a`. The MinGW
version of `libsynchronization.a` pulls in a dependency
`api-ms-core-synch-l1-2-0.dll` which causes rustc to not work on Windows 7
(tracked in rust-lang#49438)

The `parking_lot` crate is not currently used in the compiler unless parallel
queries are enabled. This feature is not enabled by default and not used at all
in the beta/stable compilers. As a result the dependency in this commit was
removed and the CI build which checks parallel queries was disabled.

This isn't a great long-term solution but should hopefully be enough of a patch
for beta to buy us some more time to figure this out.
@alexcrichton
Copy link
Member

I've excised the dependency which enables synchapi in #50254 which should get rustc working again at least. It's not a complete solution unfortunately.

@Zoxc do you have thoughts on this? The parking_lot dependency is what's enabling the synchapi feature in winapi which is buggy here. Is it possible to not use parking_lot? Is it possible to have a different Windows implementation?

@alexcrichton
Copy link
Member

And to be clear #50254 is intended to be a beta-only fix, a fix is still needed for master.

bors added a commit that referenced this issue Apr 26, 2018
[beta] Remove dependency on `parking_lot`

Unfortunately the `parking_lot` crate enables the `synchapi` feature of the
`winapi` crate which activates a dependency on `libsynchronization.a`. The MinGW
version of `libsynchronization.a` pulls in a dependency
`api-ms-core-synch-l1-2-0.dll` which causes rustc to not work on Windows 7
(tracked in #49438)

The `parking_lot` crate is not currently used in the compiler unless parallel
queries are enabled. This feature is not enabled by default and not used at all
in the beta/stable compilers. As a result the dependency in this commit was
removed and the CI build which checks parallel queries was disabled.

This isn't a great long-term solution but should hopefully be enough of a patch
for beta to buy us some more time to figure this out.
@Zoxc
Copy link
Contributor

Zoxc commented Apr 26, 2018

We could just remove the dependency on libsynchronization.a, since that's only supposed to have Win 8+ symbols, which rustc can't use anyway.

@alexcrichton
Copy link
Member

@Zoxc would you be willing to look into removing parking_lot's dependency on synchapi or otherwise fixing winapi on MinGW to not require libsynchronization.a in this case?

@retep998
Copy link
Member

retep998 commented Apr 26, 2018

The nondeterministic ordering of libraries is never an issue when winapi is used normally, because normally none of the libraries import the same symbol from two different dlls. It's only because of the unique way that rustbuild builds winapi (by relying on an external MinGW toolchain instead of bundled import libraries) which causes this issue to be possible. Besides, it's not like the random order causes the issue, it merely makes the issue nondeterministic and therefore slightly harder to debug.

There are functions in the synchapi module that need synchronization.lib so I can't just make winapi stop depending on it. However, the only thing parking_lot even uses from synchapi is Sleep, so getting rid of the synchapi dependence there shouldn't be too hard.

@alexcrichton
Copy link
Member

@retep998 we like to keep the builds of rustc as deterministic as possible. Ordering not only affects the final artifact but interediate rlibs. Would you accept a PR to make the ordering deterministic?

@alexcrichton
Copy link
Member

@retep998 this also seems like it's buggy on behalf of winapi? The documentation indicates that Sleep, despite being in the Synchapi.h header, resides in kernel32.dll. Activating the synchapi header doesn't seem like it should unconditionally activate linking to the synchronization library?

alexcrichton added a commit to alexcrichton/parking_lot that referenced this issue Apr 27, 2018
Pulling in `parking_lot` to the Rust compiler unfortunately had adverse side
effects on the binary dependencies of rustc -- rust-lang/rust#49438. It turns
out this is due to a number of bugs happening all at once, and currently we're
looking for ways to fix the beta/master branches of rustc to fix this for now
(not necessarily in the best way).

This commit removes the activation of the `synchapi` feature of `winapi` which
prevents `libsynchronization.a` from being linked in (and prevents rustc from
accidentally picking up a dependency on a missing DLL). The `synchapi` feature
was only activated for access to the `Sleep` function, but that function
actually resides in `kernel32.dll`. As a result this commit inlines the
definition of `Sleep` into this crate to avoid needing to depend on an upstream
definition.
@retep998
Copy link
Member

retep998 commented Apr 27, 2018

@alexcrichton If you enable the synchapi module, then it will enable linking to any library necessary for all functions in that module (after all, I don't know what the user will be using from that module). But as long as you don't actually use those functions then no references to those functions will be emitted and there will be no unintentional runtime dependencies, which is why I chose to unconditionally enable linking to libraries and not offer finer grained control over which libraries actually get linked to (it would require a major version bump to require that users specify which libraries to link to, and would make targeting store apps even harder). When used normally with the Windows SDK or the bundled import libraries this ends up working correctly with no issues.

In this specific case however, Rust is choosing to use import libraries from MinGW instead of using winapi's bundled import libraries and the libsynchronization.a import library from MinGW has imports that don't exist in synchronization.lib which may override the similar imports from libkernel32.a.

I will make the ordering deterministic though, but that won't solve the underlying problem of MinGW's libsynchronization.a being wrong.

alexcrichton added a commit to alexcrichton/parking_lot that referenced this issue Apr 27, 2018
Pulling in `parking_lot` to the Rust compiler unfortunately had adverse side
effects on the binary dependencies of rustc -- rust-lang/rust#49438. It turns
out this is due to a number of bugs happening all at once, and currently we're
looking for ways to fix the beta/master branches of rustc to fix this for now
(not necessarily in the best way).

This commit removes the activation of the `synchapi` feature of `winapi` which
prevents `libsynchronization.a` from being linked in (and prevents rustc from
accidentally picking up a dependency on a missing DLL). The `synchapi` feature
was only activated for access to the `Sleep` function, but that function
actually resides in `kernel32.dll`. As a result this commit inlines the
definition of `Sleep` into this crate to avoid needing to depend on an upstream
definition.
@alexcrichton
Copy link
Member

Switching the regression tag to nightly as this should be fixed on beta now.

@retep998 there's a whole mess of bugs here, and I'm looking for any route to fix it. Sounds like it's not the easiest to fix in winapi, so we won't rely on fixing it there then.

@alexcrichton alexcrichton added regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. and removed regression-from-stable-to-beta Performance or correctness regression from stable to beta. labels Apr 27, 2018
@alexcrichton alexcrichton modified the milestones: 1.26, 1.27 Apr 27, 2018
@alexcrichton alexcrichton moved this from Triaged to Fixed in 1.26 regressions Apr 27, 2018
alexcrichton added a commit to alexcrichton/rust that referenced this issue Apr 27, 2018
This commit updates `parking_lot` to pull in Amanieu/parking_lot#70 and...

Closes rust-lang#49438
kennytm added a commit to kennytm/rust that referenced this issue Apr 27, 2018
…Mark-Simulacrum

Update `parking_lot` dependencies

This commit updates `parking_lot` to pull in Amanieu/parking_lot#70 and...

Closes rust-lang#49438
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. O-windows-gnu Toolchain: GNU, Operating system: Windows P-high High priority regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.
Projects
No open projects
Development

No branches or pull requests

10 participants