Skip to content

Commit

Permalink
Rollup merge of rust-lang#86174 - lqd:const-ub-align, r=RalfJung
Browse files Browse the repository at this point in the history
Detect incorrect vtable alignment during const eval

This PR fixes rust-lang#86132 by detecting invalid alignment values for trait objects in the interpreter, and emitting an error about this conversion failure, to avoid the ICE.

I've noticed that the error emitted at https://github.com/rust-lang/rust/blob/a50d72158e08e02cfc051b863017bdbd2c45b637/compiler/rustc_mir/src/interpret/traits.rs#L163-L166 doesn't seem to be present in the const-ub tests, so I've tried adding a test that triggers both of these cases: one for the invalid size, and another for the invalid alignment that rust-lang#86132 tracks (I have found different magic values triggering different `Align::from_bytes` errors than the "power of 2" one, if need be).

However, when doing that, I *cannot* for the life of me figure out the correct incantation to make these 2 errors trigger with the "it is undefined behavior to use this value" message rather than the "any use of this value will cause an error" lint.

I've tried Oli's suggestions of different values, tuples and arrays, using the transparent wrapper trick from the other tests and I was only able to trigger the regular const-ub errors about the size of the vtable, or that the drop pointer was invalid. Maybe these "type validation failed" errors happen before this part of the interpreter is reached and there just needs some magic incorrect values to bypass them, I don't know.

Since this fixes an ICE, and if the constants are indeed used, these 2 tests will turn into a hard error, I thought I'd open the PR anyways. And if ``@RalfJung`` you know of a way I could manage that (if you think that these tests are worth checking that the `throw_ub_format!` does indeed create const-ub errors as we expect) I'd be grateful.

For that reason, r? ``@RalfJung`` and cc ``@oli-obk.``
  • Loading branch information
JohnTitor authored Jun 11, 2021
2 parents 8f35f8c + d449903 commit cad884f
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 1 deletion.
4 changes: 3 additions & 1 deletion compiler/rustc_mir/src/interpret/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,13 +158,15 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
let size = u64::try_from(self.force_bits(size, pointer_size)?).unwrap();
let align = vtable.read_ptr_sized(pointer_size * 2)?.check_init()?;
let align = u64::try_from(self.force_bits(align, pointer_size)?).unwrap();
let align = Align::from_bytes(align)
.map_err(|e| err_ub_format!("invalid vtable: alignment {}", e))?;

if size >= self.tcx.data_layout.obj_size_bound() {
throw_ub_format!(
"invalid vtable: \
size is bigger than largest supported object"
);
}
Ok((Size::from_bytes(size), Align::from_bytes(align).unwrap()))
Ok((Size::from_bytes(size), align))
}
}
21 changes: 21 additions & 0 deletions src/test/ui/consts/const-eval/ub-incorrect-vtable.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// This test contains code with incorrect vtables in a const context:
// - from issue 86132: a trait object with invalid alignment caused an ICE in const eval, and now
// triggers an error
// - a similar test that triggers a previously-untested const UB error: emitted close to the above
// error, it checks the correctness of the size

trait Trait {}

const INVALID_VTABLE_ALIGNMENT: &dyn Trait =
unsafe { std::mem::transmute((&92u8, &[0usize, 1usize, 1000usize])) };
//~^ ERROR any use of this value will cause an error
//~| WARNING this was previously accepted by the compiler
//~| invalid vtable: alignment `1000` is not a power of 2

const INVALID_VTABLE_SIZE: &dyn Trait =
unsafe { std::mem::transmute((&92u8, &[1usize, usize::MAX, 1usize])) };
//~^ ERROR any use of this value will cause an error
//~| WARNING this was previously accepted by the compiler
//~| invalid vtable: size is bigger than largest supported object

fn main() {}
27 changes: 27 additions & 0 deletions src/test/ui/consts/const-eval/ub-incorrect-vtable.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
error: any use of this value will cause an error
--> $DIR/ub-incorrect-vtable.rs:10:14
|
LL | / const INVALID_VTABLE_ALIGNMENT: &dyn Trait =
LL | | unsafe { std::mem::transmute((&92u8, &[0usize, 1usize, 1000usize])) };
| |______________^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^__-
| |
| invalid vtable: alignment `1000` is not a power of 2
|
= note: `#[deny(const_err)]` on by default
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #71800 <https://github.com/rust-lang/rust/issues/71800>

error: any use of this value will cause an error
--> $DIR/ub-incorrect-vtable.rs:16:14
|
LL | / const INVALID_VTABLE_SIZE: &dyn Trait =
LL | | unsafe { std::mem::transmute((&92u8, &[1usize, usize::MAX, 1usize])) };
| |______________^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^__-
| |
| invalid vtable: size is bigger than largest supported object
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #71800 <https://github.com/rust-lang/rust/issues/71800>

error: aborting due to 2 previous errors

0 comments on commit cad884f

Please sign in to comment.