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

C-cmse-nonsecure-call: improved error messages #127814

Merged
merged 11 commits into from
Jul 19, 2024
8 changes: 6 additions & 2 deletions compiler/rustc_hir_analysis/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -63,13 +63,17 @@ hir_analysis_cmse_call_generic =

hir_analysis_cmse_call_inputs_stack_spill =
arguments for `"C-cmse-nonsecure-call"` function too large to pass via registers
.label = these arguments don't fit in the available registers
.label = {$plural ->
[false] this argument doesn't
*[true] these arguments don't
} fit in the available registers
.note = functions with the `"C-cmse-nonsecure-call"` ABI must pass all their arguments via the 4 32-bit available argument registers

hir_analysis_cmse_call_output_stack_spill =
return value of `"C-cmse-nonsecure-call"` function too large to pass via registers
.label = this type doesn't fit in the available registers
.note = functions with the `"C-cmse-nonsecure-call"` ABI must pass their result via the available return registers
.note1 = functions with the `"C-cmse-nonsecure-call"` ABI must pass their result via the available return registers
.note2 = the result must either be a (transparently wrapped) i64, u64 or f64, or be at most 4 bytes in size

hir_analysis_coerce_unsized_may = the trait `{$trait_name}` may only be implemented for a coercion between structures

Expand Down
4 changes: 3 additions & 1 deletion compiler/rustc_hir_analysis/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1690,11 +1690,13 @@ pub struct CmseCallInputsStackSpill {
#[primary_span]
#[label]
pub span: Span,
pub plural: bool,
}

#[derive(Diagnostic)]
#[diag(hir_analysis_cmse_call_output_stack_spill, code = E0798)]
#[note]
#[note(hir_analysis_note1)]
#[note(hir_analysis_note2)]
pub struct CmseCallOutputStackSpill {
#[primary_span]
#[label]
Expand Down
50 changes: 27 additions & 23 deletions compiler/rustc_hir_analysis/src/hir_ty_lowering/cmse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use crate::errors;
/// conditions, but by checking them here rustc can emit nicer error messages.
pub fn validate_cmse_abi<'tcx>(
tcx: TyCtxt<'tcx>,
dcx: &DiagCtxtHandle<'_>,
dcx: DiagCtxtHandle<'_>,
hir_id: HirId,
abi: abi::Abi,
fn_sig: ty::PolyFnSig<'tcx>,
Expand All @@ -30,25 +30,20 @@ pub fn validate_cmse_abi<'tcx>(
return;
};

// fn(u32, u32, u32, u16, u16) -> u32,
// ^^^^^^^^^^^^^^^^^^^^^^^ ^^^
let output_span = bare_fn_ty.decl.output.span();
let inputs_span = match (
bare_fn_ty.param_names.first(),
bare_fn_ty.decl.inputs.first(),
bare_fn_ty.decl.inputs.last(),
) {
(Some(ident), Some(ty1), Some(ty2)) => ident.span.to(ty1.span).to(ty2.span),
_ => *bare_fn_span,
};

