feat(guest): replace musl with picolibc by andreiltd · Pull Request #831 · hyperlight-dev/hyperlight

andreiltd · 2025-08-28T11:48:24Z

No description provided.

jprendes

I love this PR!

jprendes · 2025-08-28T13:04:24Z

src/hyperlight_guest_bin/src/host_bridge.rs

+    )
+    .unwrap_or_default();
+
+    let n = bytes.len();


Do we want to assert n <= count?
A wrongly implemented HostRead could cause a buffer overflow

jprendes · 2025-08-28T13:06:08Z

src/hyperlight_guest_bin/src/host_bridge.rs

+    }
+
+    let slice = unsafe { core::slice::from_raw_parts(buf as *const u8, count) };
+    let s = core::str::from_utf8(slice).unwrap_or("<invalid utf8>");


question: what do you think of using String::from_utf8_lossy instead? we are doing s.to_string() anyway.

Sure! As long as from_utf8_lossy doesn't allocate if string is valid utf-8 which I believe is the case?

It returns a Cow, so, that's correct, but you will need to allocate a string to send it to the host function anyway... so... ¯\_(ツ)_/¯

jprendes · 2025-08-28T13:13:47Z

src/hyperlight_guest_bin/Cargo.toml

 [features]
-default = ["libc", "printf"]
-libc = [] # compile musl libc
-printf = [ "libc" ] # compile printf


opinion: I would just put everything under the same libc feature

Sure, I guess any code from libc if not referenced will be DCE anyway 🤷

jprendes · 2025-08-28T14:02:51Z

src/hyperlight_guest_bin/src/host_bridge.rs

