Skip to content
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

make set_for_current function unsafe #346

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kitcatier
Copy link

Checklist
  • tests are passing with cargo test.
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message is clear

pub fn set_for_current(core_id: CoreId) {
tracing::trace!("Executor: placement: set affinity on core {}", core_id.id);
set_for_current_helper(core_id);
}

pub fn set_for_current(core_id: CoreId) {
// Turn `core_id` into a `libc::cpu_set_t` with only
// one core active.
let mut set = new_cpu_set();
unsafe { CPU_SET(core_id.id, &mut set) };
// Set the current thread's core affinity.
unsafe {
sched_setaffinity(
0, // Defaults to current thread
mem::size_of::<cpu_set_t>(),
&set,
);
}
}

pub fn set_for_current(core_id: CoreId) {
// Convert `CoreId` back into mask.
let mask: DWORD_PTR = 1 << core_id.id;
// Set core affinity for current thread.
unsafe {
SetThreadAffinityMask(GetCurrentThread(), mask);
}
}

Target_OS = "windows"
Hello, in some tests of your bastion_executor project, my program appeared "attempt to shift left with overflow", and the traceback indicated that the set_for_current function in this project crashed when doing a left shift operation.

extern crate bastion_executor;
use bastion_executor::placement::CoreId;
fn main() {
    let a:CoreId = CoreId { id: 771157545605903283 };
    let _ = bastion_executor::placement::set_for_current(a);
}

thread 'main' panicked at 'attempt to shift left with overflow'
stack backtrace: 0: std::panicking::begin_panic_handler at /rustc/73c9eaf21454b718e7c549984d9eb6e1f75e995c/library\std\src\panicking.rs:575 1: core::panicking::panic_fmt at /rustc/73c9eaf21454b718e7c549984d9eb6e1f75e995c/library\core\src\panicking.rs:65 2: core::panicking::panic at /rustc/73c9eaf21454b718e7c549984d9eb6e1f75e995c/library\core\src\panicking.rs:115 3: bastion_executor::placement::windows::set_for_current at C:\Users\.cargo\registry\src\github.com-1ecc6299db9ec823\bastion-executor-0.4.2\src\placement.rs:216 4: bastion_executor::placement::set_for_current_helper at C:\Users\.cargo\registry\src\github.com-1ecc6299db9ec823\bastion-executor-0.4.2\src\placement.rs:180 5: bastion_executor::placement::set_for_current at C:\Users\.cargo\registry\src\github.com-1ecc6299db9ec823\bastion-executor-0.4.2\src\placement.rs:22 6: hello::main at .\src\main.rs:15 7: core::ops::function::FnOnce::call_once<void (*)(),tuple$<> > at /rustc/73c9eaf21454b718e7c549984d9eb6e1f75e995c\library\core\src\ops\function.rs:510
Target_OS = "linux"
thread 'main' panicked at 'index out of bounds: the len is 16 but the index is 12049336650092238'
stack backtrace: 0: rust_begin_unwind at /rustc/8a97b4812a7a46bb5206487c2455b9c5bfcbd1b9/library/std/src/panicking.rs:575:5 1: core::panicking::panic_fmt at /rustc/8a97b4812a7a46bb5206487c2455b9c5bfcbd1b9/library/core/src/panicking.rs:64:14 2: core::panicking::panic_bounds_check at /rustc/8a97b4812a7a46bb5206487c2455b9c5bfcbd1b9/library/core/src/panicking.rs:147:5 3: libc::unix::linux_like::linux::CPU_SET at /home/dl2/.cargo/registry/src/github.com-1ecc6299db9ec823/libc-0.2.139/src/unix/linux_like/linux/mod.rs:3630:9 4: bastion_executor::placement::linux::set_for_current at /home/dl2/.cargo/registry/src/github.com-1ecc6299db9ec823/bastion-executor-0.4.2/src/placement.rs:76:18 5: bastion_executor::placement::set_for_current_helper at /home/dl2/.cargo/registry/src/github.com-1ecc6299db9ec823/bastion-executor-0.4.2/src/placement.rs:44:5 6: bastion_executor::placement::set_for_current at /home/dl2/.cargo/registry/src/github.com-1ecc6299db9ec823/bastion-executor-0.4.2/src/placement.rs:22:5 7: hello_world::main at ./src/main.rs:17:13 8: core::ops::function::FnOnce::call_once at /rustc/8a97b4812a7a46bb5206487c2455b9c5bfcbd1b9/library/core/src/ops/function.rs:507:5
I think it should be marked as unsafe to remind other callers of the real legal input parameters to avoid possible errors.

@o0Ignition0o
Copy link
Contributor

Oh you're right!

I wonder if we should check for CoreId validity before, or make it return a reuslt

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants