Skip to content

Commit d85b4d0

Browse files
authored
analyze: initial implementation of NON_NULL static analysis (#1081)
This adds a very basic static analysis for `NON_NULL` within the current `dataflow` framework. It starts by optimistically assuming that all pointers are `NON_NULL`, and removes the permission from pointers into which a `ptr::null()` or equivalent might flow. This branch just implements the static analysis, not rewriting. I'm actually not a huge fan of this design - I think we'd probably get much better results with a path-sensitive analysis that can handle common patterns from C like `if !p.is_null() { let q = (&p).field; /* use q... */ }` by detecting null checks in the CFG. But the simple path-insensitive version is sufficient for now, and gives us somewhere to plug in the dynamic analysis results.
2 parents 2a6b735 + 7b99d25 commit d85b4d0

File tree

20 files changed

+185
-87
lines changed

20 files changed

+185
-87
lines changed

c2rust-analyze/src/analyze.rs

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -867,8 +867,11 @@ fn run(tcx: TyCtxt) {
867867
// don't want to rewrite those
868868
gacx.foreign_mentioned_tys = foreign_mentioned_tys(tcx);
869869

870-
let mut gasn =
871-
GlobalAssignment::new(gacx.num_pointers(), PermissionSet::UNIQUE, FlagSet::empty());
870+
const INITIAL_PERMS: PermissionSet =
871+
PermissionSet::union_all([PermissionSet::UNIQUE, PermissionSet::NON_NULL]);
872+
const INITIAL_FLAGS: FlagSet = FlagSet::empty();
873+
874+
let mut gasn = GlobalAssignment::new(gacx.num_pointers(), INITIAL_PERMS, INITIAL_FLAGS);
872875
for (ptr, &info) in gacx.ptr_info().iter() {
873876
if should_make_fixed(info) {
874877
gasn.flags[ptr].insert(FlagSet::FIXED);
@@ -886,14 +889,14 @@ fn run(tcx: TyCtxt) {
886889

887890
for (ptr, perms) in gacx.known_fn_ptr_perms() {
888891
let existing_perms = &mut gasn.perms[ptr];
889-
existing_perms.remove(PermissionSet::UNIQUE);
892+
existing_perms.remove(INITIAL_PERMS);
890893
assert_eq!(*existing_perms, PermissionSet::empty());
891894
*existing_perms = perms;
892895
}
893896

894897
for info in func_info.values_mut() {
895898
let num_pointers = info.acx_data.num_pointers();
896-
let mut lasn = LocalAssignment::new(num_pointers, PermissionSet::UNIQUE, FlagSet::empty());
899+
let mut lasn = LocalAssignment::new(num_pointers, INITIAL_PERMS, INITIAL_FLAGS);
897900

898901
for (ptr, &info) in info.acx_data.local_ptr_info().iter() {
899902
if should_make_fixed(info) {
@@ -990,6 +993,7 @@ fn run(tcx: TyCtxt) {
990993
if !node_info.unique {
991994
perms.remove(PermissionSet::UNIQUE);
992995
}
996+
// TODO: PermissionSet::NON_NULL
993997

994998
if perms != old_perms {
995999
let added = perms & !old_perms;

c2rust-analyze/src/borrowck/type_check.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -602,6 +602,11 @@ impl<'tcx> TypeChecker<'tcx, '_> {
602602
self.visit_operand(p)
603603
});
604604
}
605+
Callee::Null { .. } => {
606+
// Just visit the place. The null pointer returned here has no origin, so
607+
// there's no need to call `do_assign` to set up subset relations.
608+
let _pl_lty = self.visit_place(destination);
609+
}
605610
}
606611
}
607612
// TODO(spernsteiner): handle other `TerminatorKind`s

c2rust-analyze/src/context.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ impl PermissionSet {
166166

167167
/// The permissions for a (byte-)string literal.
168168
//
169-
// `.union` is used here since it's a `const fn`, unlike `BitOr::bitor`.
169+
// `union_all` is used here since it's a `const fn`, unlike `BitOr::bitor`.
170170
pub const STRING_LITERAL: Self = Self::union_all([Self::READ, Self::OFFSET_ADD]);
171171
}
172172

@@ -456,10 +456,11 @@ impl<'a, 'tcx> AnalysisCtxt<'_, 'tcx> {
456456
let ptr = lty.label;
457457
let expected_perms = PermissionSet::STRING_LITERAL;
458458
let mut actual_perms = asn.perms()[ptr];
459-
// Ignore `UNIQUE` as it gets automatically added to all permissions
460-
// and then removed later if it can't apply.
461-
// We don't care about `UNIQUE` for const refs, so just unset it here.
459+
// Ignore `UNIQUE` and `NON_NULL` as they get automatically added to all permissions
460+
// and then removed later if it can't apply. We don't care about `UNIQUE` or
461+
// `NON_NULL` for const refs, so just unset it here.
462462
actual_perms.set(PermissionSet::UNIQUE, false);
463+
actual_perms.set(PermissionSet::NON_NULL, false);
463464
assert_eq!(expected_perms, actual_perms);
464465
}
465466
}

c2rust-analyze/src/dataflow/mod.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,7 @@ impl DataflowConstraints {
4040
self.constraints.push(Constraint::AllPerms(ptr, perms));
4141
}
4242

43-
#[allow(dead_code)]
44-
fn _add_no_perms(&mut self, ptr: PointerId, perms: PermissionSet) {
43+
fn add_no_perms(&mut self, ptr: PointerId, perms: PermissionSet) {
4544
self.constraints.push(Constraint::NoPerms(ptr, perms));
4645
}
4746

@@ -86,7 +85,7 @@ impl DataflowConstraints {
8685
// Permissions that should be propagated "down": if the superset (`b`)
8786
// doesn't have it, then the subset (`a`) should have it removed.
8887
#[allow(bad_style)]
89-
let PROPAGATE_DOWN = PermissionSet::UNIQUE;
88+
let PROPAGATE_DOWN = PermissionSet::UNIQUE | PermissionSet::NON_NULL;
9089
// Permissions that should be propagated "up": if the subset (`a`) has it,
9190
// then the superset (`b`) should be given it.
9291
#[allow(bad_style)]

c2rust-analyze/src/dataflow/type_check.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,9 @@ impl<'tcx> TypeChecker<'tcx, '_> {
123123
if !op.constant().copied().map(is_null_const).unwrap_or(false) {
124124
panic!("Creating non-null pointers from exposed addresses not supported");
125125
}
126+
// The target type of the cast must not have `NON_NULL` permission.
127+
self.constraints
128+
.add_no_perms(to_lty.label, PermissionSet::NON_NULL);
126129
}
127130
CastKind::PointerExposeAddress => {
128131
// Allow, as [`CastKind::PointerFromExposedAddress`] is the dangerous one,
@@ -599,6 +602,15 @@ impl<'tcx> TypeChecker<'tcx, '_> {
599602
assert!(args.len() == 1);
600603
self.visit_operand(&args[0]);
601604
}
605+
Callee::Null { .. } => {
606+
assert!(args.len() == 0);
607+
self.visit_place(destination, Mutability::Mut);
608+
let pl_lty = self.acx.type_of(destination);
609+
// We are assigning a null pointer to `destination`, so it must not have the
610+
// `NON_NULL` flag.
611+
self.constraints
612+
.add_no_perms(pl_lty.label, PermissionSet::NON_NULL);
613+
}
602614
}
603615
}
604616

c2rust-analyze/src/pointee_type/type_check.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -332,6 +332,9 @@ impl<'tcx> TypeChecker<'tcx, '_> {
332332
Callee::IsNull => {
333333
// No constraints.
334334
}
335+
Callee::Null { .. } => {
336+
// No constraints.
337+
}
335338
}
336339
}
337340
}

