fix: address code review comments

Signed-off-by: Tomasz Andrzejak <andreiltd@gmail.com>

Author: andreiltd

docs/picolibc.md

@@ -0,0 +1,86 @@
+# Picolibc Integration
+
+Hyperlight uses [picolibc](https://github.com/picolibc/picolibc) as its C
+standard library for guest binaries, replacing the previous musl-based
+approach. Picolibc is a lightweight C library designed for embedded systems,
+making it well-suited for Hyperlight's micro-VM environment.
+
+## Overview
+
+The picolibc integration is controlled by the `libc` feature flag on the
+`hyperlight-guest-bin` crate (enabled by default). When enabled, the build
+script compiles picolibc from source using the vendored submodule at
+`src/hyperlight_guest_bin/third_party/picolibc`.
+
+The build uses a sparse checkout to exclude GPL/AGPL-licensed test and script
+files — only BSD/MIT/permissive-licensed source files are included. See
+`NOTICE.txt` for full licensing details.
+
+## Host Function Stubs
+
+When the `libc` feature is enabled, the POSIX stubs in
+`src/hyperlight_guest_bin/src/host_bridge.rs` provide C-compatible
+implementations of `read`, `write`, `clock_gettime`, `gettimeofday`, and
+other functions that picolibc calls internally.
+
+These stubs can optionally delegate to host functions. If the host registers
+these functions, the corresponding libc functionality becomes available to
+guest code. If not registered, the stubs return appropriate errors.
+
+| Host Function   | Parameters             | Return Type | Description |
+|-----------------|------------------------|-------------|-------------|
+| `HostPrint`     | `String`               | `Int`       | Used by the `write()` stub. Only stdout (fd 1) and stderr (fd 2) are supported; both delegate to this single host function. Other file descriptors return `EBADF`. |
+| `HostRead`      | `ULong` (byte count)   | `VecBytes`  | Used by the `read()` stub. Only stdin (fd 0) is supported; other file descriptors return `EBADF`. |
+| `CurrentTime`   | _(none)_               | `VecBytes`  | Used by `clock_gettime()` and `gettimeofday()`. Should return 16 bytes: 8 bytes of seconds + 8 bytes of nanoseconds. If not provided, a monotonic fallback starting at Unix timestamp `1609459200` (2021-01-01) is used. |
+
+## Build Configuration
+
+The build script (`build.rs`) generates a `picolibc.h` configuration header
+that controls which picolibc features are enabled. Key features:
+
+- Single-threaded: no locking or TLS support
+- Global errno: uses a single global `errno` variable
+- Tiny stdio: minimal stdio implementation
+- No malloc: memory allocation is handled by the Rust global allocator
+- IEEE math: math library without errno side effects
+
+For full details on available picolibc build options, see the
+[picolibc build documentation](https://github.com/picolibc/picolibc/blob/main/doc/build.md).
+
+The file list of picolibc sources to compile is maintained in `build_files.rs`.
+
+## Updating Picolibc
+
+To update picolibc to a new version:
+
+1. Update the submodule:
+   ```bash
+   cd src/hyperlight_guest_bin/third_party/picolibc
+   git fetch origin
+   git checkout <new-version-tag>
+   cd ../../../..
+   git add src/hyperlight_guest_bin/third_party/picolibc
+   ```
+
+2. Verify licensing: Check that no new GPL/AGPL-licensed source files
+   have been added to the directories we compile. The sparse checkout
+   (configured in `build.rs` `sparse_checkout()`) excludes `test/`,
+   `scripts/`, and `COPYING.GPL2`, but review any new files.
+
+3. Update `build_files.rs`: Compare the file list against the new
+   version's meson build files. Files may have been added, removed, or
+   renamed. The meson build definitions in `libc/meson.build` and
+   `libm/meson.build` (and their subdirectory `meson.build` files)
+   are the source of truth for which files to compile.
+
+4. Update version strings in `build.rs`: Update the `__PICOLIBC_VERSION__`,
+   `__PICOLIBC__`, `__PICOLIBC_MINOR__`, `__PICOLIBC_PATCHLEVEL__`,
+   `_NEWLIB_VERSION`, and related defines in `gen_config_file()`.
+
+5. Update `NOTICE.txt`: Bump the version number in the picolibc entry.
+
+6. Build and test:
+   ```bash
+   just guests
+   just test
+   ```

src/hyperlight_guest/src/error.rs

@@ -17,9 +17,10 @@ limitations under the License.
 use alloc::format;
 use alloc::string::{String, ToString as _};
 
+use anyhow;
 use hyperlight_common::flatbuffer_wrappers::guest_error::ErrorCode;
 use hyperlight_common::func::Error as FuncError;
-use {anyhow, serde_json};
+use serde_json;
 
 pub type Result<T> = core::result::Result<T, HyperlightGuestError>;
 

src/hyperlight_guest_bin/build.rs

@@ -157,10 +157,6 @@ fn gen_config_file(config_dir: &Path) -> Result<()> {
     let has = if detect_cc_feature(code)? { 1 } else { 0 };
     writeln!(file, "#define __HAVE_COMPLEX {has}")?;
 
-    let code = r#"long long x = 0; int main() { return 0; }"#;
-    let has = if detect_cc_feature(code)? { 1 } else { 0 };
-    writeln!(file, "#define __IO_LONG_LONG {has}")?;
-
     // Static undefs
     writeln!(file, "#undef __ARM_SEMIHOST")?; // -Dsemihost=false
     writeln!(file, "#undef __SEMIHOST")?; // -Dsemihost=false
@@ -294,7 +290,6 @@ fn add_libc(build: &mut cc::Build, picolibc_dir: &Path, target: &str) -> Result<
         build.file(&source_path);
     }
 
-    build.file("c/clock.c");
     Ok(())
 }
 

src/hyperlight_guest_bin/c/clock.c

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-use alloc::string::ToString;
+use alloc::string::String;
 use alloc::vec;
 use alloc::vec::Vec;
 use core::ffi::*;
@@ -34,14 +34,53 @@ fn set_errno(val: c_int) {
 }
 
 // POSIX errno values (matching picolibc sys/errno.h)
+const EINVAL: c_int = 22;
 const EIO: c_int = 5;
 const EBADF: c_int = 9;
 const ENOSYS: c_int = 88;
 
+// picolibc clock IDs (from time.h)
+const CLOCK_REALTIME: c_ulong = 1;
+const CLOCK_MONOTONIC: c_ulong = 4;
+
 static CURRENT_TIME: AtomicU64 = AtomicU64::new(0);
 
+/// Matches picolibc `struct timespec` layout for x86_64.
+#[repr(C)]
+struct Timespec {
+    tv_sec: c_long,
+    tv_nsec: c_long,
+}
+
+/// Matches picolibc `struct timeval` layout for x86_64.
+#[repr(C)]
+struct Timeval {
+    tv_sec: c_long,
+    tv_usec: c_long,
+}
+
+/// Retrieves the current time from the host as (seconds, nanoseconds).
+fn current_time() -> (u64, u64) {
+    let bytes = call_host_function::<Vec<u8>>("CurrentTime", Some(vec![]), ReturnType::VecBytes)
+        .unwrap_or_default();
+
+    if bytes.len() == 16 {
+        let secs = u64::from_ne_bytes(bytes[0..8].try_into().unwrap());
+        let nanos = u64::from_ne_bytes(bytes[8..16].try_into().unwrap());
+        (secs, nanos)
+    } else {
+        let secs = 1609459200 + CURRENT_TIME.fetch_add(1, Ordering::Relaxed);
+        (secs, 0)
+    }
+}
+
 #[unsafe(no_mangle)]
 pub extern "C" fn read(fd: c_int, buf: *mut c_void, count: usize) -> isize {
+    if buf.is_null() && count > 0 {
+        set_errno(EINVAL);
+        return -1;
+    }
+
     if fd != 0 {
         set_errno(EBADF);
         return -1;
@@ -53,7 +92,7 @@ pub extern "C" fn read(fd: c_int, buf: *mut c_void, count: usize) -> isize {
         ReturnType::VecBytes,
     ) {
         Ok(bytes) => {
-            let n = bytes.len();
+            let n = bytes.len().min(count);
             unsafe {
                 core::ptr::copy_nonoverlapping(bytes.as_ptr(), buf as *mut u8, n);
             }
@@ -68,16 +107,21 @@ pub extern "C" fn read(fd: c_int, buf: *mut c_void, count: usize) -> isize {
 
 #[unsafe(no_mangle)]
 pub extern "C" fn write(fd: c_int, buf: *const c_void, count: usize) -> isize {
+    if buf.is_null() && count > 0 {
+        set_errno(EINVAL);
+        return -1;
+    }
+
     if fd != 1 && fd != 2 {
         set_errno(EBADF);
         return -1;
     }
 
     let slice = unsafe { core::slice::from_raw_parts(buf as *const u8, count) };
-    let s = core::str::from_utf8(slice).unwrap_or("<invalid utf8>");
+    let s = String::from_utf8_lossy(slice);
     match call_host_function::<i32>(
         "HostPrint",
-        Some(vec![ParameterValue::String(s.to_string())]),
+        Some(vec![ParameterValue::String(s.into_owned())]),
         ReturnType::Int,
     ) {
         Ok(_) => count as isize,
@@ -90,24 +134,51 @@ pub extern "C" fn write(fd: c_int, buf: *const c_void, count: usize) -> isize {
 
 #[unsafe(no_mangle)]
 pub extern "C" fn _current_time(ts: *mut u64) -> c_int {
-    let bytes = call_host_function::<Vec<u8>>("CurrentTime", Some(vec![]), ReturnType::VecBytes)
-        .unwrap_or_default();
-
-    let (secs, nanos) = if bytes.len() == 16 {
-        let secs = u64::from_ne_bytes(bytes[0..8].try_into().unwrap());
-        let nanos = u64::from_ne_bytes(bytes[8..16].try_into().unwrap());
-        (secs, nanos)
-    } else {
-        let secs = 1609459200 + CURRENT_TIME.fetch_add(1, Ordering::Relaxed);
-        (secs, 0)
-    };
-
+    let (secs, nanos) = current_time();
     let ts = unsafe { core::slice::from_raw_parts_mut(ts, 2) };
     ts[0] = secs;
     ts[1] = nanos;
     0
 }
 
+#[unsafe(no_mangle)]
+pub extern "C" fn clock_gettime(clk_id: c_ulong, tp: *mut Timespec) -> c_int {
+    if tp.is_null() {
+        set_errno(EINVAL);
+        return -1;
+    }
+
+    match clk_id {
+        CLOCK_REALTIME | CLOCK_MONOTONIC => {
+            let (secs, nanos) = current_time();
+            unsafe {
+                (*tp).tv_sec = secs as c_long;
+                (*tp).tv_nsec = nanos as c_long;
+            }
+            0
+        }
+        _ => {
+            set_errno(EINVAL);
+            -1
+        }
+    }
+}
+
+#[unsafe(no_mangle)]
+pub extern "C" fn gettimeofday(tv: *mut Timeval, _tz: *mut c_void) -> c_int {
+    if tv.is_null() {
+        set_errno(EINVAL);
+        return -1;
+    }
+
+    let (secs, nanos) = current_time();
+    unsafe {
+        (*tv).tv_sec = secs as c_long;
+        (*tv).tv_usec = (nanos / 1000) as c_long;
+    }
+    0
+}
+
 #[unsafe(no_mangle)]
 pub extern "C" fn _exit(ec: c_int) -> ! {
     hyperlight_guest::exit::abort_with_code(&[ec as u8]);

src/hyperlight_guest_bin/src/host_bridge.rs

@@ -51,6 +51,7 @@ pub mod host_comm;
 pub mod memory;
 pub mod paging;
 
+/// Bridge between picolibc's POSIX expectations and the Hyperlight host.
 #[cfg(feature = "libc")]
 mod host_bridge;