Skip to content

Warn about unsafe blocks in unsafe functions #6038

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

Closed
wants to merge 3 commits into from
Closed
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
102 changes: 50 additions & 52 deletions src/libcore/gc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -231,66 +231,64 @@ unsafe fn walk_gc_roots(mem: Memory, sentinel: **Word, visitor: Visitor) {
// the stack.
let mut reached_sentinel = ptr::is_null(sentinel);
for stackwalk::walk_stack |frame| {
unsafe {
let pc = last_ret;
let Segment {segment: next_segment, boundary: boundary} =
find_segment_for_frame(frame.fp, segment);
segment = next_segment;
// Each stack segment is bounded by a morestack frame. The
// morestack frame includes two return addresses, one for
// morestack itself, at the normal offset from the frame
// pointer, and then a second return address for the
// function prologue (which called morestack after
// determining that it had hit the end of the stack).
// Since morestack itself takes two parameters, the offset
// for this second return address is 3 greater than the
// return address for morestack.
let ret_offset = if boundary { 4 } else { 1 };
last_ret = *ptr::offset(frame.fp, ret_offset) as *Word;

if ptr::is_null(pc) {
loop;
}
let pc = last_ret;
let Segment {segment: next_segment, boundary: boundary} =
find_segment_for_frame(frame.fp, segment);
segment = next_segment;
// Each stack segment is bounded by a morestack frame. The
// morestack frame includes two return addresses, one for
// morestack itself, at the normal offset from the frame
// pointer, and then a second return address for the
// function prologue (which called morestack after
// determining that it had hit the end of the stack).
// Since morestack itself takes two parameters, the offset
// for this second return address is 3 greater than the
// return address for morestack.
let ret_offset = if boundary { 4 } else { 1 };
last_ret = *ptr::offset(frame.fp, ret_offset) as *Word;

if ptr::is_null(pc) {
loop;
}

let mut delay_reached_sentinel = reached_sentinel;
let sp = is_safe_point(pc);
match sp {
Some(sp_info) => {
for walk_safe_point(frame.fp, sp_info) |root, tydesc| {
// Skip roots until we see the sentinel.
if !reached_sentinel {
if root == sentinel {
delay_reached_sentinel = true;
}
loop;
let mut delay_reached_sentinel = reached_sentinel;
let sp = is_safe_point(pc);
match sp {
Some(sp_info) => {
for walk_safe_point(frame.fp, sp_info) |root, tydesc| {
// Skip roots until we see the sentinel.
if !reached_sentinel {
if root == sentinel {
delay_reached_sentinel = true;
}
loop;
}

// Skip null pointers, which can occur when a
// unique pointer has already been freed.
if ptr::is_null(*root) {
loop;
}
// Skip null pointers, which can occur when a
// unique pointer has already been freed.
if ptr::is_null(*root) {
loop;
}

if ptr::is_null(tydesc) {
// Root is a generic box.
let refcount = **root;
if mem | task_local_heap != 0 && refcount != -1 {
if !visitor(root, tydesc) { return; }
} else if mem | exchange_heap != 0 && refcount == -1 {
if !visitor(root, tydesc) { return; }
}
} else {
// Root is a non-immediate.
if mem | stack != 0 {
if !visitor(root, tydesc) { return; }
}
if ptr::is_null(tydesc) {
// Root is a generic box.
let refcount = **root;
if mem | task_local_heap != 0 && refcount != -1 {
if !visitor(root, tydesc) { return; }
} else if mem | exchange_heap != 0 && refcount == -1 {
if !visitor(root, tydesc) { return; }
}
} else {
// Root is a non-immediate.
if mem | stack != 0 {
if !visitor(root, tydesc) { return; }
}
}
}
None => ()
}
reached_sentinel = delay_reached_sentinel;
}
None => ()
}
reached_sentinel = delay_reached_sentinel;
}
}

Expand Down
4 changes: 1 addition & 3 deletions src/libcore/pipes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,9 +156,7 @@ pub impl PacketHeader {
unsafe fn unblock(&self) {
let old_task = swap_task(&mut self.blocked_task, ptr::null());
if !old_task.is_null() {
unsafe {
rustrt::rust_task_deref(old_task)
}
rustrt::rust_task_deref(old_task)
}
match swap_state_acq(&mut self.state, Empty) {
Empty | Blocked => (),
Expand Down
6 changes: 2 additions & 4 deletions src/libcore/rt/sched/local_sched.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,8 @@ pub unsafe fn unsafe_borrow() -> &mut Scheduler {
}

pub unsafe fn unsafe_borrow_io() -> &mut IoFactoryObject {
unsafe {
let sched = unsafe_borrow();
return sched.event_loop.io().unwrap();
}
let sched = unsafe_borrow();
return sched.event_loop.io().unwrap();
}

fn tls_key() -> tls::Key {
Expand Down
3 changes: 2 additions & 1 deletion src/libcore/rt/uvio.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,15 @@
use option::*;
use result::*;

use super::io::net::ip::{IpAddr, Ipv4}; // n.b. Ipv4 is used only in tests
use super::io::net::ip::IpAddr;
use super::uv::*;
use super::rtio::*;
use ops::Drop;
use cell::{Cell, empty_cell};
use cast::transmute;
use super::sched::{Scheduler, local_sched};

#[cfg(test)] use super::io::net::ip::Ipv4;
#[cfg(test)] use super::sched::Task;
#[cfg(test)] use unstable::run_in_bare_thread;
#[cfg(test)] use uint;
Expand Down
4 changes: 2 additions & 2 deletions src/libcore/rt/uvll.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ pub enum uv_req_type {

pub unsafe fn malloc_handle(handle: uv_handle_type) -> *c_void {
assert!(handle != UV_UNKNOWN_HANDLE && handle != UV_HANDLE_TYPE_MAX);
let size = unsafe { rust_uv_handle_size(handle as uint) };
let size = rust_uv_handle_size(handle as uint);
let p = malloc(size);
assert!(p.is_not_null());
return p;
Expand All @@ -110,7 +110,7 @@ pub unsafe fn free_handle(v: *c_void) {

pub unsafe fn malloc_req(req: uv_req_type) -> *c_void {
assert!(req != UV_UNKNOWN_REQ && req != UV_REQ_TYPE_MAX);
let size = unsafe { rust_uv_req_size(req as uint) };
let size = rust_uv_req_size(req as uint);
let p = malloc(size);
assert!(p.is_not_null());
return p;
Expand Down
1 change: 1 addition & 0 deletions src/libcore/str/ascii.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,7 @@ impl ToStrConsume for ~[Ascii] {
}
}

#[cfg(test)]
mod tests {
use super::*;

Expand Down
20 changes: 9 additions & 11 deletions src/libcore/unstable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -262,18 +262,16 @@ pub impl<T:Owned> Exclusive<T> {
// the exclusive. Supporting that is a work in progress.
#[inline(always)]
unsafe fn with<U>(&self, f: &fn(x: &mut T) -> U) -> U {
unsafe {
let rec = get_shared_mutable_state(&self.x);
do (*rec).lock.lock {
if (*rec).failed {
fail!(
~"Poisoned exclusive - another task failed inside!");
}
(*rec).failed = true;
let result = f(&mut (*rec).data);
(*rec).failed = false;
result
let rec = get_shared_mutable_state(&self.x);
do (*rec).lock.lock {
if (*rec).failed {
fail!(
~"Poisoned exclusive - another task failed inside!");
}
(*rec).failed = true;
let result = f(&mut (*rec).data);
(*rec).failed = false;
result
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/libcore/unstable/weak_task.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,11 @@ pub unsafe fn weaken_task(f: &fn(Port<ShutdownMsg>)) {
let task = get_task_id();
// Expect the weak task service to be alive
assert!(service.try_send(RegisterWeakTask(task, shutdown_chan)));
unsafe { rust_dec_kernel_live_count(); }
rust_dec_kernel_live_count();
do (|| {
f(shutdown_port.take())
}).finally || {
unsafe { rust_inc_kernel_live_count(); }
rust_inc_kernel_live_count();
// Service my have already exited
service.send(UnregisterWeakTask(task));
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/back/link.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use core::char;
use core::hash::Streaming;
use core::hash;
use core::io::WriterUtil;
use core::libc::{c_int, c_uint, c_char};
use core::libc::{c_int, c_uint};
use core::os::consts::{macos, freebsd, linux, android, win32};
use core::os;
use core::ptr;
Expand Down
25 changes: 4 additions & 21 deletions src/librustc/middle/borrowck/check_loans.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
use core::prelude::*;

use middle::moves;
use middle::typeck::check::PurityState;
use middle::borrowck::{Loan, bckerr, BorrowckCtxt, inherent_mutability};
use middle::borrowck::{ReqMaps, root_map_key, save_and_restore_managed};
use middle::borrowck::{MoveError, MoveOk, MoveFromIllegalCmt};
Expand All @@ -41,11 +42,6 @@ use syntax::codemap::span;
use syntax::print::pprust;
use syntax::visit;

struct PurityState {
def: ast::node_id,
purity: ast::purity
}

struct CheckLoanCtxt {
bccx: @BorrowckCtxt,
req_maps: ReqMaps,
Expand Down Expand Up @@ -85,8 +81,7 @@ pub fn check_loans(bccx: @BorrowckCtxt,
bccx: bccx,
req_maps: req_maps,
reported: HashSet::new(),
declared_purity: @mut PurityState { purity: ast::impure_fn,
def: 0 },
declared_purity: @mut PurityState::function(ast::impure_fn, 0),
fn_args: @mut @~[]
};
let vt = visit::mk_vt(@visit::Visitor {visit_expr: check_loans_in_expr,
Expand Down Expand Up @@ -658,9 +653,7 @@ fn check_loans_in_fn(fk: &visit::fn_kind,
debug!("purity on entry=%?", copy self.declared_purity);
do save_and_restore_managed(self.declared_purity) {
do save_and_restore_managed(self.fn_args) {
self.declared_purity = @mut PurityState {
purity: declared_purity, def: src
};
self.declared_purity = @mut PurityState::function(declared_purity, src);

match *fk {
visit::fk_anon(*) |
Expand Down Expand Up @@ -810,17 +803,7 @@ fn check_loans_in_block(blk: &ast::blk,
do save_and_restore_managed(self.declared_purity) {
self.check_for_conflicting_loans(blk.node.id);

match blk.node.rules {
ast::default_blk => {
}
ast::unsafe_blk => {
*self.declared_purity = PurityState {
purity: ast::unsafe_fn,
def: blk.node.id,
};
}
}

*self.declared_purity = self.declared_purity.recurse(blk);
visit::visit_block(blk, self, vt);
}
}
Expand Down
8 changes: 3 additions & 5 deletions src/librustc/middle/trans/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ use core::hash;
use core::hashmap::{HashMap, HashSet};
use core::int;
use core::io;
use core::libc::{c_uint, c_ulonglong};
use core::libc::c_uint;
use core::uint;
use std::time;
use syntax::ast::ident;
Expand Down Expand Up @@ -2628,13 +2628,11 @@ pub fn get_item_val(ccx: @CrateContext, id: ast::node_id) -> ValueRef {
let class_ty = ty::lookup_item_type(tcx, parent_id).ty;
// This code shouldn't be reached if the class is generic
assert!(!ty::type_has_params(class_ty));
let lldty = unsafe {
T_fn(~[
let lldty = T_fn(~[
T_ptr(T_i8()),
T_ptr(type_of(ccx, class_ty))
],
T_nil())
};
T_nil());
let s = get_dtor_symbol(ccx, /*bad*/copy *pt, dt.node.id, None);

/* Make the declaration for the dtor */
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/middle/ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -514,7 +514,7 @@ pub struct substs {
}

mod primitives {
use super::{sty, t_box_};
use super::t_box_;

use syntax::ast;

Expand Down
Loading