Skip to content

Reject unsupported extern "{abi}"s consistently in all positions #142134

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

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 23 additions & 4 deletions compiler/rustc_ast_lowering/src/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use rustc_abi::ExternAbi;
use rustc_ast::ptr::P;
use rustc_ast::visit::AssocCtxt;
use rustc_ast::*;
use rustc_errors::ErrorGuaranteed;
use rustc_errors::{E0570, ErrorGuaranteed, struct_span_code_err};
use rustc_hir::def::{DefKind, PerNS, Res};
use rustc_hir::def_id::{CRATE_DEF_ID, LocalDefId};
use rustc_hir::{self as hir, HirId, LifetimeSource, PredicateOrigin};
Expand Down Expand Up @@ -1617,9 +1617,28 @@ impl<'hir> LoweringContext<'_, 'hir> {
self.error_on_invalid_abi(abi_str);
ExternAbi::Rust
});
let sess = self.tcx.sess;
let features = self.tcx.features();
gate_unstable_abi(sess, features, span, extern_abi);
let tcx = self.tcx;

// we can't do codegen for unsupported ABIs, so error now so we won't get farther
if !tcx.sess.target.is_abi_supported(extern_abi) {
let mut err = struct_span_code_err!(
tcx.dcx(),
span,
E0570,
"`{extern_abi}` is not a supported ABI for the current target",
);

if let ExternAbi::Stdcall { unwind } = extern_abi {
let c_abi = ExternAbi::C { unwind };
let system_abi = ExternAbi::System { unwind };
err.help(format!("if you need `extern {extern_abi}` on win32 and `extern {c_abi}` everywhere else, \
use `extern {system_abi}`"
));
}
err.emit();
}
// stability gate even things that are already errored on
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why would we emit a stability error on top of the hard error? Usually we avoid showing more than one error for the same thing if possible.

gate_unstable_abi(tcx.sess, tcx.features(), span, extern_abi);
extern_abi
}

Expand Down
60 changes: 19 additions & 41 deletions compiler/rustc_hir_analysis/src/check/check.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::cell::LazyCell;
use std::ops::ControlFlow;

use rustc_abi::FieldIdx;
use rustc_abi::{ExternAbi, FieldIdx};
use rustc_attr_data_structures::ReprAttr::ReprPacked;
use rustc_data_structures::unord::{UnordMap, UnordSet};
use rustc_errors::codes::*;
Expand All @@ -12,7 +12,6 @@ use rustc_infer::infer::{RegionVariableOrigin, TyCtxtInferExt};
use rustc_infer::traits::{Obligation, ObligationCauseCode};
use rustc_lint_defs::builtin::{
REPR_TRANSPARENT_EXTERNAL_PRIVATE_FIELDS, UNSUPPORTED_CALLING_CONVENTIONS,
UNSUPPORTED_FN_PTR_CALLING_CONVENTIONS,
};
use rustc_middle::hir::nested_filter;
use rustc_middle::middle::resolve_bound_vars::ResolvedArg;
Expand All @@ -37,54 +36,32 @@ use {rustc_attr_data_structures as attrs, rustc_hir as hir};
use super::compare_impl_item::check_type_bounds;
use super::*;

fn add_abi_diag_help<T: EmissionGuarantee>(abi: ExternAbi, diag: &mut Diag<'_, T>) {
if let ExternAbi::Cdecl { unwind } = abi {
let c_abi = ExternAbi::C { unwind };
diag.help(format!("use `extern {c_abi}` instead",));
} else if let ExternAbi::Stdcall { unwind } = abi {
let c_abi = ExternAbi::C { unwind };
let system_abi = ExternAbi::System { unwind };
diag.help(format!(
"if you need `extern {abi}` on win32 and `extern {c_abi}` everywhere else, \
use `extern {system_abi}`"
));
}
}

