Skip to content

Commit 539b58d

Browse files
committed
Unignore and restructure test_drop test. Also now run on windows
Signed-off-by: Ludvig Liljenberg <[email protected]>
1 parent b419bde commit 539b58d

File tree

4 files changed

+94
-146
lines changed

4 files changed

+94
-146
lines changed

Cargo.lock

Lines changed: 0 additions & 107 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Justfile

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,6 @@ test-isolated target=default-target features="":
151151
cargo test {{ if features =="" {''} else if features=="no-default-features" {"--no-default-features" } else {"--no-default-features -F init-paging," + features } }} --profile={{ if target == "debug" { "dev" } else { target } }} -p hyperlight-host --lib -- sandbox::uninitialized::tests::test_log_trace --exact --ignored
152152
cargo test {{ if features =="" {''} else if features=="no-default-features" {"--no-default-features" } else {"--no-default-features -F init-paging," + features } }} --profile={{ if target == "debug" { "dev" } else { target } }} -p hyperlight-host --lib -- sandbox::initialized_multi_use::tests::create_1000_sandboxes --exact --ignored
153153
cargo test {{ if features =="" {''} else if features=="no-default-features" {"--no-default-features" } else {"--no-default-features -F init-paging," + features } }} --profile={{ if target == "debug" { "dev" } else { target } }} -p hyperlight-host --lib -- sandbox::outb::tests::test_log_outb_log --exact --ignored
154-
cargo test {{ if features =="" {''} else if features=="no-default-features" {"--no-default-features" } else {"--no-default-features -F init-paging," + features } }} --profile={{ if target == "debug" { "dev" } else { target } }} -p hyperlight-host --lib -- mem::shared_mem::tests::test_drop --exact --ignored
155154
cargo test {{ if features =="" {''} else if features=="no-default-features" {"--no-default-features" } else {"--no-default-features -F init-paging," + features } }} --profile={{ if target == "debug" { "dev" } else { target } }} -p hyperlight-host --test integration_test -- log_message --exact --ignored
156155
@# metrics tests
157156
cargo test {{ if features =="" {''} else if features=="no-default-features" {"--no-default-features" } else {"--no-default-features -F function_call_metrics,init-paging," + features } }} --profile={{ if target == "debug" { "dev" } else { target } }} -p hyperlight-host --lib -- metrics::tests::test_metrics_are_emitted --exact

src/hyperlight_host/Cargo.toml

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -114,9 +114,6 @@ windows = { version = "0.62", features = [
114114
"Win32_System_Diagnostics_ToolHelp",
115115
] }
116116

117-
[target.'cfg(unix)'.dev-dependencies]
118-
proc-maps = "0.4.0"
119-
120117
[build-dependencies]
121118
anyhow = { version = "1.0.99" }
122119
cfg_aliases = "0.2.1"

src/hyperlight_host/src/mem/shared_mem.rs

Lines changed: 94 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1127,51 +1127,110 @@ mod tests {
11271127
assert_eq!(data, ret_vec);
11281128
}
11291129

