Auto merge of #86245 - lqd:const-ub-align, r=RalfJung

Fix ICEs on invalid vtable size/alignment const UB errors

The invalid vtable size/alignment errors from `InterpCx::read_size_and_align_from_vtable` were "freeform const UB errors", causing ICEs when reaching validation. This PR turns them into const UB hard errors to catch them during validation and avoid that.

Fixes #86193

r? `@RalfJung`

(It seemed cleaner to have 2 variants but they can be merged into one variant with a message payload if you prefer that ?)
This commit is contained in:
bors 2021-06-13 12:08:59 +00:00
commit fb3ea63d9b
7 changed files with 149 additions and 38 deletions

View File

@ -227,7 +227,11 @@ pub enum UndefinedBehaviorInfo<'tcx> {
/// Invalid metadata in a wide pointer (using `str` to avoid allocations).
InvalidMeta(&'static str),
/// Invalid drop function in vtable.
InvalidDropFn(FnSig<'tcx>),
InvalidVtableDropFn(FnSig<'tcx>),
/// Invalid size in a vtable: too large.
InvalidVtableSize,
/// Invalid alignment in a vtable: too large, or not a power of 2.
InvalidVtableAlignment(String),
/// Reading a C string that does not end within its allocation.
UnterminatedCString(Pointer),
/// Dereferencing a dangling pointer after it got freed.
@ -287,11 +291,15 @@ impl fmt::Display for UndefinedBehaviorInfo<'_> {
RemainderByZero => write!(f, "calculating the remainder with a divisor of zero"),
PointerArithOverflow => write!(f, "overflowing in-bounds pointer arithmetic"),
InvalidMeta(msg) => write!(f, "invalid metadata in wide pointer: {}", msg),
InvalidDropFn(sig) => write!(
InvalidVtableDropFn(sig) => write!(
f,
"invalid drop function signature: got {}, expected exactly one argument which must be a pointer type",
sig
),
InvalidVtableSize => {
write!(f, "invalid vtable: size is bigger than largest supported object")
}
InvalidVtableAlignment(msg) => write!(f, "invalid vtable: alignment {}", msg),
UnterminatedCString(p) => write!(
f,
"reading a null-terminated string starting at {} with no null found before end of allocation",

View File

@ -137,9 +137,10 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
// The drop function takes `*mut T` where `T` is the type being dropped, so get that.
let args = fn_sig.inputs();
if args.len() != 1 {
throw_ub!(InvalidDropFn(fn_sig));
throw_ub!(InvalidVtableDropFn(fn_sig));
}
let ty = args[0].builtin_deref(true).ok_or_else(|| err_ub!(InvalidDropFn(fn_sig)))?.ty;
let ty =
args[0].builtin_deref(true).ok_or_else(|| err_ub!(InvalidVtableDropFn(fn_sig)))?.ty;
Ok((drop_instance, ty))
}
@ -158,14 +159,10 @@ 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))?;
let align = Align::from_bytes(align).map_err(|e| err_ub!(InvalidVtableAlignment(e)))?;
if size >= self.tcx.data_layout.obj_size_bound() {
throw_ub_format!(
"invalid vtable: \
size is bigger than largest supported object"
);
throw_ub!(InvalidVtableSize);
}
Ok((Size::from_bytes(size), align))
}

View File

@ -349,12 +349,16 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
err_ub!(InvalidFunctionPointer(..)) |
err_unsup!(ReadBytesAsPointer) =>
{ "invalid drop function pointer in vtable (not pointing to a function)" },
err_ub!(InvalidDropFn(..)) =>
err_ub!(InvalidVtableDropFn(..)) =>
{ "invalid drop function pointer in vtable (function has incompatible signature)" },
);
try_validation!(
self.ecx.read_size_and_align_from_vtable(vtable),
self.path,
err_ub!(InvalidVtableSize) =>
{ "invalid vtable: size is bigger than largest supported object" },
err_ub!(InvalidVtableAlignment(msg)) =>
{ "invalid vtable: alignment {}", msg },
err_unsup!(ReadPointerAsBytes) => { "invalid size or align in vtable" },
);
// FIXME: More checks for the vtable.

View File