+pub extern "C" fn write(fd: c_int, buf: *const c_void, count: usize) -> isize {
+    // Only stdout (fd=1) and stderr (fd=2)
+    if fd != 1 && fd != 2 {
+        return count as isize;


question: shuoldn't this return -1, or some other indication of error? and set errno?

Right, I think that in order to support this properly we would need to generate the bindings to libc headers and use that to reference errno, EBADF etc. But then, it might be better to just have picolibc-sys crate. Anyway I agree we should report an error here somehow.

jprendes · 2025-08-28T14:05:04Z

src/hyperlight_guest_bin/src/host_bridge.rs

+pub extern "C" fn read(fd: c_int, buf: *mut c_void, count: usize) -> isize {
+    // Only stdin (fd=0) and only if HostRead is defined
+    if fd != 0 || !CAPS.host_read() {
+        return 0;


question: should the case of fd != 0 return -1 and set errno instead?

jsturtevant · 2025-08-28T21:07:02Z

will help with #282

src/hyperlight_host/tests/sandbox_host_tests.rs

raftario · 2025-12-11T17:37:01Z

Is this still on the table ?

jsturtevant · 2025-12-11T20:50:19Z

Yes but need approval on licensing before being able to move forward: cncf/foundation#1117

jprendes · 2026-02-25T10:35:50Z

src/hyperlight_guest_bin/.cargo/config.toml

@@ -1,2 +1,3 @@
 [build]
 target = "x86_64-unknown-none"
+rustflags = ["-Zdirect-access-external-data"]


Where does this come from?

IIRC rustc generated unsupported reallocations without this flag

This is a nightly flag, right?
Also, this will not apply to comsumers of hyperlight-guest-bin, right?

I think it's worth to recheck and try to remove it.

Everything seems to work without the flag, I remove it for now 🤷‍♂️

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

jsturtevant · 2026-03-03T01:22:06Z

src/hyperlight_guest_bin/build.rs

+
+    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}")?;


this gets writeln!(file, "#undef __IO_LONG_LONG")?; // minimal format alittle while later (178). Do we need this here?

jsturtevant · 2026-03-03T01:24:30Z

src/hyperlight_guest_bin/c/clock.c

did we write this file? is there attribution that is needed?

If this is really necessary, I think @jprendes just wrote a Rust implementation for this in hyperlight-js. maybe we could use it?

jsturtevant · 2026-03-03T01:24:44Z

src/hyperlight_guest_bin/c/clock.c

+    (void)__tz;
+
+    _current_time(current_time);
+    tv->tv_sec = current_time[0];


do we need to check for null here?

jsturtevant · 2026-03-03T01:24:53Z

src/hyperlight_guest_bin/c/clock.c

+        case CLOCK_REALTIME:
+        case CLOCK_MONOTONIC:
+            _current_time(current_time);
+            tp->tv_sec = current_time[0];


jsturtevant · 2026-03-03T01:29:47Z

src/hyperlight_guest_bin/src/host_bridge.rs

+        Some(vec![ParameterValue::ULong(count as u64)]),
+        ReturnType::VecBytes,
+    ) {
+        Ok(bytes) => {
+            let n = bytes.len();
+            unsafe {
+                core::ptr::copy_nonoverlapping(bytes.as_ptr(), buf as *mut u8, n);


should not rely on the host provided length. nonoverlapping could write past the buffer lenghth. We are in the guest here so it would just cause corruption but might be tough to track down I think

jsturtevant · 2026-03-03T01:35:47Z

src/hyperlight_guest_bin/src/host_bridge.rs

+        return -1;
+    }
+
+    let slice = unsafe { core::slice::from_raw_parts(buf as *const u8, count) };


do we need a null check on the buf here? https://doc.rust-lang.org/core/slice/fn.from_raw_parts.html#safety

data must be non-null and aligned even for zero-length slices

jsturtevant · 2026-03-03T01:44:43Z

NOTICE.txt

-obstacle to adoption, that text has been removed.
+Note: The picolibc submodule uses sparse checkout to exclude
+GPL/AGPL-licensed test and script files that are not needed for
+building. Only BSD/MIT/permissive-licensed source files are included.


Just wanted to surface this. We should include it in the PR description I think

dblnz

Great work, Tomasz!! This is one hell of a diff and I like it 😆

dblnz · 2026-03-04T08:16:28Z

src/hyperlight_guest_bin/c/clock.c

If this is really necessary, I think @jprendes just wrote a Rust implementation for this in hyperlight-js. maybe we could use it?

dblnz · 2026-03-04T08:28:21Z

src/hyperlight_guest_bin/src/lib.rs

 pub mod paging;

+#[cfg(feature = "libc")]
+mod host_bridge;


Is there a place we document or mention the functions needed to be implemented by the host for the guest to have libc work?
I am referring to HostRead and the others from host_bridge.rs

dblnz · 2026-03-04T08:41:49Z

src/hyperlight_guest_bin/build.rs

+        "#
+        );
+
+        let has = if detect_cc_feature(&code)? { 1 } else { 0 };


These here are quite a lot of invocations of clang. Do you think this can be done in a single call? Something like printing an integer for each builtin and then parsing the output to identify which is supported.

dblnz · 2026-03-04T08:47:16Z

src/hyperlight_guest_bin/build.rs

+        .flag("-fno-stack-protector")
+        .flag("-fstack-clash-protection")
+        .flag("-mstack-probe-size=4096")
+        // We don't use a different stack for all interrupts, so there


I think we have a different stack for the Exceptions.

https://github.com/hyperlight-dev/hyperlight/blob/main/src/hyperlight_guest_bin/src/arch/amd64/init.rs#L95

dblnz · 2026-03-04T08:54:32Z

src/hyperlight_guest_bin/build_files.rs

How do you see the process of updating to a new picolibc version?
Checkout the new version, verify copyright headers, check the checked out files against the ones in this file, update this file in case files were removed or added?

Maybe it's worth mentioning in the readme, to have some instructions

dblnz · 2026-03-04T08:58:10Z

Justfile


 ensure-cargo-hyperlight:
-    command -v cargo-hyperlight >/dev/null 2>&1 || cargo install --locked cargo-hyperlight
+    cargo install --locked --force --git https://github.com/hyperlight-dev/cargo-hyperlight --branch picolibc cargo-hyperlight


This changes to the "official version of the tool" before merging this PR, right?

andreiltd requested review from danbugs, dblnz, devigned, jprendes, ludfjig, marosset, simongdavies and syntactically as code owners August 28, 2025 11:48

andreiltd marked this pull request as draft August 28, 2025 11:48

andreiltd force-pushed the libc-takeover branch from 617d834 to 3f8ffcf Compare August 28, 2025 12:01

andreiltd added the kind/enhancement For PRs adding features, improving functionality, docs, tests, etc. label Aug 28, 2025

andreiltd force-pushed the libc-takeover branch 6 times, most recently from eaf1d54 to b18a374 Compare August 28, 2025 13:44

jprendes reviewed Aug 28, 2025

View reviewed changes

jsturtevant mentioned this pull request Aug 28, 2025

[License Exception Request] [Hyperlight] [Older BSD like licenses] cncf/foundation#1117

Open

12 tasks

andreiltd force-pushed the libc-takeover branch 5 times, most recently from 499c59d to 2ae0a3a Compare August 29, 2025 18:42

jprendes reviewed Sep 4, 2025

View reviewed changes

src/hyperlight_host/tests/sandbox_host_tests.rs Outdated Show resolved Hide resolved

andreiltd force-pushed the libc-takeover branch from 2ae0a3a to d3826af Compare September 4, 2025 11:29

andreiltd mentioned this pull request Sep 5, 2025

Use mimalloc as a guest allocator #867

Draft

andreiltd force-pushed the libc-takeover branch from d2043ac to 2bd1ff4 Compare September 9, 2025 09:31

andreiltd force-pushed the libc-takeover branch 5 times, most recently from 9f5c6c1 to a59c7ce Compare September 9, 2025 12:19

andreiltd force-pushed the libc-takeover branch 2 times, most recently from 65634a7 to 980cb20 Compare February 25, 2026 10:31

jprendes reviewed Feb 25, 2026

View reviewed changes

andreiltd force-pushed the libc-takeover branch 2 times, most recently from c24ce45 to d9fb9e5 Compare February 25, 2026 11:33

feat(guest): replace musl with picolibc

16db912

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

andreiltd force-pushed the libc-takeover branch 2 times, most recently from e4caabe to 7b99307 Compare February 25, 2026 17:59

bump picolibc to 1.8.11

fb1549f

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

andreiltd force-pushed the libc-takeover branch from 7b99307 to fb1549f Compare February 25, 2026 18:12

andreiltd marked this pull request as ready for review February 26, 2026 20:11

jsturtevant reviewed Mar 3, 2026

View reviewed changes

dblnz reviewed Mar 4, 2026

View reviewed changes

Conversation

andreiltd commented Aug 28, 2025

Uh oh!

jprendes left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jsturtevant commented Aug 28, 2025

Uh oh!

Uh oh!

raftario commented Dec 11, 2025

Uh oh!

jsturtevant commented Dec 11, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dblnz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development