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
39 changes: 39 additions & 0 deletions compiler/rustc_error_codes/src/error_codes/E0798.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
Functions marked as `C-cmse-nonsecure-call` place restrictions on their
inputs and outputs.

- inputs must fit in the 4 available 32-bit argument registers. Alignment
is relevant.
- outputs must either fit in 4 bytes, or be a foundational type of
size 8 (`i64`, `u64`, `f64`).
- no generics can be used in the signature

For more information,
see [arm's aapcs32](https://github.com/ARM-software/abi-aa/releases).

Erroneous code example:

```ignore (only fails on supported targets)
#![feature(abi_c_cmse_nonsecure_call)]
#[no_mangle]
pub fn test(
f: extern "C-cmse-nonsecure-call" fn(u32, u32, u32, u32, u32) -> u32,
) -> u32 {
f(1, 2, 3, 4, 5)
}
```

Arguments' alignment is respected. In the example below, padding is inserted
so that the `u64` argument is passed in registers r2 and r3. There is then no
room left for the final `f32` argument

```ignore (only fails on supported targets)
#![feature(abi_c_cmse_nonsecure_call)]
#[no_mangle]
pub fn test(
f: extern "C-cmse-nonsecure-call" fn(u32, u64, f32) -> u32,
) -> u32 {
f(1, 2, 3.0)
}
```
1 change: 1 addition & 0 deletions compiler/rustc_error_codes/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -536,6 +536,7 @@ E0794: 0794,
E0795: 0795,
E0796: 0796,
E0797: 0797,
E0798: 0798,
);
)
}
Expand Down
17 changes: 17 additions & 0 deletions compiler/rustc_hir_analysis/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,23 @@ hir_analysis_cannot_capture_late_bound_ty =
hir_analysis_closure_implicit_hrtb = implicit types in closure signatures are forbidden when `for<...>` is present
.label = `for<...>` is here

hir_analysis_cmse_call_generic =
function pointers with the `"C-cmse-nonsecure-call"` ABI cannot contain generics in their type

hir_analysis_cmse_call_inputs_stack_spill =
arguments for `"C-cmse-nonsecure-call"` function too large to pass via 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
.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

hir_analysis_coerce_unsized_multi = implementing the trait `CoerceUnsized` requires multiple coercions
Expand Down
27 changes: 27 additions & 0 deletions compiler/rustc_hir_analysis/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1682,3 +1682,30 @@ pub struct InvalidReceiverTy<'tcx> {
#[note]
#[help]
pub struct EffectsWithoutNextSolver;

#[derive(Diagnostic)]
#[diag(hir_analysis_cmse_call_inputs_stack_spill, code = E0798)]
#[note]
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(hir_analysis_note1)]
#[note(hir_analysis_note2)]
pub struct CmseCallOutputStackSpill {
#[primary_span]
#[label]
pub span: Span,
}

#[derive(Diagnostic)]
#[diag(hir_analysis_cmse_call_generic, code = E0798)]
pub struct CmseCallGeneric {
#[primary_span]
pub span: Span,
}
156 changes: 156 additions & 0 deletions compiler/rustc_hir_analysis/src/hir_ty_lowering/cmse.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
use rustc_errors::DiagCtxtHandle;
use rustc_hir as hir;
use rustc_hir::HirId;
use rustc_middle::ty::layout::LayoutError;
use rustc_middle::ty::{self, ParamEnv, TyCtxt};
use rustc_span::Span;
use rustc_target::spec::abi;

use crate::errors;

/// Check conditions on inputs and outputs that the cmse ABIs impose: arguments and results MUST be
/// returned via registers (i.e. MUST NOT spill to the stack). LLVM will also validate these
/// conditions, but by checking them here rustc can emit nicer error messages.
pub fn validate_cmse_abi<'tcx>(
tcx: TyCtxt<'tcx>,
dcx: DiagCtxtHandle<'_>,
hir_id: HirId,
abi: abi::Abi,
fn_sig: ty::PolyFnSig<'tcx>,
) {
if let abi::Abi::CCmseNonSecureCall = abi {
let hir_node = tcx.hir_node(hir_id);
let hir::Node::Ty(hir::Ty {
span: bare_fn_span,
kind: hir::TyKind::BareFn(bare_fn_ty),
..
}) = hir_node
else {
// might happen when this ABI is used incorrectly. That will be handled elsewhere
return;
};

oli-obk marked this conversation as resolved.
Show resolved Hide resolved
match is_valid_cmse_inputs(tcx, fn_sig) {
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, *bare_fn_span) {
dcx.emit_err(err);
}
}
}

match is_valid_cmse_output(tcx, fn_sig) {
Ok(true) => {}
Ok(false) => {
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, *bare_fn_span) {
dcx.emit_err(err);
}
}
};
}
}

/// Returns whether the inputs will fit into the available registers
fn is_valid_cmse_inputs<'tcx>(
tcx: TyCtxt<'tcx>,
fn_sig: ty::PolyFnSig<'tcx>,
) -> Result<Result<(), usize>, &'tcx LayoutError<'tcx>> {
let mut span = None;
let mut accum = 0u64;

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));
}
}

match span {
None => Ok(Ok(())),
Some(span) => Ok(Err(span)),
}
}

