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.

Sign up for GitHub

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

Jump to bottom

fix ICE in CMSE type validation #130064

Merged
merged 2 commits into from
Sep 9, 2024
Merged

fix ICE in CMSE type validation #130064

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
46115fd
fix ICE in CMSE type validation
folkertdev Sep 6, 2024
e186cc6
do `PolyFnSig` -> `FnSig` conversion later
folkertdev Sep 9, 2024
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
13 changes: 8 additions & 5 deletions compiler/rustc_hir_analysis/src/hir_ty_lowering/cmse.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ pub(crate) fn validate_cmse_abi<'tcx>(
abi: abi::Abi,
fn_sig: ty::PolyFnSig<'tcx>,
) {
// this type is only used for layout computation, which does not rely on regions
let fn_sig = tcx.instantiate_bound_regions_with_erased(fn_sig);

if let abi::Abi::CCmseNonSecureCall = abi {
let hir_node = tcx.hir_node(hir_id);
let hir::Node::Ty(hir::Ty {
Expand Down Expand Up @@ -67,13 +70,13 @@ pub(crate) fn validate_cmse_abi<'tcx>(
/// Returns whether the inputs will fit into the available registers
fn is_valid_cmse_inputs<'tcx>(
tcx: TyCtxt<'tcx>,
fn_sig: ty::PolyFnSig<'tcx>,
fn_sig: ty::FnSig<'tcx>,
Copy link
Member

Choose a reason for hiding this comment

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

One tweak -- can you change this back to PolyFnSig and do the binder instantiate call inside this fn?

) -> 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()))?;
for (index, ty) in fn_sig.inputs().iter().enumerate() {
let layout = tcx.layout_of(ParamEnv::reveal_all().and(*ty))?;
Copy link
Member

Choose a reason for hiding this comment

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

Side-note: this param-env is not right, so it'll probably ICE in other cases...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, #130104 was just reported and is not fixed by this branch (currently). How do we get a correct ParamEnv here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

actually that second ICE is due to encountering Infer being considered a bug in layout computation

rust/compiler/rustc_ty_utils/src/layout.rs

Lines 677 to 683 in 263a3ae

ty::Bound(..) | ty::CoroutineWitness(..) | ty::Infer(_) | ty::Error(_) => {
bug!("Layout::compute: unexpected type `{}`", ty)
}
ty::Placeholder(..) | ty::Param(_) => {
return Err(error(cx, LayoutError::Unknown(ty)));
}

Moving ty::Infer(_) to the ty::Placeholder(..) branch fixes that problem, but might hide bugs in other cases?

Copy link
Member

Choose a reason for hiding this comment

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

yeah, that is 100% not the right fix. the problem here is that we need to delay the calculation of these layouts until after type-checking is done for types that show up.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

here #127814 (comment) is where some discussion around the current design happened. Is there an existing way to perform that traversal after type checking to perform this layout check?


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

Expand Down
25 changes: 23 additions & 2 deletions tests/ui/cmse-nonsecure/cmse-nonsecure-call/generics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,31 @@ impl Copy for u32 {}
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
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]
}

type WithReference = extern "C-cmse-nonsecure-call" fn(&usize);

trait Trait {}
type WithTraitObject = extern "C-cmse-nonsecure-call" fn(&dyn Trait) -> &dyn Trait;
//~^ ERROR return value of `"C-cmse-nonsecure-call"` function too large to pass via registers [E0798]

type WithStaticTraitObject =
extern "C-cmse-nonsecure-call" fn(&'static dyn Trait) -> &'static dyn Trait;
//~^ ERROR return value of `"C-cmse-nonsecure-call"` function too large to pass via registers [E0798]

#[repr(transparent)]
struct WrapperTransparent<'a>(&'a dyn Trait);

type WithTransparentTraitObject =
extern "C-cmse-nonsecure-call" fn(WrapperTransparent) -> WrapperTransparent;
//~^ ERROR return value of `"C-cmse-nonsecure-call"` function too large to pass via registers [E0798]

type WithVarArgs = extern "C-cmse-nonsecure-call" fn(u32, ...);
//~^ ERROR C-variadic function must have a compatible calling convention, like `C` or `cdecl` [E0045]
45 changes: 39 additions & 6 deletions tests/ui/cmse-nonsecure/cmse-nonsecure-call/generics.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,26 +22,59 @@ LL | struct Test<T: Copy, U> {
| +++

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

error: aborting due to 5 previous errors
error[E0798]: return value of `"C-cmse-nonsecure-call"` function too large to pass via registers
--> $DIR/generics.rs:27:73
|
LL | type WithTraitObject = extern "C-cmse-nonsecure-call" fn(&dyn Trait) -> &dyn Trait;
| ^^^^^^^^^^ 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/generics.rs:31:62
|
LL | extern "C-cmse-nonsecure-call" fn(&'static dyn Trait) -> &'static dyn Trait;
| ^^^^^^^^^^^^^^^^^^ 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/generics.rs:38:62
|
LL | extern "C-cmse-nonsecure-call" fn(WrapperTransparent) -> WrapperTransparent;
| ^^^^^^^^^^^^^^^^^^ 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[E0045]: C-variadic function must have a compatible calling convention, like `C` or `cdecl`
--> $DIR/generics.rs:41:20
|
LL | type WithVarArgs = extern "C-cmse-nonsecure-call" fn(u32, ...);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ C-variadic function must have a compatible calling convention

error: aborting due to 9 previous errors

Some errors have detailed explanations: E0412, E0562, E0798.
For more information about an error, try `rustc --explain E0412`.
Some errors have detailed explanations: E0045, E0412, E0562, E0798.
For more information about an error, try `rustc --explain E0045`.
Loading