Skip to content

ci: require safety comments for unsafe blocks #203

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
swananan merged 1 commit into from
May 24, 2026
Merged

ci: require safety comments for unsafe blocks #203

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ jobs:
${{ runner.os }}-${{ github.job }}-target-

- name: Run clippy
run: cargo clippy --all-targets --all-features -- -D warnings
run: cargo clippy --all-targets --all-features -- -D warnings -D clippy::undocumented_unsafe_blocks

test:
name: Test Suite
Expand Down
8 changes: 6 additions & 2 deletions e2e-tests/tests/common/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ pub fn init() {
});

// Register an atexit cleanup to remove built fixtures after all tests
REGISTER_CLEANUP.call_once(|| unsafe {
REGISTER_CLEANUP.call_once(|| {
extern "C" fn cleanup_fixtures() {
if preserve_precompiled_fixtures() {
return;
Expand All @@ -277,7 +277,11 @@ pub fn init() {
fixture.cleanup(&base);
}
}
libc::atexit(cleanup_fixtures);
// SAFETY: cleanup_fixtures has C ABI, captures no Rust references, and
// remains available for the process lifetime.
unsafe {
libc::atexit(cleanup_fixtures);
}
});
}

Expand Down
1 change: 1 addition & 0 deletions e2e-tests/tests/common/termination.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ const TERMINATION_POLL_INTERVAL: Duration = Duration::from_millis(50);

#[cfg(unix)]
pub(crate) fn send_sigterm(pid: u32, label: &str) -> anyhow::Result<()> {
// SAFETY: libc::kill has no pointer arguments; pid and signal are plain values.
let rc = unsafe { libc::kill(pid as libc::pid_t, libc::SIGTERM) };
if rc == 0 {
return Ok(());
Expand Down
2 changes: 2 additions & 0 deletions e2e-tests/tests/fixtures/rust_global_program/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,8 @@ pub mod math {
}

fn touch_globals() -> i32 {
// SAFETY: This single-threaded fixture mutates its own static test globals to
// keep DWARF global-variable scenarios observable.
unsafe {
G_COUNTER = G_COUNTER.wrapping_add(1);
CONFIG.a = CONFIG.a.wrapping_add(1);
Expand Down
2 changes: 2 additions & 0 deletions e2e-tests/tests/script_execution.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,8 @@ fn ensure_global_cleanup_registered() {
println!("🧹 Global cleanup completed");
}

// SAFETY: cleanup_on_exit has C ABI, captures no Rust references, and
// remains available for the process lifetime.
unsafe {
libc::atexit(cleanup_on_exit);
}
Expand Down
4 changes: 4 additions & 0 deletions ghostscope-compiler/src/ebpf/codegen/backtrace.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> {
.into_value_after_runtime_returns();

// Write InstructionHeader.inst_type
// SAFETY: inst_buffer points at a reserved instruction region and the
// offset is within InstructionHeader.
let inst_type_ptr = unsafe {
self.builder
.build_gep(
Expand All @@ -39,6 +41,8 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> {
.map_err(|e| CodeGenError::LLVMError(format!("Failed to store inst_type: {e}")))?;

// Write InstructionHeader.data_length (u16)
// SAFETY: inst_buffer points at a reserved instruction region and the
// data_length offset is within InstructionHeader.
let data_length_ptr = unsafe {
self.builder
.build_gep(
Expand Down
7 changes: 7 additions & 0 deletions ghostscope-compiler/src/ebpf/codegen/expr_error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> {
.map_err(|e| CodeGenError::LLVMError(format!("Failed to store inst_type: {e}")))?;

// data_length
// SAFETY: inst_buffer points at a reserved ExprError instruction region
// and data_length is within InstructionHeader.
let data_length_ptr = unsafe {
self.builder
.build_gep(
Expand Down Expand Up @@ -61,6 +63,8 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> {

// Payload fields after header
// string_index at offset sizeof(InstructionHeader) + 0 (u16)
// SAFETY: the payload immediately follows InstructionHeader in the
// reserved ExprError instruction region.
let si_ptr = unsafe {
self.builder
.build_gep(
Expand Down Expand Up @@ -95,6 +99,7 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> {
.map_err(|e| CodeGenError::LLVMError(format!("Failed to store string_index: {e}")))?;

// error_code at +2, flags at +3
// SAFETY: error_code offset is within ExprErrorData in the reserved payload.
let ec_ptr = unsafe {
self.builder
.build_gep(
Expand Down Expand Up @@ -125,6 +130,7 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> {
self.builder
.build_store(ec_ptr, ec_i8)
.map_err(|e| CodeGenError::LLVMError(format!("Failed to store error_code: {e}")))?;
// SAFETY: flags offset is within ExprErrorData in the reserved payload.
let fl_ptr = unsafe {
self.builder
.build_gep(
Expand Down Expand Up @@ -155,6 +161,7 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> {
.map_err(|e| CodeGenError::LLVMError(format!("Failed to store flags: {e}")))?;

// failing_addr at +4 (u64)
// SAFETY: failing_addr offset is within ExprErrorData in the reserved payload.
let addr_ptr = unsafe {
self.builder
.build_gep(
Expand Down
26 changes: 26 additions & 0 deletions ghostscope-compiler/src/ebpf/codegen/format.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -475,6 +475,8 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> {
.build_store(buffer, inst_type_val)
.map_err(|e| CodeGenError::LLVMError(format!("Failed to store inst_type: {e}")))?;
// data_length at +1
// SAFETY: buffer points at a reserved PrintComplexFormat instruction
// region and offset 1 is the InstructionHeader data_length field.
let data_length_ptr = unsafe {
self.builder
.build_gep(
Expand Down Expand Up @@ -504,6 +506,8 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> {
.map_err(|e| CodeGenError::LLVMError(format!("Failed to store data_length: {e}")))?;

// Write PrintComplexFormatData at offset 4
// SAFETY: PrintComplexFormatData starts immediately after InstructionHeader
// at offset 4 in the reserved instruction region.
let data_ptr = unsafe {
self.builder
.build_gep(
Expand Down Expand Up @@ -534,6 +538,7 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> {
.build_store(fsi_ptr, fsi_val)
.map_err(|e| CodeGenError::LLVMError(format!("Failed to store fsi: {e}")))?;
// arg_count (u8) at +2
// SAFETY: arg_count offset is within PrintComplexFormatData.
let arg_cnt_ptr = unsafe {
self.builder
.build_gep(
Expand All @@ -558,6 +563,8 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> {
let reserved_len = effective_reserved[arg_index];

// Base pointer = data_ptr + offset
// SAFETY: offset is advanced by the statically computed per-argument
// payload sizes and remains within the reserved instruction region.
let arg_base = unsafe {
self.builder
.build_gep(
Expand Down Expand Up @@ -590,6 +597,7 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> {
.map_err(|e| CodeGenError::LLVMError(format!("Failed to store vni: {e}")))?;

// type_index(u16) at +2
// SAFETY: type_index offset is within the per-argument payload header.
let ti_ptr = unsafe {
self.builder
.build_gep(
Expand Down Expand Up @@ -618,6 +626,7 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> {
.map_err(|e| CodeGenError::LLVMError(format!("Failed to store ti: {e}")))?;

// status(u8) at +5
// SAFETY: status offset is within the per-argument payload header.
let apl_ptr = unsafe {
self.builder
.build_gep(
Expand All @@ -635,6 +644,7 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> {
.map_err(|e| CodeGenError::LLVMError(format!("Failed to store status: {e}")))?;

// access_path_len(u8) at +4
// SAFETY: access_path_len offset is within the per-argument payload header.
let apl_ptr2 = unsafe {
self.builder
.build_gep(
Expand All @@ -656,6 +666,8 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> {

// access_path bytes at +6..+6+len
for (i, b) in a.access_path.iter().enumerate() {
// SAFETY: i is bounded by access_path.len(), which was included in
// the per-argument reserved payload length.
let byte_ptr = unsafe {
self.builder
.build_gep(
Expand All @@ -676,6 +688,8 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> {
}

// data_len(u16) at +6+path_len (store reserved_len to keep layout consistent)
// SAFETY: data_len follows the access path bytes inside the reserved
// per-argument payload.
let dl_ptr = unsafe {
self.builder
.build_gep(
Expand Down Expand Up @@ -707,6 +721,8 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> {
.map_err(|e| CodeGenError::LLVMError(format!("Failed to store data_len: {e}")))?;

// variable data starts at +8+path_len
// SAFETY: var_data_ptr follows the per-argument header and access path
// inside the reserved payload.
let var_data_ptr = unsafe {
self.builder
.build_gep(
Expand All @@ -728,6 +744,8 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> {
match &a.source {
ComplexArgSource::ImmediateBytes { bytes, .. } => {
for (i, b) in bytes.iter().enumerate() {
// SAFETY: i is bounded by bytes.len(), and immediate bytes
// are included in the per-argument reserved payload.
let byte_ptr = unsafe {
self.builder
.build_gep(
Expand Down Expand Up @@ -874,6 +892,8 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> {
self.builder
.build_store(errno_ptr, errno)
.map_err(|e| CodeGenError::LLVMError(e.to_string()))?;
// SAFETY: read-error payload reserves 12 bytes, so addr starts
// at byte 4 after the errno field.
let addr_ptr_i8 = unsafe {
self.builder
.build_gep(
Expand Down Expand Up @@ -1099,6 +1119,8 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> {
.map_err(|e| CodeGenError::LLVMError(e.to_string()))?;
}
if eff_max_len as usize >= DYNAMIC_READ_ERROR_PAYLOAD_LEN {
// SAFETY: eff_max_len is at least the 12-byte read-error
// payload, so addr starts at byte 4 after errno.
let addr_ptr_i8 = unsafe {
self.builder
.build_gep(
Expand Down Expand Up @@ -1281,6 +1303,8 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> {
&format!("expr_byte_{i}"),
)
.map_err(|e| CodeGenError::LLVMError(e.to_string()))?;
// SAFETY: i is bounded by n, the immediate payload
// size reserved for this argument.
let byte_ptr = unsafe {
self.builder
.build_gep(
Expand Down Expand Up @@ -1423,6 +1447,8 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> {
CodeGenError::LLVMError(format!("Failed to store errno: {e}"))
})?;
// write addr at [4..12]
// SAFETY: read-error payload reserves 12 bytes, so addr starts
// at byte 4 after the errno field.
let addr_ptr_i8 = unsafe {
self.builder
.build_gep(
Expand Down
Loading
Loading