/// Returns whether the output will fit into the available registers
fn is_valid_cmse_output<'tcx>(
tcx: TyCtxt<'tcx>,
fn_sig: ty::PolyFnSig<'tcx>,
) -> Result<bool, &'tcx LayoutError<'tcx>> {
let mut ret_ty = fn_sig.output().skip_binder();
let layout = tcx.layout_of(ParamEnv::reveal_all().and(ret_ty))?;
let size = layout.layout.size().bytes();

if size <= 4 {
return Ok(true);
} else if size > 8 {
return Ok(false);
oli-obk marked this conversation as resolved.
Show resolved Hide resolved
}

// next we need to peel any repr(transparent) layers off
'outer: loop {
let ty::Adt(adt_def, args) = ret_ty.kind() else {
break;
};

if !adt_def.repr().transparent() {
break;
}

// the first field with non-trivial size and alignment must be the data
for variant_def in adt_def.variants() {
for field_def in variant_def.fields.iter() {
let ty = field_def.ty(tcx, args);
let layout = tcx.layout_of(ParamEnv::reveal_all().and(ty))?;

if !layout.layout.is_1zst() {
ret_ty = ty;
continue 'outer;
}
}
}
}

Ok(ret_ty == tcx.types.i64 || ret_ty == tcx.types.u64 || ret_ty == tcx.types.f64)
}

fn cmse_layout_err<'tcx>(
layout_err: &'tcx LayoutError<'tcx>,
span: Span,
) -> Option<crate::errors::CmseCallGeneric> {
use LayoutError::*;

match layout_err {
Unknown(ty) => {
if ty.is_impl_trait() {
None // prevent double reporting of this error
} else {
Some(errors::CmseCallGeneric { span })
}
}
SizeOverflow(..) | NormalizationFailure(..) | ReferencesError(..) | Cycle(..) => {
None // not our job to report these
}
}
}
4 changes: 4 additions & 0 deletions compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
//! trait references and bounds.

mod bounds;
mod cmse;
pub mod errors;
pub mod generics;
mod lint;
Expand Down Expand Up @@ -2324,6 +2325,9 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
let fn_ty = tcx.mk_fn_sig(input_tys, output_ty, decl.c_variadic, safety, abi);
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);

// Find any late-bound regions declared in return type that do
// not appear in the arguments. These are not well-formed.
//
Expand Down
21 changes: 21 additions & 0 deletions tests/ui/cmse-nonsecure/cmse-nonsecure-call/generics.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
//@ compile-flags: --target thumbv8m.main-none-eabi --crate-type lib
//@ needs-llvm-components: arm
#![feature(abi_c_cmse_nonsecure_call, no_core, lang_items)]
#![no_core]
#[lang = "sized"]
pub trait Sized {}
#[lang = "copy"]
pub trait Copy {}
impl Copy for u32 {}

#[repr(C)]
struct Wrapper<T>(T);

struct Test<T: Copy> {
f1: extern "C-cmse-nonsecure-call" fn<U: Copy>(U, u32, u32, u32) -> u64, //~ ERROR cannot find type `U` in this scope
//~^ ERROR function pointer types may not have generic parameters
f2: extern "C-cmse-nonsecure-call" fn(impl Copy, u32, u32, u32) -> u64,
//~^ ERROR `impl Trait` is not allowed in `fn` pointer parameters
f3: extern "C-cmse-nonsecure-call" fn(T, u32, u32, u32) -> u64, //~ ERROR [E0798]
f4: extern "C-cmse-nonsecure-call" fn(Wrapper<T>, u32, u32, u32) -> u64, //~ ERROR [E0798]
}
47 changes: 47 additions & 0 deletions tests/ui/cmse-nonsecure/cmse-nonsecure-call/generics.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
error: function pointer types may not have generic parameters
--> $DIR/generics.rs:15:42
|
LL | f1: extern "C-cmse-nonsecure-call" fn<U: Copy>(U, u32, u32, u32) -> u64,
| ^^^^^^^^^

error[E0412]: cannot find type `U` in this scope
--> $DIR/generics.rs:15:52
|
LL | struct Test<T: Copy> {
| - similarly named type parameter `T` defined here
LL | f1: extern "C-cmse-nonsecure-call" fn<U: Copy>(U, u32, u32, u32) -> u64,
| ^
|
help: a type parameter with a similar name exists
|
LL | f1: extern "C-cmse-nonsecure-call" fn<U: Copy>(T, u32, u32, u32) -> u64,
| ~
help: you might be missing a type parameter
|
LL | struct Test<T: Copy, U> {
| +++

error[E0562]: `impl Trait` is not allowed in `fn` pointer parameters
--> $DIR/generics.rs:17:43
|
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: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:9
|
LL | f4: extern "C-cmse-nonsecure-call" fn(Wrapper<T>, u32, u32, u32) -> u64,
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to 5 previous errors

Some errors have detailed explanations: E0412, E0562, E0798.
For more information about an error, try `rustc --explain E0412`.
24 changes: 0 additions & 24 deletions tests/ui/cmse-nonsecure/cmse-nonsecure-call/params-on-registers.rs

This file was deleted.

Loading
Loading