Report CodegenError::Unsupported as a rustc diagnostic instead of panicking#1669
Open
soumik15630m wants to merge 1 commit into
Open
Report CodegenError::Unsupported as a rustc diagnostic instead of panicking#1669soumik15630m wants to merge 1 commit into
soumik15630m wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When
Context::compilereturnsErr(CodegenError::Unsupported(_)),src/base.rscurrently falls through to the generic catch-all arm andpanics:
This turns a recoverable, known-cause compilation error into an ICE.
The codebase already has a precedent for handling a specific
CodegenErrorvariant gracefully —CodegenError::Verifieris caughtin its own arm just above and reported via
early_dcx.early_fatalinstead of panicking. This PR adds a matching arm for
CodegenError::Unsupported.Motivation
This gap was found while working on
bytecodealliance/wasmtime#13783, which changes x64 codegen to return
CodegenError::Unsupportedinstead of panicking when a function'sstack frame is too large to encode (see
#1656). Before that PR, Cranelift
never returned this error variant from this call site, so this gap in
cg_clif's own error handling was never exercised. With rust-lang/rustc_codegen_cranelift#13783's fix
applied,
Context::compilenow reaches this code path and previouslyhit the generic
panic!, producing an ICE with a stack trace insteadof a clean compiler error.
Testing
Built this against a local patch of bytecodealliance/wasmtime#13783
(via
[patch.crates-io]) and rebuilt withy.sh build. Compiling therepro from #1656
(
let x = [0; 536870787_usize];) now produces:error: cranelift codegen error: stack frame size exceeds the 2GB limit supported by the x64 backend (see #1656)instead of panicking on the
Errat this call site.Known follow-up (not fixed here)
With this change, the error is reported cleanly, but the worker thread
that calls
early_fatalfrom inside this closure then triggers asecondary panic in
rustc_codegen_ssa("expected abort due to workerthread errors",
back/write.rs:2107). This reproduces for theexisting
Verifierarm too, using this sameearly_fatalpattern —it's a pre-existing issue in how rustc's codegen coordinator handles a
worker thread calling
early_fatal, tracked separately in #1664, andout of scope for this PR.