@ -0,0 +1,52 @@
error: any use of this value will cause an error
--> $DIR/ub-incorrect-vtable.rs:19: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:25: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[E0080]: it is undefined behavior to use this value
--> $DIR/ub-incorrect-vtable.rs:36:1
|
LL | / const INVALID_VTABLE_ALIGNMENT_UB: W<&dyn Trait> =
LL | | unsafe { std::mem::transmute((&92u8, &(drop_me as fn(*mut usize), 1usize, 1000usize))) };
| |_____________________________________________________________________________________________^ type validation failed: encountered invalid vtable: alignment `1000` is not a power of 2 at .0
|
= note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
= note: the raw bytes of the constant (size: 8, align: 4) {
╾─allocN─╼ ╾─allocN─╼ │ ╾──╼╾──╼
}
error[E0080]: it is undefined behavior to use this value
--> $DIR/ub-incorrect-vtable.rs:41:1
|
LL | / const INVALID_VTABLE_SIZE_UB: W<&dyn Trait> =
LL | | unsafe { std::mem::transmute((&92u8, &(drop_me as fn(*mut usize), usize::MAX, 1usize))) };
| |______________________________________________________________________________________________^ type validation failed: encountered invalid vtable: size is bigger than largest supported object at .0
|
= note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
= note: the raw bytes of the constant (size: 8, align: 4) {
╾─allocN─╼ ╾─allocN─╼ │ ╾──╼╾──╼
}
error: aborting due to 4 previous errors
For more information about this error, try `rustc --explain E0080`.

View File

@ -0,0 +1,52 @@
error: any use of this value will cause an error
--> $DIR/ub-incorrect-vtable.rs:19: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:25: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[E0080]: it is undefined behavior to use this value
--> $DIR/ub-incorrect-vtable.rs:36:1
|
LL | / const INVALID_VTABLE_ALIGNMENT_UB: W<&dyn Trait> =
LL | | unsafe { std::mem::transmute((&92u8, &(drop_me as fn(*mut usize), 1usize, 1000usize))) };
| |_____________________________________________________________________________________________^ type validation failed: encountered invalid vtable: alignment `1000` is not a power of 2 at .0
|
= note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
= note: the raw bytes of the constant (size: 16, align: 8) {
╾───────allocN───────╼ ╾───────allocN───────╼ │ ╾──────╼╾──────╼
}
error[E0080]: it is undefined behavior to use this value
--> $DIR/ub-incorrect-vtable.rs:41:1
|
LL | / const INVALID_VTABLE_SIZE_UB: W<&dyn Trait> =
LL | | unsafe { std::mem::transmute((&92u8, &(drop_me as fn(*mut usize), usize::MAX, 1usize))) };
| |______________________________________________________________________________________________^ type validation failed: encountered invalid vtable: size is bigger than largest supported object at .0
|
= note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
= note: the raw bytes of the constant (size: 16, align: 8) {
╾───────allocN───────╼ ╾───────allocN───────╼ │ ╾──────╼╾──────╼
}
error: aborting due to 4 previous errors
For more information about this error, try `rustc --explain E0080`.

View File

@ -3,6 +3,15 @@
// 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
//
// As is, this code will only hard error when the constants are used, and the errors are emitted via
// the `#[allow]`-able `const_err` lint. However, if the transparent wrapper technique to prevent
// reborrows is used -- from `ub-wide-ptr.rs` -- these two errors reach validation and would trigger
// ICEs as tracked by #86193. So we also use the transparent wrapper to verify proper validation
// errors are emitted instead of ICEs.
// stderr-per-bitwidth
// normalize-stderr-test "alloc\d+" -> "allocN"
trait Trait {}
@ -18,4 +27,20 @@ const INVALID_VTABLE_SIZE: &dyn Trait =
//~| WARNING this was previously accepted by the compiler
//~| invalid vtable: size is bigger than largest supported object
#[repr(transparent)]
struct W<T>(T);
// The drop fn is checked before size/align are, so get ourselves a "sufficiently valid" drop fn
fn drop_me(_: *mut usize) {}
const INVALID_VTABLE_ALIGNMENT_UB: W<&dyn Trait> =
unsafe { std::mem::transmute((&92u8, &(drop_me as fn(*mut usize), 1usize, 1000usize))) };
//~^^ ERROR it is undefined behavior to use this value
//~| invalid vtable: alignment `1000` is not a power of 2
const INVALID_VTABLE_SIZE_UB: W<&dyn Trait> =
unsafe { std::mem::transmute((&92u8, &(drop_me as fn(*mut usize), usize::MAX, 1usize))) };
//~^^ ERROR it is undefined behavior to use this value
//~| invalid vtable: size is bigger than largest supported object
fn main() {}

View File

@ -1,27 +0,0 @@
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