Skip to content

librustc: Forbid destructors from accessing non-Owned fields. #5460

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

Merged
merged 1 commit into from
Mar 22, 2013
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions src/libcore/condition.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ struct Guard<T, U> {
cond: &'self Condition/&self<T, U>
}

#[unsafe_destructor]
impl<T, U> Drop for Guard/&self<T, U> {
fn finalize(&self) {
unsafe {
Expand Down
14 changes: 8 additions & 6 deletions src/libcore/io.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1230,15 +1230,17 @@ pub mod fsync {
arg: Arg<t>,
}

#[unsafe_destructor]
impl<T:Copy> Drop for Res<T> {
fn finalize(&self) {
match self.arg.opt_level {
None => (),
Some(level) => {
// fail hard if not succesful
fail_unless!(((self.arg.fsync_fn)(self.arg.val, level) != -1));
match self.arg.opt_level {
None => (),
Some(level) => {
// fail hard if not succesful
fail_unless!(((self.arg.fsync_fn)(self.arg.val, level)
!= -1));
}
}
}
}
}

Expand Down
1 change: 1 addition & 0 deletions src/libcore/option.rs
Original file line number Diff line number Diff line change
Expand Up @@ -514,6 +514,7 @@ fn test_unwrap_resource() {
i: @mut int,
}

#[unsafe_destructor]
impl ::ops::Drop for R {
fn finalize(&self) { *(self.i) += 1; }
}
Expand Down
16 changes: 10 additions & 6 deletions src/libcore/pipes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,7 @@ struct BufferResource<T> {

}

#[unsafe_destructor]
impl<T> ::ops::Drop for BufferResource<T> {
fn finalize(&self) {
unsafe {
Expand Down Expand Up @@ -445,16 +446,17 @@ pub fn try_recv<T:Owned,Tbuffer:Owned>(p: RecvPacketBuffered<T, Tbuffer>)
let p_ = p.unwrap();
let p = unsafe { &*p_ };

#[unsafe_destructor]
struct DropState {
p: &'self PacketHeader,

drop {
if task::failing() {
self.p.state = Terminated;
let old_task = swap_task(&mut self.p.blocked_task,
ptr::null());
if !old_task.is_null() {
unsafe {
unsafe {
if task::failing() {
self.p.state = Terminated;
let old_task = swap_task(&mut self.p.blocked_task,
ptr::null());
if !old_task.is_null() {
rustrt::rust_task_deref(old_task);
}
}
Expand Down Expand Up @@ -773,6 +775,7 @@ pub struct SendPacketBuffered<T, Tbuffer> {
mut buffer: Option<BufferResource<Tbuffer>>,
}

#[unsafe_destructor]
impl<T:Owned,Tbuffer:Owned> ::ops::Drop for SendPacketBuffered<T,Tbuffer> {
fn finalize(&self) {
//if self.p != none {
Expand Down Expand Up @@ -842,6 +845,7 @@ pub struct RecvPacketBuffered<T, Tbuffer> {
mut buffer: Option<BufferResource<Tbuffer>>,
}

#[unsafe_destructor]
impl<T:Owned,Tbuffer:Owned> ::ops::Drop for RecvPacketBuffered<T,Tbuffer> {
fn finalize(&self) {
//if self.p != none {
Expand Down
1 change: 1 addition & 0 deletions src/libcore/unstable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ struct ArcDestruct<T> {
mut data: *libc::c_void,
}

#[unsafe_destructor]
impl<T> Drop for ArcDestruct<T>{
fn finalize(&self) {
unsafe {
Expand Down
1 change: 1 addition & 0 deletions src/libcore/unstable/finally.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ struct Finallyalizer {
dtor: &'self fn()
}

#[unsafe_destructor]
impl Drop for Finallyalizer/&self {
fn finalize(&self) {
(self.dtor)();
Expand Down
99 changes: 94 additions & 5 deletions src/librustc/middle/kind.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ use core::str;
use core::vec;
use std::oldmap::HashMap;
use syntax::ast::*;
use syntax::attr::attrs_contains_name;
use syntax::codemap::{span, spanned};
use syntax::print::pprust::expr_to_str;
use syntax::{visit, ast_util};
Expand Down Expand Up @@ -55,6 +56,8 @@ use syntax::{visit, ast_util};
// primitives in the stdlib are explicitly annotated to only take sendable
// types.

use core::hashmap::linear::LinearSet;

pub const try_adding: &'static str = "Try adding a move";

pub type rval_map = HashMap<node_id, ()>;
Expand All @@ -63,7 +66,7 @@ pub struct Context {
tcx: ty::ctxt,
method_map: typeck::method_map,
last_use_map: liveness::last_use_map,
current_item: node_id
current_item: node_id,
}

pub fn check_crate(tcx: ty::ctxt,
Expand All @@ -74,16 +77,15 @@ pub fn check_crate(tcx: ty::ctxt,
tcx: tcx,
method_map: method_map,
last_use_map: last_use_map,
current_item: -1
current_item: -1,
};
let visit = visit::mk_vt(@visit::Visitor {
visit_arm: check_arm,
visit_expr: check_expr,
visit_fn: check_fn,
visit_ty: check_ty,
visit_item: |i, cx, v| {
visit::visit_item(i, Context { current_item: i.id,.. cx }, v);
},
visit_item: check_item,
visit_block: check_block,
.. *visit::default_visitor()
});
visit::visit_crate(*crate, ctx, visit);
Expand All @@ -92,6 +94,93 @@ pub fn check_crate(tcx: ty::ctxt,

type check_fn = @fn(Context, @freevar_entry);

fn check_struct_safe_for_destructor(cx: Context,
span: span,
struct_did: def_id) {
let struct_tpt = ty::lookup_item_type(cx.tcx, struct_did);
if struct_tpt.bounds.len() == 0 {
let struct_ty = ty::mk_struct(cx.tcx, struct_did, ty::substs {
self_r: None,
self_ty: None,
tps: ~[]
});
if !ty::type_is_owned(cx.tcx, struct_ty) {
cx.tcx.sess.span_err(span,
~"cannot implement a destructor on a struct \
that is not Owned");
cx.tcx.sess.span_note(span,
~"use \"#[unsafe_destructor]\" on the \
implementation to force the compiler to \
allow this");
}
} else {
cx.tcx.sess.span_err(span,
~"cannot implement a destructor on a struct \
with type parameters");
cx.tcx.sess.span_note(span,
~"use \"#[unsafe_destructor]\" on the \
implementation to force the compiler to \
allow this");
}
}

fn check_block(block: &blk, cx: Context, visitor: visit::vt<Context>) {
visit::visit_block(block, cx, visitor);
}

fn check_item(item: @item, cx: Context, visitor: visit::vt<Context>) {
// If this is a destructor, check kinds.
if !attrs_contains_name(item.attrs, "unsafe_destructor") {
match item.node {
item_impl(_, Some(trait_ref), self_type, _) => {
match cx.tcx.def_map.find(&trait_ref.ref_id) {
None => cx.tcx.sess.bug(~"trait ref not in def map!"),
Some(trait_def) => {
let trait_def_id = ast_util::def_id_of_def(trait_def);
if cx.tcx.lang_items.drop_trait() == trait_def_id {
// Yes, it's a destructor.
match self_type.node {
ty_path(_, path_node_id) => {
let struct_def = cx.tcx.def_map.get(
&path_node_id);
let struct_did =
ast_util::def_id_of_def(struct_def);
check_struct_safe_for_destructor(
cx,
self_type.span,
struct_did);
}
_ => {
cx.tcx.sess.span_bug(self_type.span,
~"the self type for \
the Drop trait \
impl is not a \
path");
}
}
}
}
}
}
item_struct(struct_def, _) => {
match struct_def.dtor {
None => {}
Some(ref dtor) => {
let struct_did = def_id { crate: 0, node: item.id };
check_struct_safe_for_destructor(cx,
dtor.span,
struct_did);
}
}
}
_ => {}
}
}

let cx = Context { current_item: item.id, ..cx };
visit::visit_item(item, cx, visitor);
}

// Yields the appropriate function to check the kind of closed over
// variables. `id` is the node_id for some expression that creates the
// closure.
Expand Down
9 changes: 6 additions & 3 deletions src/librustc/middle/trans/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,11 +88,14 @@ pub struct icx_popper {
ccx: @CrateContext,
}

#[unsafe_destructor]
impl Drop for icx_popper {
fn finalize(&self) {
if self.ccx.sess.count_llvm_insns() {
self.ccx.stats.llvm_insn_ctxt.pop();
}
unsafe {
if self.ccx.sess.count_llvm_insns() {
self.ccx.stats.llvm_insn_ctxt.pop();
}
}
}
}

Expand Down
1 change: 1 addition & 0 deletions src/libstd/arena.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ pub struct Arena {
priv mut chunks: @List<Chunk>,
}

#[unsafe_destructor]
impl Drop for Arena {
fn finalize(&self) {
unsafe {
Expand Down
9 changes: 6 additions & 3 deletions src/libstd/c_vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,14 @@ struct DtorRes {
dtor: Option<@fn()>,
}

#[unsafe_destructor]
impl Drop for DtorRes {
fn finalize(&self) {
match self.dtor {
option::None => (),
option::Some(f) => f()
unsafe {
match self.dtor {
option::None => (),
option::Some(f) => f()
}
}
}
}
Expand Down
1 change: 1 addition & 0 deletions src/libstd/future.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ pub struct Future<A> {

// FIXME(#2829) -- futures should not be copyable, because they close
// over ~fn's that have pipes and so forth within!
#[unsafe_destructor]
impl<A> Drop for Future<A> {
fn finalize(&self) {}
}
Expand Down
1 change: 1 addition & 0 deletions src/libstd/net_tcp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ pub struct TcpSocket {
socket_data: @TcpSocketData,
}

#[unsafe_destructor]
impl Drop for TcpSocket {
fn finalize(&self) {
unsafe {
Expand Down
1 change: 1 addition & 0 deletions src/libstd/sort.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1190,6 +1190,7 @@ mod big_tests {
key: &'self fn(@uint),
}

#[unsafe_destructor]
impl Drop for LVal/&self {
fn finalize(&self) {
let x = unsafe { task::local_data::local_data_get(self.key) };
Expand Down
12 changes: 11 additions & 1 deletion src/libstd/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,9 +167,12 @@ type SemRelease = SemReleaseGeneric<'self, ()>;
type SemAndSignalRelease = SemReleaseGeneric<'self, ~[Waitqueue]>;
struct SemReleaseGeneric<Q> { sem: &'self Sem<Q> }

#[unsafe_destructor]
impl<Q:Owned> Drop for SemReleaseGeneric/&self<Q> {
fn finalize(&self) {
self.sem.release();
unsafe {
self.sem.release();
}
}
}

Expand All @@ -189,6 +192,7 @@ fn SemAndSignalRelease(sem: &'r Sem<~[Waitqueue]>)
/// A mechanism for atomic-unlock-and-deschedule blocking and signalling.
pub struct Condvar { priv sem: &'self Sem<~[Waitqueue]> }

#[unsafe_destructor]
impl Drop for Condvar/&self { fn finalize(&self) {} }

pub impl Condvar/&self {
Expand Down Expand Up @@ -261,6 +265,7 @@ pub impl Condvar/&self {
sem: &'self Sem<~[Waitqueue]>,
}

#[unsafe_destructor]
impl Drop for SemAndSignalReacquire/&self {
fn finalize(&self) {
unsafe {
Expand Down Expand Up @@ -613,6 +618,7 @@ struct RWlockReleaseRead {
lock: &'self RWlock,
}

#[unsafe_destructor]
impl Drop for RWlockReleaseRead/&self {
fn finalize(&self) {
unsafe {
Expand Down Expand Up @@ -643,10 +649,12 @@ fn RWlockReleaseRead(lock: &'r RWlock) -> RWlockReleaseRead/&r {

// FIXME(#3588) should go inside of downgrade()
#[doc(hidden)]
#[unsafe_destructor]
struct RWlockReleaseDowngrade {
lock: &'self RWlock,
}

#[unsafe_destructor]
impl Drop for RWlockReleaseDowngrade/&self {
fn finalize(&self) {
unsafe {
Expand Down Expand Up @@ -685,10 +693,12 @@ fn RWlockReleaseDowngrade(lock: &'r RWlock) -> RWlockReleaseDowngrade/&r {

/// The "write permission" token used for rwlock.write_downgrade().
pub struct RWlockWriteMode { priv lock: &'self RWlock }
#[unsafe_destructor]
impl Drop for RWlockWriteMode/&self { fn finalize(&self) {} }

/// The "read permission" token used for rwlock.write_downgrade().
pub struct RWlockReadMode { priv lock: &'self RWlock }
#[unsafe_destructor]
impl Drop for RWlockReadMode/&self { fn finalize(&self) {} }

pub impl RWlockWriteMode/&self {
Expand Down
1 change: 1 addition & 0 deletions src/libstd/task_pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ pub struct TaskPool<T> {

}

#[unsafe_destructor]
impl<T> Drop for TaskPool<T> {
fn finalize(&self) {
for self.channels.each |channel| {
Expand Down
1 change: 1 addition & 0 deletions src/libsyntax/parse/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,7 @@ pub struct Parser {

}

#[unsafe_destructor]
impl Drop for Parser {
/* do not copy the parser; its state is tied to outside state */
fn finalize(&self) {}
Expand Down
1 change: 1 addition & 0 deletions src/test/auxiliary/issue-2526.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ struct arc_destruct<T> {
_data: int,
}

#[unsafe_destructor]
impl<T:Const> Drop for arc_destruct<T> {
fn finalize(&self) {}
}
Expand Down
Loading