pub fn check_abi(tcx: TyCtxt<'_>, hir_id: hir::HirId, span: Span, abi: ExternAbi) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand this PR correctly, we still do not call this function for trait declarations. This means that UNSUPPORTED_CALLING_CONVENTIONS is not emitted for them, and so moving cdecl and friends to Invalid will be another sudden breaking change with no prior lint. Is that right?

How hard would it be to find a place in hir_analysis (or wherever) to call this on trait declarations, too?

// FIXME: this should be checked earlier, e.g. in `rustc_ast_lowering`, to fix
// things like #86232.
fn add_help<T: EmissionGuarantee>(abi: ExternAbi, diag: &mut Diag<'_, T>) {
if let ExternAbi::Cdecl { unwind } = abi {
let c_abi = ExternAbi::C { unwind };
diag.help(format!("use `extern {c_abi}` instead",));
} else if let ExternAbi::Stdcall { unwind } = abi {
let c_abi = ExternAbi::C { unwind };
let system_abi = ExternAbi::System { unwind };
diag.help(format!(
"if you need `extern {abi}` on win32 and `extern {c_abi}` everywhere else, \
use `extern {system_abi}`"
));
}
}

match AbiMap::from_target(&tcx.sess.target).canonize_abi(abi, false) {
AbiMapping::Direct(..) => (),
AbiMapping::Invalid => {
let mut err = struct_span_code_err!(
tcx.dcx(),
span,
E0570,
"`{abi}` is not a supported ABI for the current target",
);
add_help(abi, &mut err);
err.emit();
}
// already erred in rustc_ast_lowering
AbiMapping::Invalid => (),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given the history of leaky checks, maybe add a delay_span_bug here?

AbiMapping::Deprecated(..) => {
tcx.node_span_lint(UNSUPPORTED_CALLING_CONVENTIONS, hir_id, span, |lint| {
lint.primary_message("use of calling convention not supported on this target");
add_help(abi, lint);
});
}
}
}