1130-
/// A test to ensure that, if a `SharedMem` instance is cloned
1131-
/// and _all_ clones are dropped, the memory region will no longer
1132-
/// be valid.
1133-
///
1134-
/// This test is ignored because it is incompatible with other tests as
1135-
/// they may be allocating memory at the same time.
1136-
///
1137-
/// Marking this test as ignored means that running `cargo test` will not
1138-
/// run it. This feature will allow a developer who runs that command
1139-
/// from their workstation to be successful without needing to know about
1140-
/// test interdependencies. This test will, however, be run explicitly as a
1141-
/// part of the CI pipeline.
1130+
/// Makes sure drop actually frees the underlying memory
11421131
#[test]
1143-
#[ignore]
1144-
#[cfg(target_os = "linux")]
11451132
fn test_drop() {
1146-
use proc_maps::maps_contain_addr;
1147-
1148-
let pid = std::process::id();
1149-
11501133
let eshm = ExclusiveSharedMemory::new(PAGE_SIZE_USIZE).unwrap();
11511134
let (hshm1, gshm) = eshm.build();
11521135
let hshm2 = hshm1.clone();
1153-
let addr = hshm1.raw_ptr() as usize;
1154-
1155-
// ensure the address is in the process's virtual memory
1156-
let maps_before_drop = proc_maps::get_process_maps(pid.try_into().unwrap()).unwrap();
1157-
assert!(
1158-
maps_contain_addr(addr, &maps_before_drop),
1159-
"shared memory address {:#x} was not found in process map, but should be",
1160-
addr,
1161-
);
1162-
// drop both shared memory instances, which should result
1136+
let addr = hshm1.raw_ptr();
1137+
let _size = hshm1.raw_mem_size();
1138+
1139+
// Verify memory is initially accessible
1140+
#[cfg(target_os = "linux")]
1141+
{
1142+
let result =
1143+
unsafe { libc::madvise(addr as *mut libc::c_void, _size, libc::MADV_NORMAL) };
1144+
assert_eq!(
1145+
result,
1146+
0,
1147+
"Memory should be accessible before drop - madvise failed with errno: {}",
1148+
std::io::Error::last_os_error().raw_os_error().unwrap()
1149+
);
1150+
}
1151+
1152+
#[cfg(target_os = "windows")]
1153+
{
1154+
use core::ffi::c_void;
1155+
use windows::Win32::System::Memory::{
1156+
MEM_FREE, MEMORY_BASIC_INFORMATION, VirtualQuery,
1157+
};
1158+
1159+
let mut mbi = MEMORY_BASIC_INFORMATION::default();
1160+
let result = unsafe {
1161+
VirtualQuery(
1162+
Some(addr as *const c_void),
1163+
&mut mbi,
1164+
std::mem::size_of::<MEMORY_BASIC_INFORMATION>(),
1165+
)
1166+
};
1167+
assert_ne!(
1168+
result, 0,
1169+
"VirtualQuery should succeed on mapped memory at address {:#x}",
1170+
addr as usize
1171+
);
1172+
assert_ne!(
1173+
mbi.State, MEM_FREE,
1174+
"Memory should not be in MEM_FREE state before drop at address {:#x}",
1175+
addr as usize
1176+
);
1177+
}
1178+
1179+
// Drop both shared memory instances, which should result
11631180
// in freeing the memory region
11641181
drop(hshm1);
11651182
drop(hshm2);
11661183
drop(gshm);
11671184

1168-
let maps_after_drop = proc_maps::get_process_maps(pid.try_into().unwrap()).unwrap();
1169-
// now, ensure the address is not in the process's virtual memory
1170-
assert!(
1171-
!maps_contain_addr(addr, &maps_after_drop),
1172-
"shared memory address {:#x} was found in the process map, but shouldn't be",
1173-
addr
1174-
);
1185+
// Verify memory is no longer accessible
1186+
#[cfg(target_os = "linux")]
1187+
{
1188+
// Try madvise on the unmapped memory - should fail with ENOMEM
1189+
let result =
1190+
unsafe { libc::madvise(addr as *mut libc::c_void, _size, libc::MADV_NORMAL) };
1191+
assert_eq!(
1192+
result, -1,
1193+
"madvise should return -1 on unmapped memory at address {:#x}",
1194+
addr as usize
1195+
);
1196+
1197+
let errno = std::io::Error::last_os_error().raw_os_error().unwrap_or(-1);
1198+
assert_eq!(
1199+
errno,
1200+
libc::ENOMEM,
1201+
"Expected ENOMEM for unmapped memory, but got errno: {} at address {:#x}",
1202+
errno,
1203+
addr as usize
1204+
);
1205+
}
1206+
1207+
#[cfg(target_os = "windows")]
1208+
{
1209+
use core::ffi::c_void;
1210+
use windows::Win32::System::Memory::{
1211+
MEM_FREE, MEMORY_BASIC_INFORMATION, VirtualQuery,
1212+
};
1213+
1214+
// Try VirtualQuery on the unmapped memory - should show MEM_FREE state
1215+
let mut mbi_after = MEMORY_BASIC_INFORMATION::default();
1216+
let result = unsafe {
1217+
VirtualQuery(
1218+
Some(addr as *const c_void),
1219+
&mut mbi_after,
1220+
std::mem::size_of::<MEMORY_BASIC_INFORMATION>(),
1221+
)
1222+
};
1223+
assert_ne!(
1224+
result, 0,
1225+
"VirtualQuery should succeed even on unmapped memory at address {:#x}",
1226+
addr as usize
1227+
);
1228+
assert_eq!(
1229+
mbi_after.State, MEM_FREE,
1230+
"Memory should be in MEM_FREE state after drop at address {:#x}",
1231+
addr as usize
1232+
);
1233+
}
11751234
}
11761235

11771236
#[cfg(target_os = "linux")]

0 commit comments

Comments
 (0)