c2rust-analyze/src/util.rs

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,9 @@ pub enum Callee<'tcx> {
191191
/// core::ptr::is_null
192192
IsNull,
193193

194+
/// core::ptr::null or core::ptr::null_mut
195+
Null { mutbl: Mutability },
196+
194197
/// `core::mem::size_of<T>`
195198
SizeOf { ty: Ty<'tcx> },
196199
}
@@ -342,6 +345,30 @@ fn builtin_callee<'tcx>(tcx: TyCtxt<'tcx>, did: DefId, substs: SubstsRef<'tcx>)
342345
Some(Callee::IsNull)
343346
}
344347

348+
"null" | "null_mut" => {
349+
// The `core::ptr::null/null_mut` function.
350+
let parent_did = tcx.parent(did);
351+
if tcx.def_kind(parent_did) != DefKind::Mod {
352+
return None;
353+
}
354+
if tcx.item_name(parent_did).as_str() != "ptr" {
355+
return None;
356+
}
357+
let grandparent_did = tcx.parent(parent_did);
358+
if grandparent_did.index != CRATE_DEF_INDEX {
359+
return None;
360+
}
361+
if tcx.crate_name(grandparent_did.krate).as_str() != "core" {
362+
return None;
363+
}
364+
let mutbl = match name.as_str() {
365+
"null" => Mutability::Not,
366+
"null_mut" => Mutability::Mut,
367+
_ => unreachable!(),
368+
};
369+
Some(Callee::Null { mutbl })
370+
}
371+
345372
"size_of" => {
346373
// The `core::mem::size_of` function.
347374
let parent_did = tcx.parent(did);

c2rust-analyze/tests/filecheck.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ define_tests! {
5555
insertion_sort_driver,
5656
insertion_sort_rewrites,
5757
known_fn,
58+
non_null,
5859
offset1,
5960
offset2,
6061
pointee,

c2rust-analyze/tests/filecheck/alias1.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,23 +2,23 @@ use std::ptr;
22

33
// CHECK-LABEL: final labeling for "alias1_good"
44
pub unsafe fn alias1_good() {
5-
// CHECK-DAG: ([[@LINE+1]]: mut x): addr_of = READ | WRITE | UNIQUE,
5+
// CHECK-DAG: ([[@LINE+1]]: mut x): addr_of = READ | WRITE | UNIQUE | NON_NULL,
66
let mut x = 0;
7-
// CHECK-DAG: ([[@LINE+1]]: p): {{.*}}type = UNIQUE#
7+
// CHECK-DAG: ([[@LINE+1]]: p): {{.*}}type = UNIQUE | NON_NULL#
88
let p = ptr::addr_of_mut!(x);
9-
// CHECK-DAG: ([[@LINE+1]]: q): {{.*}}type = READ | WRITE | UNIQUE#
9+
// CHECK-DAG: ([[@LINE+1]]: q): {{.*}}type = READ | WRITE | UNIQUE | NON_NULL#
1010
let q = ptr::addr_of_mut!(x);
1111
*q = 1;
1212
}
1313

1414
// CHECK-LABEL: final labeling for "alias1_bad"
1515
pub unsafe fn alias1_bad() {
16-
// CHECK-DAG: ([[@LINE+2]]: mut x): addr_of = READ | WRITE,
16+
// CHECK-DAG: ([[@LINE+2]]: mut x): addr_of = READ | WRITE | NON_NULL,
1717
// CHECK-DAG: ([[@LINE+1]]: mut x): addr_of flags = CELL,
1818
let mut x = 0;
19-
// CHECK-DAG: ([[@LINE+1]]: p): {{.*}}type = READ | WRITE#
19+
// CHECK-DAG: ([[@LINE+1]]: p): {{.*}}type = READ | WRITE | NON_NULL#
2020
let p = ptr::addr_of_mut!(x);
21-
// CHECK-DAG: ([[@LINE+2]]: q): {{.*}}type = (empty)#
21+
// CHECK-DAG: ([[@LINE+2]]: q): {{.*}}type = NON_NULL#
2222
// CHECK-DAG: ([[@LINE+1]]: q): {{.*}}type flags = CELL#
2323
let q = ptr::addr_of_mut!(x);
2424
*p = 1;

c2rust-analyze/tests/filecheck/alias2.rs

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,43 +1,43 @@
11
use std::ptr;
22

33
// CHECK-LABEL: final labeling for "alias2_copy_good"
4-
// CHECK-DAG: ([[@LINE+1]]: x): {{.*}}type = READ | WRITE | UNIQUE#
4+
// CHECK-DAG: ([[@LINE+1]]: x): {{.*}}type = READ | WRITE | UNIQUE | NON_NULL#
55
pub unsafe fn alias2_copy_good(x: *mut i32) {
6-
// CHECK-DAG: ([[@LINE+1]]: p): {{.*}}type = UNIQUE#
6+
// CHECK-DAG: ([[@LINE+1]]: p): {{.*}}type = UNIQUE | NON_NULL#
77
let p = x;
8-
// CHECK-DAG: ([[@LINE+1]]: q): {{.*}}type = READ | WRITE | UNIQUE#
8+
// CHECK-DAG: ([[@LINE+1]]: q): {{.*}}type = READ | WRITE | UNIQUE | NON_NULL#
99
let q = x;
1010
*q = 1;
1111
}
1212

1313
// CHECK-LABEL: final labeling for "alias2_addr_of_good"
14-
// CHECK-DAG: ([[@LINE+1]]: x): {{.*}}type = READ | WRITE | UNIQUE#
14+
// CHECK-DAG: ([[@LINE+1]]: x): {{.*}}type = READ | WRITE | UNIQUE | NON_NULL#
1515
pub unsafe fn alias2_addr_of_good(x: *mut i32) {
16-
// CHECK-DAG: ([[@LINE+1]]: p): {{.*}}type = UNIQUE#
16+
// CHECK-DAG: ([[@LINE+1]]: p): {{.*}}type = UNIQUE | NON_NULL#
1717
let p = ptr::addr_of_mut!(*x);
18-
// CHECK-DAG: ([[@LINE+1]]: q): {{.*}}type = READ | WRITE | UNIQUE#
18+
// CHECK-DAG: ([[@LINE+1]]: q): {{.*}}type = READ | WRITE | UNIQUE | NON_NULL#
1919
let q = ptr::addr_of_mut!(*x);
2020
*q = 1;
2121
}
2222

2323
// CHECK-LABEL: final labeling for "alias2_copy_bad"
24-
// CHECK-DAG: ([[@LINE+2]]: x): {{.*}}type = READ | WRITE#
24+
// CHECK-DAG: ([[@LINE+2]]: x): {{.*}}type = READ | WRITE | NON_NULL#
2525
// CHECK-DAG: ([[@LINE+1]]: x): {{.*}}type flags = CELL#
2626
pub unsafe fn alias2_copy_bad(x: *mut i32) {
27-
// CHECK-DAG: ([[@LINE+1]]: p): {{.*}}type = READ | WRITE#
27+
// CHECK-DAG: ([[@LINE+1]]: p): {{.*}}type = READ | WRITE | NON_NULL#
2828
let p = x;
29-
// CHECK-DAG: ([[@LINE+1]]: q): {{.*}}type = (empty)#
29+
// CHECK-DAG: ([[@LINE+1]]: q): {{.*}}type = NON_NULL#
3030
let q = x;
3131
*p = 1;
3232
}
3333

3434
// CHECK-LABEL: final labeling for "alias2_addr_of_bad"
35-
// CHECK-DAG: ([[@LINE+2]]: x): {{.*}}type = READ | WRITE#
35+
// CHECK-DAG: ([[@LINE+2]]: x): {{.*}}type = READ | WRITE | NON_NULL#
3636
// CHECK-DAG: ([[@LINE+1]]: x): {{.*}}type flags = CELL#
3737
pub unsafe fn alias2_addr_of_bad(x: *mut i32) {
38-
// CHECK-DAG: ([[@LINE+1]]: p): {{.*}}type = READ | WRITE#
38+
// CHECK-DAG: ([[@LINE+1]]: p): {{.*}}type = READ | WRITE | NON_NULL#
3939
let p = ptr::addr_of_mut!(*x);
40-
// CHECK-DAG: ([[@LINE+1]]: q): {{.*}}type = (empty)#
40+
// CHECK-DAG: ([[@LINE+1]]: q): {{.*}}type = NON_NULL#
4141
let q = ptr::addr_of_mut!(*x);
4242
*p = 1;
4343
}

0 commit comments

Comments
 (0)