microsoft / microsoft/litebox

Fix suseconds_t type signedness for Linux ABI compatibility

Open Beginner friendly
#608 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug
Dominant language
Rust
Stars
2.7k
Forks
144
Avg merge
12h 21m
Merged PRs (30d)
146

Description

## Summary

The `suseconds_t` type is defined as unsigned (`u32`/`u64`) but should be signed (`i32`/`i64`) per Linux kernel ABI.

## Location

`litebox_common_linux/src/lib.rs` lines 771-781:

```rust
cfg_if::cfg_if! {
if #[cfg(all(target_arch = "x86"))] {
pub type time_t = i32;
pub type suseconds_t = u32; // <-- Should be i32
} else if #[cfg(all(target_arch = "x86_64"))] {
pub type time_t = i64;
pub type suseconds_t = u64; // <-- Should be i64
} else {
compile_error!("Unsupported architecture");
}
}
```

## Expected Behavior

Per Linux kernel (`include/uapi/asm-generic/posix_types.h`):

```c
typedef __kernel_long_t __kernel_suseconds_t;
```

Where `__kernel_long_t` is `long` (signed). So:
- On x86 (32-bit): `suseconds_t` should be `i32`
- On x86_64 (64-bit): `suseconds_t` should be `i64`

## Impact

This affects the `TimeVal` struct which is used in:
- `Rusage.ru_utime` and `Rusage.ru_stime` (getrusage syscall)
- `ItimerVal` (setitimer/getitimer syscalls)
- Any other syscalls using `TimeVal`

**Current impact is minimal** since:
1. `suseconds_t` values are typically non-negative microseconds (0-999999)
2. Current implementations return zeroed values

However, for strict ABI compatibility and future implementations that populate real time values, this should be fixed.

## Suggested Fix

```rust
cfg_if::cfg_if! {
if #[cfg(all(target_arch = "x86"))] {
pub type time_t = i32;
pub type suseconds_t = i32; // Changed from u32
} else if #[cfg(all(target_arch = "x86_64"))] {
pub type time_t = i64;
pub type suseconds_t = i64; // Changed from u64
} else {
compile_error!("Unsupported architecture");
}
}
```

## Additional Consideration

Consider adding compile-time size assertions for structs that depend on these types:

```rust
#[cfg(target_arch = "x86_64")]
const _: () = assert!(core::mem::size_of::() == 16);
#[cfg(target_arch = "x86")]
const _: () = assert!(core::mem::size_of::() == 8);
```

## Found During

Code review of PR #606 (getrusage syscall implementation).

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

In litebox_common_linux/src/lib.rs lines 771-781, inspect the architecture-specific aliases and the TimeVal definitions and usages. Confirm the aliases use signed types matching the Linux ABI on x86 and x86_64, then check the affected Rusage and ItimerVal layouts, including the proposed size assertions if included. Done when the Rust definitions and dependent layouts are ABI-compatible.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
operating-systems
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
65/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.