pub fn check_abi_fn_ptr(tcx: TyCtxt<'_>, hir_id: hir::HirId, span: Span, abi: ExternAbi) {
// This is always an FCW, even for `AbiMapping::Invalid`, since we started linting later than
// in `check_abi` above.
match AbiMap::from_target(&tcx.sess.target).canonize_abi(abi, false) {
AbiMapping::Direct(..) => (),
AbiMapping::Deprecated(..) | AbiMapping::Invalid => {
tcx.node_span_lint(UNSUPPORTED_FN_PTR_CALLING_CONVENTIONS, hir_id, span, |lint| {
lint.primary_message(format!(
"the calling convention {abi} is not supported on this target"
));
add_abi_diag_help(abi, lint);
});
}
}
Expand Down Expand Up @@ -845,6 +822,7 @@ pub(crate) fn check_item_type(tcx: TyCtxt<'_>, def_id: LocalDefId) {
let hir::ItemKind::ForeignMod { abi, items } = it.kind else {
return;
};

check_abi(tcx, it.hir_id(), it.span, abi);

for item in items {
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_hir_analysis/src/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,8 @@ pub mod wfcheck;

use std::num::NonZero;

pub use check::{check_abi, check_abi_fn_ptr};
use rustc_abi::{ExternAbi, VariantIdx};
pub use check::check_abi;
use rustc_abi::VariantIdx;
use rustc_data_structures::fx::{FxHashSet, FxIndexMap};
use rustc_errors::{Diag, ErrorGuaranteed, pluralize, struct_span_code_err};
use rustc_hir::def_id::{DefId, LocalDefId};
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ use rustc_trait_selection::traits::wf::object_region_bounds;
use rustc_trait_selection::traits::{self, ObligationCtxt};
use tracing::{debug, instrument};

use crate::check::check_abi_fn_ptr;
use crate::check::check_abi;
use crate::errors::{AmbiguousLifetimeBound, BadReturnTypeNotation};
use crate::hir_ty_lowering::errors::{GenericsArgsErrExtend, prohibit_assoc_item_constraint};
use crate::hir_ty_lowering::generics::{check_generic_arg_count, lower_generic_args};
Expand Down Expand Up @@ -2739,7 +2739,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
if let hir::Node::Ty(hir::Ty { kind: hir::TyKind::BareFn(bare_fn_ty), span, .. }) =
tcx.hir_node(hir_id)
{
check_abi_fn_ptr(tcx, hir_id, *span, bare_fn_ty.abi);
check_abi(tcx, hir_id, *span, bare_fn_ty.abi);
}

// reject function types that violate cmse ABI requirements
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_lint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -609,6 +609,7 @@ fn register_builtins(store: &mut LintStore) {
"converted into hard error, see issue #127323 \
<https://github.com/rust-lang/rust/issues/127323> for more information",
);
store.register_removed("unsupported_fn_ptr_calling_conventions", "converted into hard error");
store.register_removed(
"undefined_naked_function_abi",
"converted into hard error, see PR #139001 \
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_lint_defs/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,6 @@ declare_lint_pass! {
UNSAFE_OP_IN_UNSAFE_FN,
UNSTABLE_NAME_COLLISIONS,
UNSTABLE_SYNTAX_PRE_EXPANSION,
UNSUPPORTED_FN_PTR_CALLING_CONVENTIONS,
UNUSED_ASSIGNMENTS,
UNUSED_ASSOCIATED_TYPE_BOUNDS,
UNUSED_ATTRIBUTES,
Expand Down
10 changes: 0 additions & 10 deletions tests/crashes/132430.rs

This file was deleted.

7 changes: 0 additions & 7 deletions tests/crashes/138738.rs

This file was deleted.

4 changes: 2 additions & 2 deletions tests/debuginfo/type-names.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
// gdb-check:type = type_names::GenericStruct<type_names::mod1::Struct2, type_names::mod1::mod2::Struct3>

// gdb-command:whatis generic_struct2
// gdb-check:type = type_names::GenericStruct<type_names::Struct1, extern "fastcall" fn(isize) -> usize>
// gdb-check:type = type_names::GenericStruct<type_names::Struct1, extern "system" fn(isize) -> usize>

// gdb-command:whatis mod_struct
// gdb-check:type = type_names::mod1::Struct2
Expand Down Expand Up @@ -379,7 +379,7 @@ fn main() {
let simple_struct = Struct1;
let generic_struct1: GenericStruct<mod1::Struct2, mod1::mod2::Struct3> =
GenericStruct(PhantomData);
let generic_struct2: GenericStruct<Struct1, extern "fastcall" fn(isize) -> usize> =
let generic_struct2: GenericStruct<Struct1, extern "system" fn(isize) -> usize> =
GenericStruct(PhantomData);
let mod_struct = mod1::Struct2;

Expand Down
4 changes: 2 additions & 2 deletions tests/incremental/hashes/trait_defs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,7 @@ trait TraitAddExternModifier {

// Change extern "C" to extern "stdcall"
#[cfg(any(cfail1,cfail4))]
trait TraitChangeExternCToRustIntrinsic {
trait TraitChangeExternCToExternSystem {
// --------------------------------------------------------------
// -------------------------
// --------------------------------------------------------------
Expand All @@ -458,7 +458,7 @@ trait TraitChangeExternCToRustIntrinsic {
#[rustc_clean(cfg="cfail3")]
#[rustc_clean(except="opt_hir_owner_nodes,fn_sig", cfg="cfail5")]
#[rustc_clean(cfg="cfail6")]
extern "stdcall" fn method();
extern "system" fn method();
}


Expand Down
9 changes: 3 additions & 6 deletions tests/rustdoc-json/fn_pointer/abi.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#![feature(abi_vectorcall)]
#![feature(rust_cold_cc)]

//@ is "$.index[?(@.name=='AbiRust')].inner.type_alias.type.function_pointer.header.abi" \"Rust\"
pub type AbiRust = fn();
Expand All @@ -15,8 +15,5 @@ pub type AbiCUnwind = extern "C-unwind" fn();
//@ is "$.index[?(@.name=='AbiSystemUnwind')].inner.type_alias.type.function_pointer.header.abi" '{"System": {"unwind": true}}'
pub type AbiSystemUnwind = extern "system-unwind" fn();

//@ is "$.index[?(@.name=='AbiVecorcall')].inner.type_alias.type.function_pointer.header.abi.Other" '"\"vectorcall\""'
pub type AbiVecorcall = extern "vectorcall" fn();

//@ is "$.index[?(@.name=='AbiVecorcallUnwind')].inner.type_alias.type.function_pointer.header.abi.Other" '"\"vectorcall-unwind\""'
pub type AbiVecorcallUnwind = extern "vectorcall-unwind" fn();
//@ is "$.index[?(@.name=='AbiRustCold')].inner.type_alias.type.function_pointer.header.abi.Other" '"\"rust-cold\""'
pub type AbiRustCold = extern "rust-cold" fn();
9 changes: 3 additions & 6 deletions tests/rustdoc-json/fns/abi.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#![feature(abi_vectorcall)]
#![feature(rust_cold_cc)]

//@ is "$.index[?(@.name=='abi_rust')].inner.function.header.abi" \"Rust\"
pub fn abi_rust() {}
Expand All @@ -15,8 +15,5 @@ pub extern "C-unwind" fn abi_c_unwind() {}
//@ is "$.index[?(@.name=='abi_system_unwind')].inner.function.header.abi" '{"System": {"unwind": true}}'
pub extern "system-unwind" fn abi_system_unwind() {}

//@ is "$.index[?(@.name=='abi_vectorcall')].inner.function.header.abi.Other" '"\"vectorcall\""'
pub extern "vectorcall" fn abi_vectorcall() {}

//@ is "$.index[?(@.name=='abi_vectorcall_unwind')].inner.function.header.abi.Other" '"\"vectorcall-unwind\""'
pub extern "vectorcall-unwind" fn abi_vectorcall_unwind() {}
//@ is "$.index[?(@.name=='abi_rust_cold')].inner.function.header.abi.Other" '"\"rust-cold\""'
pub extern "rust-cold" fn abi_rust_cold() {}
17 changes: 5 additions & 12 deletions tests/rustdoc-json/methods/abi.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
#![feature(abi_vectorcall)]

#![feature(rust_cold_cc)]
//@ has "$.index[?(@.name=='Foo')]"
pub struct Foo;

Expand All @@ -19,11 +18,8 @@ impl Foo {
//@ is "$.index[?(@.name=='abi_system_unwind')].inner.function.header.abi" '{"System": {"unwind": true}}'
pub extern "system-unwind" fn abi_system_unwind() {}

//@ is "$.index[?(@.name=='abi_vectorcall')].inner.function.header.abi.Other" '"\"vectorcall\""'
pub extern "vectorcall" fn abi_vectorcall() {}

//@ is "$.index[?(@.name=='abi_vectorcall_unwind')].inner.function.header.abi.Other" '"\"vectorcall-unwind\""'
pub extern "vectorcall-unwind" fn abi_vectorcall_unwind() {}
//@ is "$.index[?(@.name=='abi_rust_cold')].inner.function.header.abi.Other" '"\"rust-cold\""'
pub extern "rust-cold" fn abi_rust_cold() {}
}

pub trait Bar {
Expand All @@ -42,9 +38,6 @@ pub trait Bar {
//@ is "$.index[?(@.name=='trait_abi_system_unwind')].inner.function.header.abi" '{"System": {"unwind": true}}'
extern "system-unwind" fn trait_abi_system_unwind() {}

//@ is "$.index[?(@.name=='trait_abi_vectorcall')].inner.function.header.abi.Other" '"\"vectorcall\""'
extern "vectorcall" fn trait_abi_vectorcall() {}

//@ is "$.index[?(@.name=='trait_abi_vectorcall_unwind')].inner.function.header.abi.Other" '"\"vectorcall-unwind\""'
extern "vectorcall-unwind" fn trait_abi_vectorcall_unwind() {}
//@ is "$.index[?(@.name=='trait_abi_rust_cold')].inner.function.header.abi.Other" '"\"rust-cold\""'
extern "rust-cold" fn trait_abi_rust_cold() {}
}
27 changes: 27 additions & 0 deletions tests/rustdoc-json/vectorcall.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
#![feature(abi_vectorcall)]
//@ only-x86_64

//@ is "$.index[?(@.name=='AbiVectorcall')].inner.type_alias.type.function_pointer.header.abi.Other" '"\"vectorcall\""'
pub type AbiVectorcall = extern "vectorcall" fn();

//@ is "$.index[?(@.name=='AbiVectorcallUnwind')].inner.type_alias.type.function_pointer.header.abi.Other" '"\"vectorcall-unwind\""'
pub type AbiVectorcallUnwind = extern "vectorcall-unwind" fn();

//@ has "$.index[?(@.name=='Foo')]"
pub struct Foo;

impl Foo {
//@ is "$.index[?(@.name=='abi_vectorcall')].inner.function.header.abi.Other" '"\"vectorcall\""'
pub extern "vectorcall" fn abi_vectorcall() {}

//@ is "$.index[?(@.name=='abi_vectorcall_unwind')].inner.function.header.abi.Other" '"\"vectorcall-unwind\""'
pub extern "vectorcall-unwind" fn abi_vectorcall_unwind() {}
}

pub trait Bar {
//@ is "$.index[?(@.name=='trait_abi_vectorcall')].inner.function.header.abi.Other" '"\"vectorcall\""'
extern "vectorcall" fn trait_abi_vectorcall() {}

//@ is "$.index[?(@.name=='trait_abi_vectorcall_unwind')].inner.function.header.abi.Other" '"\"vectorcall-unwind\""'
extern "vectorcall-unwind" fn trait_abi_vectorcall_unwind() {}
}
7 changes: 7 additions & 0 deletions tests/ui/abi/unsupported-abi-transmute.rs
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All these tests seem to assume that "gpu-kernel" is not a valid ABI. But that's target-specific, so this will fail when the test suite is run on a GPU target, right?

Like the other unsupported ABI tests, I think this should use --target and #![no_core] (ideally with minicore) so that the test does not depend on what the host target happens to be.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm, I mean technically GPUs shouldn't ever be hosts so I wouldn't think we would build tests for them, but then Aaron Hsu's codfns compiler exists so maybe I shouldn't think that.

Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#![feature(abi_gpu_kernel)]
// Check we error before unsupported ABIs reach codegen stages.

fn main() {
let a = unsafe { core::mem::transmute::<usize, extern "gpu-kernel" fn(i32)>(4) }(2);
//~^ ERROR E0570
}
9 changes: 9 additions & 0 deletions tests/ui/abi/unsupported-abi-transmute.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
error[E0570]: `"gpu-kernel"` is not a supported ABI for the current target
--> $DIR/unsupported-abi-transmute.rs:5:59
|
LL | let a = unsafe { core::mem::transmute::<usize, extern "gpu-kernel" fn(i32)>(4) }(2);
| ^^^^^^^^^^^^

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0570`.
9 changes: 9 additions & 0 deletions tests/ui/abi/unsupported-extern-abi-in-type-impl.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
//@ compile-flags: --crate-type=lib
//@ edition: 2018
#![feature(abi_gpu_kernel)]
struct Test;

impl Test {
pub extern "gpu-kernel" fn test(val: &str) {}
//~^ ERROR [E0570]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Error codes are pretty meaningless, making the test file hard to read. IMO it'd be better to have a snippet of the error message in the test file here.

}
9 changes: 9 additions & 0 deletions tests/ui/abi/unsupported-extern-abi-in-type-impl.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
error[E0570]: `"gpu-kernel"` is not a supported ABI for the current target
--> $DIR/unsupported-extern-abi-in-type-impl.rs:7:16
|
LL | pub extern "gpu-kernel" fn test(val: &str) {}
| ^^^^^^^^^^^^

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0570`.
11 changes: 11 additions & 0 deletions tests/ui/abi/unsupported-trait-decl.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
//@ compile-flags: --crate-type=lib

#![feature(abi_gpu_kernel)]

trait T {
extern "gpu-kernel" fn mu();
//~^ ERROR[E0570]
}

type TAU = extern "gpu-kernel" fn();
//~^ ERROR[E0570]
15 changes: 15 additions & 0 deletions tests/ui/abi/unsupported-trait-decl.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
error[E0570]: `"gpu-kernel"` is not a supported ABI for the current target
--> $DIR/unsupported-trait-decl.rs:6:12
|
LL | extern "gpu-kernel" fn mu();
| ^^^^^^^^^^^^

error[E0570]: `"gpu-kernel"` is not a supported ABI for the current target
--> $DIR/unsupported-trait-decl.rs:10:19
|
LL | type TAU = extern "gpu-kernel" fn();
| ^^^^^^^^^^^^

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0570`.
Loading
Loading