Repository navigation
Cranelift: Doesn't compile anymore on latest Windows Nightly #8919
Copy link
Copy link
Closed
Labels
bugIncorrect behavior in the current implementation that needs fixingIncorrect behavior in the current implementation that needs fixingcraneliftIssues related to the Cranelift code generatorIssues related to the Cranelift code generator
Description
error: failed to run custom build command for `cranelift-codegen v0.109.0`
Caused by:
process didn't exit successfully: `D:\a\livesplit-core\livesplit-core\target\debug\build\cranelift-codegen-16ee0d76fd74ed56\build-script-build` (exit code: 0xc00000fd, STATUS_STACK_OVERFLOW)
--- stdout
cargo:rustc-cfg=feature="x86"
cargo:rerun-if-changed=build.rs
cargo:rerun-if-changed=src\prelude.isle
cargo:rerun-if-changed=src\prelude_opt.isle
cargo:rerun-if-changed=src\opts\arithmetic.isle
cargo:rerun-if-changed=src\opts\bitops.isle
cargo:rerun-if-changed=src\opts\cprop.isle
cargo:rerun-if-changed=src\opts\extends.isle
cargo:rerun-if-changed=src\opts\icmp.isle
cargo:rerun-if-changed=src\opts\remat.isle
cargo:rerun-if-changed=src\opts\selects.isle
cargo:rerun-if-changed=src\opts\shifts.isle
cargo:rerun-if-changed=src\opts\spaceship.isle
cargo:rerun-if-changed=src\opts\spectre.isle
cargo:rerun-if-changed=src\opts\vector.isle
--- stderr
Writing generated file: D:\a\livesplit-core\livesplit-core\target\x86_64-pc-windows-msvc\debug\build\cranelift-codegen-e0c1105143d63b20\out\settings.rs
Writing generated file: D:\a\livesplit-core\livesplit-core\target\x86_64-pc-windows-msvc\debug\build\cranelift-codegen-e0c1105143d63b20\out\types.rs
Writing generated file: D:\a\livesplit-core\livesplit-core\target\x86_64-pc-windows-msvc\debug\build\cranelift-codegen-e0c1105143d63b20\out\opcodes.rs
Writing generated file: D:\a\livesplit-core\livesplit-core\target\x86_64-pc-windows-msvc\debug\build\cranelift-codegen-e0c1105143d63b20\out\inst_builder.rs
Writing generated file: D:\a\livesplit-core\livesplit-core\target\x86_64-pc-windows-msvc\debug\build\cranelift-codegen-e0c1105143d63b20\out\settings-x86.rs
Writing generated file: D:\a\livesplit-core\livesplit-core\target\x86_64-pc-windows-msvc\debug\build\cranelift-codegen-e0c1105143d63b20\out\clif_opt.isle
Writing generated file: D:\a\livesplit-core\livesplit-core\target\x86_64-pc-windows-msvc\debug\build\cranelift-codegen-e0c1105143d63b20\out\clif_lower.isle
Rebuilding D:\a\livesplit-core\livesplit-core\target\x86_64-pc-windows-msvc\debug\build\cranelift-codegen-e0c1105143d63b20\out\isle_opt.rs
thread 'main' has overflowed its stack
Versions and Environment
Cranelift version or commit: cranelift-codegen v0.104.3 as well as cranelift-codegen v0.109.0
Operating system: Windows
Architecture: x86_64
Rust: x86_64-pc-windows-msvc
latest update on 2024-07-08, rust version 1.81.0-nightly (20ae37c18 2024-07-07)
Actually seems to have been happening since at least:
latest update on 2024-06-30, rust version 1.81.0-nightly (ba1d7f4a0 2024-06-29)
Last time I saw it working was:
latest update on 2024-06-24, rust version 1.81.0-nightly (bcf94dec5 2024-06-23)
Extra Info
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
bugIncorrect behavior in the current implementation that needs fixingIncorrect behavior in the current implementation that needs fixingcraneliftIssues related to the Cranelift code generatorIssues related to the Cranelift code generator
Activity
CryZe commentedon Jul 8, 2024
This might just be a temporary / general issue with the latest Windows nightly compiler. This does not reproduce on any other target, not even the GNU variant.
alexcrichton commentedon Jul 8, 2024
Would you be able to run a bisection to determine what caused this? The
cargo bisect-rustcsubcommand I believe is the go-to tool for this.CryZe commentedon Jul 8, 2024
It's pretty late here, I'll look into it tomorrow.
CryZe commentedon Jul 9, 2024
I was not able to reproduce it outside of the Github Actions, so I'll bisect it there manually now.
CryZe commentedon Jul 9, 2024
The regression happened between
nightly-2024-06-28(working) andnightly-2024-06-29(broken), which puts the regression in the following range:rust-lang/rust@9c3bc80...e9e6e2e
CryZe commentedon Jul 9, 2024
The only thing that stands out to me is some changes to LLD and the patchable function entries.
CryZe commentedon Jul 9, 2024
I was able to reproduce it locally and run cargo bisect-rustc on it. The regression is in rust-lang/rust@99f77a2 which is rust-lang/rust#127076
alexcrichton commentedon Jul 9, 2024
Thanks for investigating! Nothing obvious there and my hunch would be rust-lang/rust#126970 but even then it's probably a case where we had a really deep stack before and just happened to overflow now.
Since you're able to reproduce locally are you able to capture a backtrace to see where the stack overflow is coming from?
CryZe commentedon Jul 9, 2024
This is how you can reproduce it (with a sufficiently up to date Windows MSVC nightly toolchain):
what's interesting is that this does NOT reproduce it (which is slightly different for cargo in how it's able to reuse dependencies of the host vs. the target):
CryZe commentedon Jul 9, 2024
Mmmh, that does not look super healthy regardless of whether it's a slight regression.
But I believe you are right. The PR you linked does inline the
clone_intofunction, which possibly got inlined into any of these functions on the stack here, increasing the stack usage a little, which was just enough to push it over the edge.alexcrichton commentedon Jul 9, 2024
For ISLE-related folks, does anyone know if this is a bug to have this much recursion between here and here? If so is it possible to somewhat easily make this a loop rather than recursive? If not is there a way to reduce the size of functions to avoid having such a large stack frame?
fitzgen commentedon Jul 9, 2024
cc @jameysharp
jameysharp commentedon Jul 9, 2024
This recursion certainly could be rewritten to use an explicit stack. Its depth is proportional to the number of constraints in the left-hand side of a rule, so I didn't expect it to be a problem.
It would be nice to figure out which rule it was generating code for at the time. (It must be a mid-end optimization rule if
Loopis involved.) But I suppose we don't really need to know; presumably we consider all the rules to be of reasonable size so we need ISLE to just handle them.It's also worth noting that since build scripts are compiled in debug mode, rustc is building this without optimization.
ISLE: Make block codegen non-recursive
jameysharp commentedon Jul 10, 2024
@CryZe, could you test whether #8935 fixes this issue for you?
There are several other parts of ISLE which also recurse over the structure of a user-provided rule, so I'm concerned that we may have this problem again in the future. But
emit_blockis one of the last things to run so I believe all the other recursive functions are currently succeeding. As a result I don't want to change them right now.CryZe commentedon Jul 10, 2024
The PR does indeed seem to fix the issue. Thanks for working on it :)
ISLE: Make block codegen non-recursive (#8935)
jameysharp commentedon Jul 11, 2024
I forgot to mark that PR as fixing this, so now that it's merged I'll close this issue.
ISLE: Make block codegen non-recursive (bytecodealliance#8935)
ISLE: Make block codegen non-recursive (bytecodealliance#8935)
ISLE: Make block codegen non-recursive (#8935) (#8947)
sunshowers commentedon Aug 4, 2024
Quite likely related: rust-lang/rust#128422
That's fundamentally a
rustcissue, but would this fix work around that?cfallin commentedon Aug 5, 2024
@sunshowers I don't think the fix above would address that issue unfortunately -- Jamey's change removes recursion (in favor of iteration with an explicit stack) in the ISLE DSL-to-Rust compiler, but the generated Rust code that causes rust-lang/rust#128422 will be exactly the same. (They're certainly related in the sense that the matcher decision tree gets quite deep!)
sunshowers commentedon Aug 5, 2024
That makes sense! Thanks @cfallin.