Skip to content

Commit

Permalink
Auto merge of #99420 - RalfJung:vtable, r=oli-obk
Browse files Browse the repository at this point in the history
make vtable pointers entirely opaque

This implements the scheme discussed in rust-lang/unsafe-code-guidelines#338: vtable pointers should be considered entirely opaque and not even readable by Rust code, similar to function pointers.

- We have a new kind of `GlobalAlloc` that symbolically refers to a vtable.
- Miri uses that kind of allocation when generating a vtable.
- The codegen backends, upon encountering such an allocation, call `vtable_allocation` to obtain an actually dataful allocation for this vtable.
- We need new intrinsics to obtain the size and align from a vtable (for some `ptr::metadata` APIs), since direct accesses are UB now.

I had to touch quite a bit of code that I am not very familiar with, so some of this might not make much sense...
r? `@oli-obk`
  • Loading branch information
bors committed Jul 22, 2022
2 parents 31b9b01 + d46dfa2 commit aa01891
Show file tree
Hide file tree
Showing 69 changed files with 673 additions and 527 deletions.
37 changes: 27 additions & 10 deletions compiler/rustc_codegen_cranelift/src/constant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -195,9 +195,8 @@ pub(crate) fn codegen_const_value<'tcx>(
}
Scalar::Ptr(ptr, _size) => {
let (alloc_id, offset) = ptr.into_parts(); // we know the `offset` is relative
let alloc_kind = fx.tcx.get_global_alloc(alloc_id);
let base_addr = match alloc_kind {
Some(GlobalAlloc::Memory(alloc)) => {
let base_addr = match fx.tcx.global_alloc(alloc_id) {
GlobalAlloc::Memory(alloc) => {
let data_id = data_id_for_alloc_id(
&mut fx.constants_cx,
fx.module,
Expand All @@ -211,13 +210,27 @@ pub(crate) fn codegen_const_value<'tcx>(
}
fx.bcx.ins().global_value(fx.pointer_type, local_data_id)
}
Some(GlobalAlloc::Function(instance)) => {
GlobalAlloc::Function(instance) => {
let func_id = crate::abi::import_function(fx.tcx, fx.module, instance);
let local_func_id =
fx.module.declare_func_in_func(func_id, &mut fx.bcx.func);
fx.bcx.ins().func_addr(fx.pointer_type, local_func_id)
}
Some(GlobalAlloc::Static(def_id)) => {
GlobalAlloc::VTable(ty, trait_ref) => {
let alloc_id = fx.tcx.vtable_allocation((ty, trait_ref));
let alloc = fx.tcx.global_alloc(alloc_id).unwrap_memory();
// FIXME: factor this common code with the `Memory` arm into a function?
let data_id = data_id_for_alloc_id(
&mut fx.constants_cx,
fx.module,
alloc_id,
alloc.inner().mutability,
);
let local_data_id =
fx.module.declare_data_in_func(data_id, &mut fx.bcx.func);
fx.bcx.ins().global_value(fx.pointer_type, local_data_id)
}
GlobalAlloc::Static(def_id) => {
assert!(fx.tcx.is_static(def_id));
let data_id = data_id_for_static(fx.tcx, fx.module, def_id, false);
let local_data_id =
Expand All @@ -227,7 +240,6 @@ pub(crate) fn codegen_const_value<'tcx>(
}
fx.bcx.ins().global_value(fx.pointer_type, local_data_id)
}
None => bug!("missing allocation {:?}", alloc_id),
};
let val = if offset.bytes() != 0 {
fx.bcx.ins().iadd_imm(base_addr, i64::try_from(offset.bytes()).unwrap())
Expand Down Expand Up @@ -357,10 +369,11 @@ fn define_all_allocs(tcx: TyCtxt<'_>, module: &mut dyn Module, cx: &mut Constant
while let Some(todo_item) = cx.todo.pop() {
let (data_id, alloc, section_name) = match todo_item {
TodoItem::Alloc(alloc_id) => {
//println!("alloc_id {}", alloc_id);
let alloc = match tcx.get_global_alloc(alloc_id).unwrap() {
let alloc = match tcx.global_alloc(alloc_id) {
GlobalAlloc::Memory(alloc) => alloc,
GlobalAlloc::Function(_) | GlobalAlloc::Static(_) => unreachable!(),
GlobalAlloc::Function(_) | GlobalAlloc::Static(_) | GlobalAlloc::VTable(..) => {
unreachable!()
}
};
let data_id = *cx.anon_allocs.entry(alloc_id).or_insert_with(|| {
module
Expand Down Expand Up @@ -424,7 +437,7 @@ fn define_all_allocs(tcx: TyCtxt<'_>, module: &mut dyn Module, cx: &mut Constant
read_target_uint(endianness, bytes).unwrap()
};

let reloc_target_alloc = tcx.get_global_alloc(alloc_id).unwrap();
let reloc_target_alloc = tcx.global_alloc(alloc_id);
let data_id = match reloc_target_alloc {
GlobalAlloc::Function(instance) => {
assert_eq!(addend, 0);
Expand All @@ -436,6 +449,10 @@ fn define_all_allocs(tcx: TyCtxt<'_>, module: &mut dyn Module, cx: &mut Constant
GlobalAlloc::Memory(target_alloc) => {
data_id_for_alloc_id(cx, module, alloc_id, target_alloc.inner().mutability)
}
GlobalAlloc::VTable(ty, trait_ref) => {
let alloc_id = tcx.vtable_allocation((ty, trait_ref));
data_id_for_alloc_id(cx, module, alloc_id, Mutability::Not)
}
GlobalAlloc::Static(def_id) => {
if tcx.codegen_fn_attrs(def_id).flags.contains(CodegenFnAttrFlags::THREAD_LOCAL)
{
Expand Down
10 changes: 10 additions & 0 deletions compiler/rustc_codegen_cranelift/src/intrinsics/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -431,6 +431,16 @@ fn codegen_regular_intrinsic_call<'tcx>(
ret.write_cvalue(fx, CValue::by_val(align, usize_layout));
};

vtable_size, (v vtable) {
let size = crate::vtable::size_of_obj(fx, vtable);
ret.write_cvalue(fx, CValue::by_val(size, usize_layout));
};

vtable_align, (v vtable) {
let align = crate::vtable::min_align_of_obj(fx, vtable);
ret.write_cvalue(fx, CValue::by_val(align, usize_layout));
};

unchecked_add | unchecked_sub | unchecked_mul | unchecked_div | exact_div | unchecked_rem
| unchecked_shl | unchecked_shr, (c x, c y) {
// FIXME trap on overflow
Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_codegen_gcc/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,11 @@ impl<'gcc, 'tcx> ConstMethods<'tcx> for CodegenCx<'gcc, 'tcx> {
GlobalAlloc::Function(fn_instance) => {
self.get_fn_addr(fn_instance)
},
GlobalAlloc::VTable(ty, trait_ref) => {
let alloc = self.tcx.global_alloc(self.tcx.vtable_allocation((ty, trait_ref))).unwrap_memory();
let init = const_alloc_to_gcc(self, alloc);
self.static_addr_of(init, alloc.inner().align, None)
}
GlobalAlloc::Static(def_id) => {
assert!(self.tcx.is_static(def_id));
self.get_static(def_id).get_address(None)
Expand Down
9 changes: 9 additions & 0 deletions compiler/rustc_codegen_llvm/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,15 @@ impl<'ll, 'tcx> ConstMethods<'tcx> for CodegenCx<'ll, 'tcx> {
self.get_fn_addr(fn_instance.polymorphize(self.tcx)),
self.data_layout().instruction_address_space,
),
GlobalAlloc::VTable(ty, trait_ref) => {
let alloc = self
.tcx
.global_alloc(self.tcx.vtable_allocation((ty, trait_ref)))
.unwrap_memory();
let init = const_alloc_to_llvm(self, alloc);
let value = self.static_addr_of(init, alloc.inner().align, None);
(value, AddressSpace::DATA)
}
GlobalAlloc::Static(def_id) => {
assert!(self.tcx.is_static(def_id));
assert!(!self.tcx.is_thread_local_static(def_id));
Expand Down
4 changes: 3 additions & 1 deletion compiler/rustc_codegen_llvm/src/consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,9 @@ pub fn const_alloc_to_llvm<'ll>(cx: &CodegenCx<'ll, '_>, alloc: ConstAllocation<

let address_space = match cx.tcx.global_alloc(alloc_id) {
GlobalAlloc::Function(..) => cx.data_layout().instruction_address_space,
GlobalAlloc::Static(..) | GlobalAlloc::Memory(..) => AddressSpace::DATA,
GlobalAlloc::Static(..) | GlobalAlloc::Memory(..) | GlobalAlloc::VTable(..) => {
AddressSpace::DATA
}
};

llvals.push(cx.scalar_to_backend(
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1420,7 +1420,7 @@ fn build_vtable_type_di_node<'ll, 'tcx>(
cx,
type_map::stub(
cx,
Stub::VtableTy { vtable_holder },
Stub::VTableTy { vtable_holder },
unique_type_id,
&vtable_type_name,
(size, pointer_align),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ impl<'ll> DINodeCreationResult<'ll> {
pub enum Stub<'ll> {
Struct,
Union,
VtableTy { vtable_holder: &'ll DIType },
VTableTy { vtable_holder: &'ll DIType },
}

pub struct StubInfo<'ll, 'tcx> {
Expand Down Expand Up @@ -180,9 +180,9 @@ pub(super) fn stub<'ll, 'tcx>(
let unique_type_id_str = unique_type_id.generate_unique_id_string(cx.tcx);

let metadata = match kind {
Stub::Struct | Stub::VtableTy { .. } => {
Stub::Struct | Stub::VTableTy { .. } => {
let vtable_holder = match kind {
Stub::VtableTy { vtable_holder } => Some(vtable_holder),
Stub::VTableTy { vtable_holder } => Some(vtable_holder),
_ => None,
};
unsafe {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_ssa/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ pub fn unsized_info<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
);
let new_vptr = bx.load(ptr_ty, gep, ptr_align);
bx.nonnull_metadata(new_vptr);
// Vtable loads are invariant.
// VTable loads are invariant.
bx.set_invariant_load(new_vptr);
new_vptr
} else {
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_codegen_ssa/src/meth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ impl<'a, 'tcx> VirtualIndex {
let gep = bx.inbounds_gep(llty, llvtable, &[bx.const_usize(self.0)]);
let ptr = bx.load(llty, gep, ptr_align);
bx.nonnull_metadata(ptr);
// Vtable loads are invariant.
// VTable loads are invariant.
bx.set_invariant_load(ptr);
ptr
}
Expand All @@ -58,7 +58,7 @@ impl<'a, 'tcx> VirtualIndex {
let usize_align = bx.tcx().data_layout.pointer_align.abi;
let gep = bx.inbounds_gep(llty, llvtable, &[bx.const_usize(self.0)]);
let ptr = bx.load(llty, gep, usize_align);
// Vtable loads are invariant.
// VTable loads are invariant.
bx.set_invariant_load(ptr);
ptr
}
Expand Down
20 changes: 19 additions & 1 deletion compiler/rustc_codegen_ssa/src/mir/intrinsic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,16 @@ use super::place::PlaceRef;
use super::FunctionCx;
use crate::common::{span_invalid_monomorphization_error, IntPredicate};
use crate::glue;
use crate::meth;
use crate::traits::*;
use crate::MemFlags;

use rustc_middle::ty::{self, Ty, TyCtxt};
use rustc_span::{sym, Span};
use rustc_target::abi::call::{FnAbi, PassMode};
use rustc_target::abi::{
call::{FnAbi, PassMode},
WrappingRange,
};

fn copy_intrinsic<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
bx: &mut Bx,
Expand Down Expand Up @@ -102,6 +106,20 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
bx.const_usize(bx.layout_of(tp_ty).align.abi.bytes())
}
}
sym::vtable_size | sym::vtable_align => {
let vtable = args[0].immediate();
let idx = match name {
sym::vtable_size => ty::COMMON_VTABLE_ENTRIES_SIZE,
sym::vtable_align => ty::COMMON_VTABLE_ENTRIES_ALIGN,
_ => bug!(),
};
let value = meth::VirtualIndex::from_index(idx).get_usize(bx, vtable);
if name == sym::vtable_align {
// Alignment is always nonzero.
bx.range_metadata(value, WrappingRange { start: 1, end: !0 });
};
value
}
sym::pref_align_of
| sym::needs_drop
| sym::type_id
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_const_eval/src/const_eval/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
// we don't deallocate it.
let (alloc_id, _, _) = ecx.ptr_get_alloc_id(ptr)?;
let is_allocated_in_another_const = matches!(
ecx.tcx.get_global_alloc(alloc_id),
ecx.tcx.try_get_global_alloc(alloc_id),
Some(interpret::GlobalAlloc::Memory(_))
);

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_const_eval/src/const_eval/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ pub(crate) fn deref_mir_constant<'tcx>(
let mplace = ecx.deref_operand(&op).unwrap();
if let Some(alloc_id) = mplace.ptr.provenance {
assert_eq!(
tcx.get_global_alloc(alloc_id).unwrap().unwrap_memory().0.0.mutability,
tcx.global_alloc(alloc_id).unwrap_memory().0.0.mutability,
Mutability::Not,
"deref_mir_constant cannot be used with mutable allocations as \
that could allow pattern matching to observe mutable statics",
Expand Down
28 changes: 8 additions & 20 deletions compiler/rustc_const_eval/src/interpret/cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -298,30 +298,18 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
self.write_immediate(val, dest)
}
(&ty::Dynamic(ref data_a, ..), &ty::Dynamic(ref data_b, ..)) => {
let val = self.read_immediate(src)?;
if data_a.principal_def_id() == data_b.principal_def_id() {
return self.write_immediate(*val, dest);
}
// trait upcasting coercion
let vptr_entry_idx = self.tcx.vtable_trait_upcasting_coercion_new_vptr_slot((
src_pointee_ty,
dest_pointee_ty,
));

if let Some(entry_idx) = vptr_entry_idx {
let entry_idx = u64::try_from(entry_idx).unwrap();
let (old_data, old_vptr) = val.to_scalar_pair()?;
let old_vptr = self.scalar_to_ptr(old_vptr)?;
let new_vptr = self
.read_new_vtable_after_trait_upcasting_from_vtable(old_vptr, entry_idx)?;
self.write_immediate(Immediate::new_dyn_trait(old_data, new_vptr, self), dest)
} else {
self.write_immediate(*val, dest)
let (old_data, old_vptr) = self.read_immediate(src)?.to_scalar_pair()?;
let old_vptr = self.scalar_to_ptr(old_vptr)?;
let (ty, old_trait) = self.get_ptr_vtable(old_vptr)?;
if old_trait != data_a.principal() {
throw_ub_format!("upcast on a pointer whose vtable does not match its type");
}
let new_vptr = self.get_vtable_ptr(ty, data_b.principal())?;
self.write_immediate(Immediate::new_dyn_trait(old_data, new_vptr, self), dest)
}
(_, &ty::Dynamic(ref data, _)) => {
// Initial cast from sized to dyn trait
let vtable = self.get_vtable(src_pointee_ty, data.principal())?;
let vtable = self.get_vtable_ptr(src_pointee_ty, data.principal())?;
let ptr = self.read_immediate(src)?.to_scalar()?;
let val = Immediate::new_dyn_trait(ptr, vtable, &*self.tcx);
self.write_immediate(val, dest)
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_const_eval/src/interpret/eval_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -631,7 +631,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
ty::Dynamic(..) => {
let vtable = self.scalar_to_ptr(metadata.unwrap_meta())?;
// Read size and align from vtable (already checks size).
Ok(Some(self.read_size_and_align_from_vtable(vtable)?))
Ok(Some(self.get_vtable_size_and_align(vtable)?))
}

ty::Slice(_) | ty::Str => {
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_const_eval/src/interpret/intern.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ fn intern_shallow<'rt, 'mir, 'tcx, M: CompileTimeMachine<'mir, 'tcx, const_eval:
// to validation to error -- it has the much better error messages, pointing out where
// in the value the dangling reference lies.
// The `delay_span_bug` ensures that we don't forget such a check in validation.
if tcx.get_global_alloc(alloc_id).is_none() {
if tcx.try_get_global_alloc(alloc_id).is_none() {
tcx.sess.delay_span_bug(ecx.tcx.span, "tried to intern dangling pointer");
}
// treat dangling pointers like other statics
Expand Down Expand Up @@ -454,7 +454,7 @@ pub fn intern_const_alloc_recursive<
.sess
.span_err(ecx.tcx.span, "encountered dangling pointer in final constant");
return Err(reported);
} else if ecx.tcx.get_global_alloc(alloc_id).is_none() {
} else if ecx.tcx.try_get_global_alloc(alloc_id).is_none() {
// We have hit an `AllocId` that is neither in local or global memory and isn't
// marked as dangling by local memory. That should be impossible.
span_bug!(ecx.tcx.span, "encountered unknown alloc id {:?}", alloc_id);
Expand Down
12 changes: 12 additions & 0 deletions compiler/rustc_const_eval/src/interpret/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -492,6 +492,18 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
let result = self.raw_eq_intrinsic(&args[0], &args[1])?;
self.write_scalar(result, dest)?;
}

sym::vtable_size => {
let ptr = self.read_pointer(&args[0])?;
let (size, _align) = self.get_vtable_size_and_align(ptr)?;
self.write_scalar(Scalar::from_machine_usize(size.bytes(), self), dest)?;
}
sym::vtable_align => {
let ptr = self.read_pointer(&args[0])?;
let (_size, align) = self.get_vtable_size_and_align(ptr)?;
self.write_scalar(Scalar::from_machine_usize(align.bytes(), self), dest)?;
}

_ => return Ok(false),
}

Expand Down
Loading

0 comments on commit aa01891

Please sign in to comment.