match is_valid_cmse_inputs(tcx, fn_sig) {
Ok(true) => {}
Ok(false) => {
dcx.emit_err(errors::CmseCallInputsStackSpill { span: inputs_span });
Ok(Ok(())) => {}
Ok(Err(index)) => {
// fn(x: u32, u32, u32, u16, y: u16) -> u32,
// ^^^^^^
let span = bare_fn_ty.param_names[index]
.span
.to(bare_fn_ty.decl.inputs[index].span)
.to(bare_fn_ty.decl.inputs.last().unwrap().span);
let plural = bare_fn_ty.param_names.len() - index != 1;
dcx.emit_err(errors::CmseCallInputsStackSpill { span, plural });
}
Err(layout_err) => {
if let Some(err) = cmse_layout_err(layout_err, inputs_span) {
if let Some(err) = cmse_layout_err(layout_err, *bare_fn_span) {
dcx.emit_err(err);
}
}
Expand All @@ -57,10 +52,11 @@ pub fn validate_cmse_abi<'tcx>(
match is_valid_cmse_output(tcx, fn_sig) {
Ok(true) => {}
Ok(false) => {
dcx.emit_err(errors::CmseCallOutputStackSpill { span: output_span });
let span = bare_fn_ty.decl.output.span();
dcx.emit_err(errors::CmseCallOutputStackSpill { span });
}
Err(layout_err) => {
if let Some(err) = cmse_layout_err(layout_err, output_span) {
if let Some(err) = cmse_layout_err(layout_err, *bare_fn_span) {
dcx.emit_err(err);
}
}
Expand All @@ -72,21 +68,29 @@ pub fn validate_cmse_abi<'tcx>(
fn is_valid_cmse_inputs<'tcx>(
tcx: TyCtxt<'tcx>,
fn_sig: ty::PolyFnSig<'tcx>,
) -> Result<bool, &'tcx LayoutError<'tcx>> {
) -> Result<Result<(), usize>, &'tcx LayoutError<'tcx>> {
let mut span = None;
let mut accum = 0u64;

for arg_def in fn_sig.inputs().iter() {
for (index, arg_def) in fn_sig.inputs().iter().enumerate() {
let layout = tcx.layout_of(ParamEnv::reveal_all().and(*arg_def.skip_binder()))?;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think at this point it may be easier to just emit the errors in is_valid_cmse_inputs and _output directly. Then you can also report a "generics not allowed" error with a span pointing at the offending thing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we have 2 reasons not to do that

  • arg_def doesn't have any span information. it's effectively just a Ty
  • in the (near, hopefully) future, the C-cmse-nonsecure-entry abi will require the same validation, and we'd like to reuse this code, so passing in BareFn-specific data will not work


let align = layout.layout.align().abi.bytes();
let size = layout.layout.size().bytes();

accum += size;
accum = accum.next_multiple_of(Ord::max(4, align));

// i.e. exceeds 4 32-bit registers
if accum > 16 {
span = span.or(Some(index));
}
}

// i.e. 4 32-bit registers
Ok(accum <= 16)
match span {
None => Ok(Ok(())),
Some(span) => Ok(Err(span)),
}
}

/// Returns whether the output will fit into the available registers
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2326,7 +2326,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
let bare_fn_ty = ty::Binder::bind_with_vars(fn_ty, bound_vars);

// reject function types that violate cmse ABI requirements
cmse::validate_cmse_abi(self.tcx(), &self.dcx(), hir_id, abi, bare_fn_ty);
cmse::validate_cmse_abi(self.tcx(), self.dcx(), hir_id, abi, bare_fn_ty);

// Find any late-bound regions declared in return type that do
// not appear in the arguments. These are not well-formed.
Expand Down
8 changes: 4 additions & 4 deletions tests/ui/cmse-nonsecure/cmse-nonsecure-call/generics.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -30,16 +30,16 @@ LL | f2: extern "C-cmse-nonsecure-call" fn(impl Copy, u32, u32, u32) -> u64,
= note: `impl Trait` is only allowed in arguments and return types of functions and methods

error[E0798]: function pointers with the `"C-cmse-nonsecure-call"` ABI cannot contain generics in their type
--> $DIR/generics.rs:19:43
--> $DIR/generics.rs:19:9
|
LL | f3: extern "C-cmse-nonsecure-call" fn(T, u32, u32, u32) -> u64,
| ^^^^^^^^^^^^^^^^
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0798]: function pointers with the `"C-cmse-nonsecure-call"` ABI cannot contain generics in their type
--> $DIR/generics.rs:20:43
--> $DIR/generics.rs:20:9
|
LL | f4: extern "C-cmse-nonsecure-call" fn(Wrapper<T>, u32, u32, u32) -> u64,
| ^^^^^^^^^^^^^^^^^^^^^^^^^
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to 5 previous errors

Expand Down
10 changes: 5 additions & 5 deletions tests/ui/cmse-nonsecure/cmse-nonsecure-call/params-via-stack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ pub struct AlignRelevant(u32);

#[no_mangle]
pub fn test(
f1: extern "C-cmse-nonsecure-call" fn(u32, u32, u32, u32, u32), //~ ERROR [E0798]
f2: extern "C-cmse-nonsecure-call" fn(u32, u32, u32, u16, u16), //~ ERROR [E0798]
f3: extern "C-cmse-nonsecure-call" fn(u32, u64, u32), //~ ERROR [E0798]
f4: extern "C-cmse-nonsecure-call" fn(AlignRelevant, u32), //~ ERROR [E0798]
f5: extern "C-cmse-nonsecure-call" fn([u32; 5]), //~ ERROR [E0798]
f1: extern "C-cmse-nonsecure-call" fn(u32, u32, u32, u32, x: u32, y: u32), //~ ERROR [E0798]
f2: extern "C-cmse-nonsecure-call" fn(u32, u32, u32, u16, u16), //~ ERROR [E0798]
f3: extern "C-cmse-nonsecure-call" fn(u32, u64, u32), //~ ERROR [E0798]
f4: extern "C-cmse-nonsecure-call" fn(AlignRelevant, u32), //~ ERROR [E0798]
f5: extern "C-cmse-nonsecure-call" fn([u32; 5]), //~ ERROR [E0798]
) {
}
20 changes: 10 additions & 10 deletions tests/ui/cmse-nonsecure/cmse-nonsecure-call/params-via-stack.stderr
Original file line number Diff line number Diff line change
@@ -1,40 +1,40 @@
error[E0798]: arguments for `"C-cmse-nonsecure-call"` function too large to pass via registers
--> $DIR/params-via-stack.rs:17:43
--> $DIR/params-via-stack.rs:17:63
|
LL | f1: extern "C-cmse-nonsecure-call" fn(u32, u32, u32, u32, u32),
| ^^^^^^^^^^^^^^^^^^^^^^^ these arguments don't fit in the available registers
LL | f1: extern "C-cmse-nonsecure-call" fn(u32, u32, u32, u32, x: u32, y: u32),
| ^^^^^^^^^^^^^^ these arguments don't fit in the available registers
|
= note: functions with the `"C-cmse-nonsecure-call"` ABI must pass all their arguments via the 4 32-bit available argument registers

error[E0798]: arguments for `"C-cmse-nonsecure-call"` function too large to pass via registers
--> $DIR/params-via-stack.rs:18:43
--> $DIR/params-via-stack.rs:18:63
|
LL | f2: extern "C-cmse-nonsecure-call" fn(u32, u32, u32, u16, u16),
| ^^^^^^^^^^^^^^^^^^^^^^^ these arguments don't fit in the available registers
| ^^^ this argument doesn't fit in the available registers
|
= note: functions with the `"C-cmse-nonsecure-call"` ABI must pass all their arguments via the 4 32-bit available argument registers

error[E0798]: arguments for `"C-cmse-nonsecure-call"` function too large to pass via registers
--> $DIR/params-via-stack.rs:19:43
--> $DIR/params-via-stack.rs:19:53
|
LL | f3: extern "C-cmse-nonsecure-call" fn(u32, u64, u32),
| ^^^^^^^^^^^^^ these arguments don't fit in the available registers
| ^^^ this argument doesn't fit in the available registers
|
= note: functions with the `"C-cmse-nonsecure-call"` ABI must pass all their arguments via the 4 32-bit available argument registers

error[E0798]: arguments for `"C-cmse-nonsecure-call"` function too large to pass via registers
--> $DIR/params-via-stack.rs:20:43
--> $DIR/params-via-stack.rs:20:58
|
LL | f4: extern "C-cmse-nonsecure-call" fn(AlignRelevant, u32),
| ^^^^^^^^^^^^^^^^^^ these arguments don't fit in the available registers
| ^^^ this argument doesn't fit in the available registers
|
= note: functions with the `"C-cmse-nonsecure-call"` ABI must pass all their arguments via the 4 32-bit available argument registers

error[E0798]: arguments for `"C-cmse-nonsecure-call"` function too large to pass via registers
--> $DIR/params-via-stack.rs:21:43
|
LL | f5: extern "C-cmse-nonsecure-call" fn([u32; 5]),
| ^^^^^^^^ these arguments don't fit in the available registers
| ^^^^^^^^ this argument doesn't fit in the available registers
|
= note: functions with the `"C-cmse-nonsecure-call"` ABI must pass all their arguments via the 4 32-bit available argument registers

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ LL | u128: extern "C-cmse-nonsecure-call" fn() -> u128,
| ^^^^ this type doesn't fit in the available registers
|
= note: functions with the `"C-cmse-nonsecure-call"` ABI must pass their result via the available return registers
= note: the result must either be a (transparently wrapped) i64, u64 or f64, or be at most 4 bytes in size

error[E0798]: return value of `"C-cmse-nonsecure-call"` function too large to pass via registers
--> $DIR/return-via-stack.rs:36:50
Expand All @@ -13,6 +14,7 @@ LL | i128: extern "C-cmse-nonsecure-call" fn() -> i128,
| ^^^^ this type doesn't fit in the available registers
|
= note: functions with the `"C-cmse-nonsecure-call"` ABI must pass their result via the available return registers
= note: the result must either be a (transparently wrapped) i64, u64 or f64, or be at most 4 bytes in size

error[E0798]: return value of `"C-cmse-nonsecure-call"` function too large to pass via registers
--> $DIR/return-via-stack.rs:25:48
Expand All @@ -21,6 +23,7 @@ LL | f1: extern "C-cmse-nonsecure-call" fn() -> ReprCU64,
| ^^^^^^^^ this type doesn't fit in the available registers
|
= note: functions with the `"C-cmse-nonsecure-call"` ABI must pass their result via the available return registers
= note: the result must either be a (transparently wrapped) i64, u64 or f64, or be at most 4 bytes in size

error[E0798]: return value of `"C-cmse-nonsecure-call"` function too large to pass via registers
--> $DIR/return-via-stack.rs:26:48
Expand All @@ -29,6 +32,7 @@ LL | f2: extern "C-cmse-nonsecure-call" fn() -> ReprCBytes,
| ^^^^^^^^^^ this type doesn't fit in the available registers
|
= note: functions with the `"C-cmse-nonsecure-call"` ABI must pass their result via the available return registers
= note: the result must either be a (transparently wrapped) i64, u64 or f64, or be at most 4 bytes in size

error[E0798]: return value of `"C-cmse-nonsecure-call"` function too large to pass via registers
--> $DIR/return-via-stack.rs:27:48
Expand All @@ -37,6 +41,7 @@ LL | f3: extern "C-cmse-nonsecure-call" fn() -> U64Compound,
| ^^^^^^^^^^^ this type doesn't fit in the available registers
|
= note: functions with the `"C-cmse-nonsecure-call"` ABI must pass their result via the available return registers
= note: the result must either be a (transparently wrapped) i64, u64 or f64, or be at most 4 bytes in size

error[E0798]: return value of `"C-cmse-nonsecure-call"` function too large to pass via registers
--> $DIR/return-via-stack.rs:28:48
Expand All @@ -45,6 +50,7 @@ LL | f4: extern "C-cmse-nonsecure-call" fn() -> ReprCAlign16,
| ^^^^^^^^^^^^ this type doesn't fit in the available registers
|
= note: functions with the `"C-cmse-nonsecure-call"` ABI must pass their result via the available return registers
= note: the result must either be a (transparently wrapped) i64, u64 or f64, or be at most 4 bytes in size

error[E0798]: return value of `"C-cmse-nonsecure-call"` function too large to pass via registers
--> $DIR/return-via-stack.rs:29:48
Expand All @@ -53,6 +59,7 @@ LL | f5: extern "C-cmse-nonsecure-call" fn() -> [u8; 5],
| ^^^^^^^ this type doesn't fit in the available registers
|
= note: functions with the `"C-cmse-nonsecure-call"` ABI must pass their result via the available return registers
= note: the result must either be a (transparently wrapped) i64, u64 or f64, or be at most 4 bytes in size

error[E0798]: return value of `"C-cmse-nonsecure-call"` function too large to pass via registers
--> $DIR/return-via-stack.rs:51:48
Expand All @@ -61,6 +68,7 @@ LL | f1: extern "C-cmse-nonsecure-call" fn() -> ReprRustUnionU64,
| ^^^^^^^^^^^^^^^^ this type doesn't fit in the available registers
|
= note: functions with the `"C-cmse-nonsecure-call"` ABI must pass their result via the available return registers
= note: the result must either be a (transparently wrapped) i64, u64 or f64, or be at most 4 bytes in size

error[E0798]: return value of `"C-cmse-nonsecure-call"` function too large to pass via registers
--> $DIR/return-via-stack.rs:52:48
Expand All @@ -69,6 +77,7 @@ LL | f2: extern "C-cmse-nonsecure-call" fn() -> ReprCUnionU64,
| ^^^^^^^^^^^^^ this type doesn't fit in the available registers
|
= note: functions with the `"C-cmse-nonsecure-call"` ABI must pass their result via the available return registers
= note: the result must either be a (transparently wrapped) i64, u64 or f64, or be at most 4 bytes in size

error: aborting due to 9 previous errors

Expand Down
Loading