Skip to content

add test for fields of references #567

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.

Sign up for GitHub

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
oinoom merged 8 commits into from
Aug 3, 2022
Merged

add test for fields of references #567

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
6bf061b
add test for fields of references
Aug 1, 2022
4ca53bd
fix bug re: source of AddrOfLocal
Aug 1, 2022
f386662
Merge branch 'master' into tests.refs.fields
kkysen Aug 2, 2022
ab7c822
Move the new test to the end so the snapshot isn't disturbed as much.
kkysen Aug 2, 2022
fed5849
Merge pull request #574 from immunant/kkysen/tests.refs.fields/test-a…
Aug 3, 2022
c80e0aa
make test use proper rust
Aug 3, 2022
31053a8
update snapshot
Aug 3, 2022
41d6fe2
Merge branch 'master' into tests.refs.fields
Aug 3, 2022
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
11 changes: 11 additions & 0 deletions analysis/test/src/pointers.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,16 @@ pub unsafe extern "C" fn test_unique_ref() {
*fresh1 = &mut x as *mut i32;
}
#[no_mangle]
pub unsafe extern "C" fn test_ref_field() {
let s = calloc(
0i32 as libc::c_ulong,
Comment thread
oinoom marked this conversation as resolved.
Outdated
::std::mem::size_of::<S>() as libc::c_ulong,
) as *mut S;
let s_ref = &mut *s;
(*s_ref).field4.field4 = (*s_ref).field4.field4;
free(s_ref as *mut S as *mut libc::c_void);
Comment thread
oinoom marked this conversation as resolved.
Outdated
}
#[no_mangle]
pub unsafe extern "C" fn test_realloc_reassign() {
let mut s = malloc(::std::mem::size_of::<S>() as libc::c_ulong);
s = realloc(s, 2 * mem::size_of::<S>() as c_ulong);
Expand Down Expand Up @@ -504,6 +514,7 @@ unsafe fn main_0(mut argc: libc::c_int, mut argv: *mut *mut libc::c_char) -> lib
test_arg_rec();
test_shared_ref();
test_unique_ref();
test_ref_field();
Comment thread
oinoom marked this conversation as resolved.
Outdated
test_realloc_reassign();
test_realloc_fresh();
test_load_addr();
Expand Down
4 changes: 3 additions & 1 deletion dynamic_instrumentation/src/instrument.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,10 +241,12 @@ impl<'tcx> Visitor<'tcx> for InstrumentationAdder<'_, 'tcx> {
statement_index: location.statement_index + 1,
..location
};

let source = remove_outer_deref(*p, self.tcx());
Copy link
Copy Markdown
Contributor

@kkysen kkysen Aug 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove_outer_deref(*p, self.tcx()) is already computed above on line 238:

                    && place_ty(&remove_outer_deref(*p, self.tcx())).is_region_ptr() =>

Could you de-duplicate this?

Copy link
Copy Markdown
Contributor

@kkysen kkysen Aug 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe a try_remove_outer_deref would be useful here:

/// Try to strip the initital [`Deref`](ProjectionElem::Deref)
/// from a [`projection`](PlaceRef::projection) sequence.
pub fn try_remove_outer_deref<'tcx>(p: Place<'tcx>, tcx: TyCtxt<'tcx>) -> Option<Place<'tcx>> {
    // Remove outer deref if present
    match p.as_ref() {
        PlaceRef {
            local,
            projection: &[ref base @ .., ProjectionElem::Deref],
        } => Some(Place {
            local,
            projection: tcx.intern_place_elems(base),
        }),
        _ => None,
    }
}

/// Strip the initital [`Deref`](ProjectionElem::Deref)
/// from a [`projection`](PlaceRef::projection) sequence
/// if there is one.
pub fn remove_outer_deref<'tcx>(p: Place<'tcx>, tcx: TyCtxt<'tcx>) -> Place<'tcx> {
    try_remove_outer_deref(p, tcx).unwrap_or(p)
}

Copy link
Copy Markdown
Contributor

@kkysen kkysen Aug 2, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could do

            Rvalue::AddressOf(_, p)
                if let Some(source) = try_remove_outer_deref(*p, self.tcx())
                && place_ty(source).is_region_ptr() =>
            {
                // Instrument which local's address is taken
                self.loc(location.successor_within_block(), copy_fn)
                    .arg_var(dest)
                    .source(source)
                    .dest(&dest)
                    .debug_mir(location)
                    .add_to(self);
            }

but that requires #![feature(if_let_guard, let_chains)]. It might be too clunky without those, I think.

Copy link
Copy Markdown
Contributor

@kkysen kkysen Aug 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about the changes in #573?

Copy link
Copy Markdown
Contributor

@kkysen kkysen Aug 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think #573 is probably too big for the scope of this PR, but keep it in my mind for after merging this PR.

// Instrument which local's address is taken
self.loc(instrumentation_location, copy_fn)
.arg_var(dest)
.source(p)
.source(&source)
.dest(&dest)
.debug_mir(location)
.add_to(self);
Expand Down
66 changes: 41 additions & 25 deletions pdg/src/snapshots/c2rust_pdg__tests__analysis_test_pdg_snapshot.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,59 +165,59 @@ g {
n[1]: copy n[0] => _7 @ bb2[14]: fn exercise_allocator; _7 = move _8 as *const u8 (Pointer(ArrayToPointer))
n[2]: copy n[1] => _6 @ bb2[16]: fn exercise_allocator; _6 = move _7 as *const i8 (Misc)
n[3]: copy n[2] => _1 @ bb0[0]: fn printf; _5 = pointers::printf(move _6, move _10)
n[4]: copy n[0] => _30 @ bb11[7]: fn exercise_allocator; _30 = &raw const (*_31)
n[4]: copy _ => _30 @ bb11[7]: fn exercise_allocator; _30 = &raw const (*_31)
Comment thread
oinoom marked this conversation as resolved.
Outdated
n[5]: copy n[4] => _29 @ bb11[8]: fn exercise_allocator; _29 = move _30 as *const u8 (Pointer(ArrayToPointer))
n[6]: copy n[5] => _28 @ bb11[10]: fn exercise_allocator; _28 = move _29 as *const i8 (Misc)
n[7]: copy n[6] => _1 @ bb0[0]: fn printf; _27 = pointers::printf(move _28, move _32)
n[8]: copy n[0] => _30 @ bb11[7]: fn exercise_allocator; _30 = &raw const (*_31)
n[8]: copy _ => _30 @ bb11[7]: fn exercise_allocator; _30 = &raw const (*_31)
n[9]: copy n[8] => _29 @ bb11[8]: fn exercise_allocator; _29 = move _30 as *const u8 (Pointer(ArrayToPointer))
n[10]: copy n[9] => _28 @ bb11[10]: fn exercise_allocator; _28 = move _29 as *const i8 (Misc)
n[11]: copy n[10] => _1 @ bb0[0]: fn printf; _27 = pointers::printf(move _28, move _32)
n[12]: copy n[0] => _60 @ bb29[7]: fn exercise_allocator; _60 = &raw const (*_61)
n[12]: copy _ => _60 @ bb29[7]: fn exercise_allocator; _60 = &raw const (*_61)
n[13]: copy n[12] => _59 @ bb29[8]: fn exercise_allocator; _59 = move _60 as *const u8 (Pointer(ArrayToPointer))
n[14]: copy n[13] => _58 @ bb29[10]: fn exercise_allocator; _58 = move _59 as *const i8 (Misc)
n[15]: copy n[14] => _1 @ bb0[0]: fn printf; _57 = pointers::printf(move _58, move _62)
n[16]: copy n[0] => _60 @ bb29[7]: fn exercise_allocator; _60 = &raw const (*_61)
n[16]: copy _ => _60 @ bb29[7]: fn exercise_allocator; _60 = &raw const (*_61)
n[17]: copy n[16] => _59 @ bb29[8]: fn exercise_allocator; _59 = move _60 as *const u8 (Pointer(ArrayToPointer))
n[18]: copy n[17] => _58 @ bb29[10]: fn exercise_allocator; _58 = move _59 as *const i8 (Misc)
n[19]: copy n[18] => _1 @ bb0[0]: fn printf; _57 = pointers::printf(move _58, move _62)
n[20]: copy n[0] => _60 @ bb29[7]: fn exercise_allocator; _60 = &raw const (*_61)
n[20]: copy _ => _60 @ bb29[7]: fn exercise_allocator; _60 = &raw const (*_61)
n[21]: copy n[20] => _59 @ bb29[8]: fn exercise_allocator; _59 = move _60 as *const u8 (Pointer(ArrayToPointer))
n[22]: copy n[21] => _58 @ bb29[10]: fn exercise_allocator; _58 = move _59 as *const i8 (Misc)
n[23]: copy n[22] => _1 @ bb0[0]: fn printf; _57 = pointers::printf(move _58, move _62)
n[24]: copy n[0] => _93 @ bb49[7]: fn exercise_allocator; _93 = &raw const (*_94)
n[24]: copy _ => _93 @ bb49[7]: fn exercise_allocator; _93 = &raw const (*_94)
n[25]: copy n[24] => _92 @ bb49[8]: fn exercise_allocator; _92 = move _93 as *const u8 (Pointer(ArrayToPointer))
n[26]: copy n[25] => _91 @ bb49[10]: fn exercise_allocator; _91 = move _92 as *const i8 (Misc)
n[27]: copy n[26] => _1 @ bb0[0]: fn printf; _90 = pointers::printf(move _91, move _95)
n[28]: copy n[0] => _93 @ bb49[7]: fn exercise_allocator; _93 = &raw const (*_94)
n[28]: copy _ => _93 @ bb49[7]: fn exercise_allocator; _93 = &raw const (*_94)
n[29]: copy n[28] => _92 @ bb49[8]: fn exercise_allocator; _92 = move _93 as *const u8 (Pointer(ArrayToPointer))
n[30]: copy n[29] => _91 @ bb49[10]: fn exercise_allocator; _91 = move _92 as *const i8 (Misc)
n[31]: copy n[30] => _1 @ bb0[0]: fn printf; _90 = pointers::printf(move _91, move _95)
n[32]: copy n[0] => _93 @ bb49[7]: fn exercise_allocator; _93 = &raw const (*_94)
n[32]: copy _ => _93 @ bb49[7]: fn exercise_allocator; _93 = &raw const (*_94)
n[33]: copy n[32] => _92 @ bb49[8]: fn exercise_allocator; _92 = move _93 as *const u8 (Pointer(ArrayToPointer))
n[34]: copy n[33] => _91 @ bb49[10]: fn exercise_allocator; _91 = move _92 as *const i8 (Misc)
n[35]: copy n[34] => _1 @ bb0[0]: fn printf; _90 = pointers::printf(move _91, move _95)
n[36]: copy n[0] => _93 @ bb49[7]: fn exercise_allocator; _93 = &raw const (*_94)
n[36]: copy _ => _93 @ bb49[7]: fn exercise_allocator; _93 = &raw const (*_94)
n[37]: copy n[36] => _92 @ bb49[8]: fn exercise_allocator; _92 = move _93 as *const u8 (Pointer(ArrayToPointer))
n[38]: copy n[37] => _91 @ bb49[10]: fn exercise_allocator; _91 = move _92 as *const i8 (Misc)
n[39]: copy n[38] => _1 @ bb0[0]: fn printf; _90 = pointers::printf(move _91, move _95)
n[40]: copy n[0] => _8 @ bb2[13]: fn simple_analysis; _8 = &raw const (*_9)
n[40]: copy _ => _8 @ bb2[13]: fn simple_analysis; _8 = &raw const (*_9)
n[41]: copy n[40] => _7 @ bb2[14]: fn simple_analysis; _7 = move _8 as *const u8 (Pointer(ArrayToPointer))
n[42]: copy n[41] => _6 @ bb2[16]: fn simple_analysis; _6 = move _7 as *const i8 (Misc)
n[43]: copy n[42] => _1 @ bb0[0]: fn printf; _5 = pointers::printf(move _6, move _10)
n[44]: copy n[0] => _5 @ bb0[7]: fn analysis2_helper; _5 = &raw const (*_6)
n[44]: copy n[42] => _5 @ bb0[7]: fn analysis2_helper; _5 = &raw const (*_6)
n[45]: copy n[44] => _4 @ bb0[8]: fn analysis2_helper; _4 = move _5 as *const u8 (Pointer(ArrayToPointer))
n[46]: copy n[45] => _3 @ bb0[10]: fn analysis2_helper; _3 = move _4 as *const i8 (Misc)
n[47]: copy n[46] => _1 @ bb0[0]: fn printf; _2 = pointers::printf(move _3, move _7)
n[48]: copy n[0] => _8 @ bb2[13]: fn inter_function_analysis; _8 = &raw const (*_9)
n[48]: copy _ => _8 @ bb2[13]: fn inter_function_analysis; _8 = &raw const (*_9)
n[49]: copy n[48] => _7 @ bb2[14]: fn inter_function_analysis; _7 = move _8 as *const u8 (Pointer(ArrayToPointer))
n[50]: copy n[49] => _6 @ bb2[16]: fn inter_function_analysis; _6 = move _7 as *const i8 (Misc)
n[51]: copy n[50] => _1 @ bb0[0]: fn printf; _5 = pointers::printf(move _6, move _10)
n[52]: copy n[0] => _10 @ bb2[20]: fn invalid; _10 = &raw const (*_11)
n[52]: copy _ => _10 @ bb2[20]: fn invalid; _10 = &raw const (*_11)
n[53]: copy n[52] => _9 @ bb2[21]: fn invalid; _9 = move _10 as *const u8 (Pointer(ArrayToPointer))
n[54]: copy n[53] => _8 @ bb2[23]: fn invalid; _8 = move _9 as *const i8 (Misc)
n[55]: copy n[54] => _1 @ bb0[0]: fn printf; _7 = pointers::printf(move _8, move _12)
n[56]: copy n[0] => _16 @ bb3[11]: fn invalid; _16 = &raw const (*_17)
n[56]: copy _ => _16 @ bb3[11]: fn invalid; _16 = &raw const (*_17)
n[57]: copy n[56] => _15 @ bb3[12]: fn invalid; _15 = move _16 as *const u8 (Pointer(ArrayToPointer))
n[58]: copy n[57] => _14 @ bb3[14]: fn invalid; _14 = move _15 as *const i8 (Misc)
n[59]: copy n[58] => _1 @ bb0[0]: fn printf; _13 = pointers::printf(move _14, move _18)
Expand Down Expand Up @@ -533,7 +533,7 @@ g {
n[2]: copy n[1] => _1 @ bb0[0]: fn connection_accepted; _13 = pointers::connection_accepted(move _14, const 0_i32)
n[3]: field.0 n[2] => _10 @ bb2[10]: fn connection_accepted; _10 = ((*_1).0: *mut pointers::fdevents)
n[4]: addr.load n[3] => _ @ bb2[10]: fn connection_accepted; _10 = ((*_1).0: *mut pointers::fdevents)
n[5]: copy n[0] => _16 @ bb5[5]: fn lighttpd_test; _16 = &raw mut (*_10)
n[5]: copy n[3] => _16 @ bb5[5]: fn lighttpd_test; _16 = &raw mut (*_10)
n[6]: copy n[5] => _1 @ bb0[0]: fn connection_close; _15 = pointers::connection_close(move _16, move _17)
n[7]: field.0 n[6] => _4 @ bb0[2]: fn connection_close; _4 = ((*_1).0: *mut pointers::fdevents)
n[8]: addr.load n[7] => _ @ bb0[2]: fn connection_close; _4 = ((*_1).0: *mut pointers::fdevents)
Expand Down Expand Up @@ -664,7 +664,7 @@ g {
n[3]: copy n[2] => _1 @ bb0[0]: fn shared_ref_foo; _4 = pointers::shared_ref_foo(move _5)
n[4]: copy n[3] => _0 @ bb0[1]: fn shared_ref_foo; _0 = _1
n[5]: copy n[4] => _4 @ bb1[0]: fn test_shared_ref; _4 = pointers::shared_ref_foo(move _5)
n[6]: copy n[0] => _6 @ bb1[4]: fn test_shared_ref; _6 = &raw const (*_4)
n[6]: copy n[5] => _6 @ bb1[4]: fn test_shared_ref; _6 = &raw const (*_4)
}
nodes_that_need_write = []

Expand All @@ -687,6 +687,22 @@ g {
}
nodes_that_need_write = []

g {
n[0]: alloc _ => _2 @ bb1[2]: fn test_ref_field; _2 = pointers::calloc(move _3, move _4)
n[1]: copy n[0] => _1 @ bb2[3]: fn test_ref_field; _1 = move _2 as *mut pointers::S (Misc)
Comment thread
kkysen marked this conversation as resolved.
Outdated
n[2]: copy n[1] => _6 @ bb2[6]: fn test_ref_field; _6 = &mut (*_1)
n[3]: field.3 n[2] => _ @ bb2[9]: fn test_ref_field; _7 = (((*_6).3: pointers::T).3: i32)
n[4]: field.3 n[3] => _7 @ bb2[9]: fn test_ref_field; _7 = (((*_6).3: pointers::T).3: i32)
n[5]: addr.load n[4] => _ @ bb2[9]: fn test_ref_field; _7 = (((*_6).3: pointers::T).3: i32)
n[6]: field.3 n[2] => _ @ bb2[10]: fn test_ref_field; (((*_6).3: pointers::T).3: i32) = move _7
n[7]: field.3 n[6] => _ @ bb2[10]: fn test_ref_field; (((*_6).3: pointers::T).3: i32) = move _7
n[8]: addr.store n[7] => _ @ bb2[10]: fn test_ref_field; (((*_6).3: pointers::T).3: i32) = move _7
n[9]: copy n[2] => _10 @ bb2[16]: fn test_ref_field; _10 = &raw mut (*_6)
n[10]: copy n[9] => _9 @ bb2[17]: fn test_ref_field; _9 = move _10 as *mut libc::c_void (Misc)
n[11]: free n[10] => _8 @ bb2[18]: fn test_ref_field; _8 = pointers::free(move _9)
}
nodes_that_need_write = [8, 7, 6, 2, 1, 0]

g {
n[0]: alloc _ => _1 @ bb1[2]: fn test_realloc_reassign; _1 = pointers::malloc(move _2)
n[1]: copy n[0] => _5 @ bb2[5]: fn test_realloc_reassign; _5 = _1
Expand Down Expand Up @@ -885,13 +901,13 @@ g {
nodes_that_need_write = [2, 0]

g {
n[0]: &_35 _ => _34 @ bb30[4]: fn main_0; _34 = &mut _35
n[1]: copy n[0] => _40 @ bb30[12]: fn main_0; _40 = &(*_34)
n[2]: copy n[1] => _39 @ bb30[13]: fn main_0; _39 = move _40 as &[i32] (Pointer(Unsize))
n[3]: copy n[2] => _1 @ bb0[0]: fn len; _38 = core::slice::<impl [i32]>::len(move _39)
n[4]: copy n[0] => _42 @ bb31[6]: fn main_0; _42 = &raw mut (*_34)
n[5]: copy n[4] => _41 @ bb31[7]: fn main_0; _41 = move _42 as *mut i32 (Pointer(ArrayToPointer))
n[6]: copy n[5] => _2 @ bb0[0]: fn insertion_sort; _36 = pointers::insertion_sort(move _37, move _41)
n[0]: &_36 _ => _35 @ bb31[4]: fn main_0; _35 = &mut _36
n[1]: copy n[0] => _41 @ bb31[12]: fn main_0; _41 = &(*_35)
n[2]: copy n[1] => _40 @ bb31[13]: fn main_0; _40 = move _41 as &[i32] (Pointer(Unsize))
n[3]: copy n[2] => _1 @ bb0[0]: fn len; _39 = core::slice::<impl [i32]>::len(move _40)
n[4]: copy n[0] => _43 @ bb32[6]: fn main_0; _43 = &raw mut (*_35)
n[5]: copy n[4] => _42 @ bb32[7]: fn main_0; _42 = move _43 as *mut i32 (Pointer(ArrayToPointer))
n[6]: copy n[5] => _2 @ bb0[0]: fn insertion_sort; _37 = pointers::insertion_sort(move _38, move _42)
n[7]: copy n[6] => _10 @ bb3[4]: fn insertion_sort; _10 = _2
n[8]: offset[1] n[7] => _9 @ bb3[9]: fn insertion_sort; _9 = std::ptr::mut_ptr::<impl *mut i32>::offset(move _10, move _11)
n[9]: addr.load n[8] => _ @ bb5[2]: fn insertion_sort; _8 = (*_9)
Expand Down Expand Up @@ -964,6 +980,6 @@ g {
}
nodes_that_need_write = [75, 74, 73, 66, 65, 64, 63, 62, 61, 54, 53, 52, 45, 44, 43, 33, 32, 31, 27, 26, 25, 15, 14, 13, 6, 5, 4, 0]

num_graphs = 76
num_nodes = 658
num_graphs = 77
num_nodes = 670