Remove custom alloca in favor of clang builtin (#106)

ludfjig · web-flow · commit 05ad28906bdf · 2025-01-28T09:51:26.000-08:00
Signed-off-by: Ludvig Liljenberg &lt;lliljenberg@microsoft.com&gt;
diff --git a/src/hyperlight_guest/Cargo.toml b/src/hyperlight_guest/Cargo.toml
@@ -13,10 +13,9 @@ Library to build guest applications for hyperlight.
 """
 
 [features]
-default = ["libc", "printf", "alloca"]
+default = ["libc", "printf"]
 libc = [] # compile musl libc
 printf = [] # compile printf
-alloca = [] # compile alloca wrapper
 
 [dependencies]
 anyhow = { version = "1.0.95", default-features = false }
diff --git a/src/hyperlight_guest/build.rs b/src/hyperlight_guest/build.rs
@@ -41,7 +41,6 @@ fn copy_includes<P: AsRef<Path>, Q: AsRef<Path> + std::fmt::Debug>(include_dir:
 
 fn cargo_main() {
     println!("cargo:rerun-if-changed=third_party");
-    println!("cargo:rerun-if-changed=src/alloca");
     println!("cargo:rerun-if-changed=include");
 
     let mut cfg = cc::Build::new();
@@ -64,19 +63,9 @@ fn cargo_main() {
             .include("third_party/musl/arch/x86_64");
     }
 
-    if cfg!(feature = "alloca") {
-        cfg.file("src/alloca/alloca.c")
-            .define("_alloca", "_alloca_wrapper")
-            .flag("-Wno-return-stack-address");
-    }
-
     let is_pe = env::var("CARGO_CFG_WINDOWS").is_ok();
 
-    if cfg!(any(
-        feature = "printf",
-        feature = "libc",
-        feature = "alloca"
-    )) {
+    if cfg!(any(feature = "printf", feature = "libc")) {
         cfg.define("HYPERLIGHT", None); // used in certain musl files for conditional compilation
 
         // silence compiler warnings
diff --git a/src/hyperlight_guest/src/alloca.rs b/src/hyperlight_guest/src/alloca.rs
diff --git a/src/hyperlight_guest/src/alloca/alloca.c b/src/hyperlight_guest/src/alloca/alloca.c
diff --git a/src/hyperlight_guest/src/lib.rs b/src/hyperlight_guest/src/lib.rs
@@ -42,7 +42,6 @@ pub mod host_error;
 pub mod host_function_call;
 pub mod host_functions;
 
-pub mod alloca;
 pub(crate) mod guest_logger;
 pub mod memory;
 pub mod print;
diff --git a/src/hyperlight_host/examples/func_ctx/main.rs b/src/hyperlight_host/examples/func_ctx/main.rs
@@ -49,16 +49,16 @@ fn main() {
 /// if anything failed.
 fn do_calls(mut ctx: MultiUseGuestCallContext) -> Result<MultiUseSandbox> {
     {
-        let res1: i32 = {
+        let res1: String = {
             let rv = ctx.call(
-                "StackAllocate",
+                "Echo",
                 ReturnType::Int,
-                Some(vec![ParameterValue::Int(1)]),
+                Some(vec![ParameterValue::String("hello".to_string())]),
             )?;
             rv.try_into()
         }
-        .map_err(|e| new_error!("failed to get StackAllocate result: {}", e))?;
-        println!("got StackAllocate res: {res1}");
+        .map_err(|e| new_error!("failed to get Echo result: {}", e))?;
+        println!("got Echo res: {res1}");
     }
     {
         let res2: i32 = {
diff --git a/src/hyperlight_host/src/func/call_ctx.rs b/src/hyperlight_host/src/func/call_ctx.rs
@@ -164,10 +164,12 @@ mod tests {
                 thread::spawn(move || {
                     let calls: Vec<TestFuncCall> = vec![
                         TestFuncCall {
-                            func_name: "StackAllocate".to_string(),
-                            ret_type: ReturnType::Int,
-                            params: Some(vec![ParameterValue::Int(i + 1)]),
-                            expected_ret: ReturnValue::Int(i + 1),
+                            func_name: "Echo".to_string(),
+                            ret_type: ReturnType::String,
+                            params: Some(vec![ParameterValue::String(
+                                format!("Hello {}", i).to_string(),
+                            )]),
+                            expected_ret: ReturnValue::String(format!("Hello {}", i).to_string()),
                         },
                         TestFuncCall {
                             func_name: "CallMalloc".to_string(),
diff --git a/src/hyperlight_host/src/sandbox/initialized_multi_use.rs b/src/hyperlight_host/src/sandbox/initialized_multi_use.rs
@@ -289,9 +289,9 @@ mod tests {
 
         for _ in 0..1000 {
             ctx.call(
-                "StackAllocate",
-                ReturnType::Int,
-                Some(vec![ParameterValue::Int(1)]),
+                "Echo",
+                ReturnType::String,
+                Some(vec![ParameterValue::String("hello".to_string())]),
             )
             .unwrap();
         }
diff --git a/src/hyperlight_host/tests/integration_test.rs b/src/hyperlight_host/tests/integration_test.rs
@@ -22,7 +22,7 @@ use hyperlight_host::mem::memory_region::MemoryRegionFlags;
 use hyperlight_host::sandbox::SandboxConfiguration;
 use hyperlight_host::sandbox_state::sandbox::EvolvableSandbox;
 use hyperlight_host::sandbox_state::transition::Noop;
-use hyperlight_host::{GuestBinary, HyperlightError, UninitializedSandbox};
+use hyperlight_host::{GuestBinary, HyperlightError, MultiUseSandbox, UninitializedSandbox};
 use hyperlight_testing::{c_simple_guest_as_string, simple_guest_as_string};
 
 pub mod common; // pub to disable dead_code warning
@@ -266,76 +266,30 @@ fn guest_malloc_abort() {
     ));
 }
 
-// checks that alloca works
+// Tests libc alloca
 #[test]
-fn dynamic_stack_allocate() {
-    let mut sbox = new_uninit().unwrap().evolve(Noop::default()).unwrap();
-
-    let bytes = 10_000; // some low number that can be allocated on stack
-
-    sbox.call_guest_function_by_name(
-        "StackAllocate",
-        ReturnType::Int,
-        Some(vec![ParameterValue::Int(bytes)]),
-    )
-    .unwrap();
-}
-
-// checks alloca fails with stackoverflow for large allocations
-#[test]
-fn dynamic_stack_allocate_overflow() {
-    let mut sbox1 = new_uninit().unwrap().evolve(Noop::default()).unwrap();
-
-    // zero is handled as special case in guest,
-    // will turn DEFAULT_GUEST_STACK_SIZE + 1
-    let bytes = 0;
-
-    let res = sbox1
-        .call_guest_function_by_name(
-            "StackAllocate",
-            ReturnType::Int,
-            Some(vec![ParameterValue::Int(bytes)]),
-        )
-        .unwrap_err();
-    println!("{:?}", res);
-    assert!(matches!(res, HyperlightError::StackOverflow()));
-}
-
-// checks alloca fails with overflow when stack pointer overflows
-#[test]
-fn dynamic_stack_allocate_pointer_overflow() {
-    let mut sbox1 = new_uninit_rust().unwrap().evolve(Noop::default()).unwrap();
-    let bytes = 10 * 1024 * 1024; // 10Mb
+fn dynamic_stack_allocate_c_guest() {
+    let path = c_simple_guest_as_string().unwrap();
+    let guest_path = GuestBinary::FilePath(path);
+    let uninit = UninitializedSandbox::new(guest_path, None, None, None);
+    let mut sbox1: MultiUseSandbox = uninit.unwrap().evolve(Noop::default()).unwrap();
 
-    let res = sbox1
+    let res2 = sbox1
         .call_guest_function_by_name(
             "StackAllocate",
             ReturnType::Int,
-            Some(vec![ParameterValue::Int(bytes)]),
+            Some(vec![ParameterValue::Int(100)]),
         )
-        .unwrap_err();
-    println!("{:?}", res);
-    assert!(matches!(res, HyperlightError::StackOverflow()));
-}
-
-// checks alloca fails with stackoverflow for huge allocations with c guest lib
-#[test]
-fn dynamic_stack_allocate_overflow_c_guest() {
-    let path = c_simple_guest_as_string().unwrap();
-    let guest_path = GuestBinary::FilePath(path);
-    let uninit = UninitializedSandbox::new(guest_path, None, None, None);
-    let mut sbox1 = uninit.unwrap().evolve(Noop::default()).unwrap();
-
-    let bytes = 0; // zero is handled as special case in guest, will turn into large number
+        .unwrap();
+    assert!(matches!(res2, ReturnValue::Int(n) if n == 100));
 
     let res = sbox1
         .call_guest_function_by_name(
             "StackAllocate",
             ReturnType::Int,
-            Some(vec![ParameterValue::Int(bytes)]),
+            Some(vec![ParameterValue::Int(128 * 1024 * 1024)]),
         )
         .unwrap_err();
-    println!("{:?}", res);
     assert!(matches!(res, HyperlightError::StackOverflow()));
 }
 
diff --git a/src/tests/c_guests/c_simpleguest/main.c b/src/tests/c_guests/c_simpleguest/main.c
@@ -3,6 +3,7 @@
 // Included from hyperlight_guest/third_party/libc
 #include "stdint.h"
 #include "string.h"
+#include "stdlib.h"
 // Included from hyperlight_guest/third_party/printf
 #include "printf.h"
 
@@ -33,11 +34,7 @@ int print_output(const char *message) {
 
 __attribute__((optnone)) 
 int stack_allocate(int32_t length) {
-  if (length == 0) {
-    length = GUEST_STACK_SIZE + 1;
-  }
-
-  void *buffer = _alloca(length);
+  void *buffer = alloca(length);
   (void)buffer;
 
   return length;
diff --git a/src/tests/rust_guests/simpleguest/src/main.rs b/src/tests/rust_guests/simpleguest/src/main.rs
@@ -46,7 +46,6 @@ use hyperlight_common::flatbuffer_wrappers::util::{
     get_flatbuffer_result_from_void,
 };
 use hyperlight_common::mem::PAGE_SIZE;
-use hyperlight_guest::alloca::_alloca;
 use hyperlight_guest::entrypoint::{abort_with_code, abort_with_code_and_message};
 use hyperlight_guest::error::{HyperlightGuestError, Result};
 use hyperlight_guest::guest_function_definition::GuestFunctionDefinition;
@@ -409,25 +408,6 @@ fn print_eleven_args(function_call: &FunctionCall) -> Result<Vec<u8>> {
     }
 }
 
-fn stack_allocate(function_call: &FunctionCall) -> Result<Vec<u8>> {
-    if let ParameterValue::Int(length) = function_call.parameters.clone().unwrap()[0].clone() {
-        let alloc_length = if length == 0 {
-            DEFAULT_GUEST_STACK_SIZE + 1
-        } else {
-            length
-        };
-
-        _alloca(alloc_length as usize);
-
-        Ok(get_flatbuffer_result_from_int(alloc_length))
-    } else {
-        Err(HyperlightGuestError::new(
-            ErrorCode::GuestFunctionParameterTypeMismatch,
-            "Invalid parameters passed to stack_allocate".to_string(),
-        ))
-    }
-}
-
 fn buffer_overrun(function_call: &FunctionCall) -> Result<Vec<u8>> {
     if let ParameterValue::String(value) = function_call.parameters.clone().unwrap()[0].clone() {
         let c_str = value.as_str();
@@ -749,14 +729,6 @@ pub extern "C" fn hyperlight_main() {
     );
     register_function(print_using_printf_def);
 
-    let stack_allocate_def = GuestFunctionDefinition::new(
-        "StackAllocate".to_string(),
-        Vec::from(&[ParameterType::Int]),
-        ReturnType::Int,
-        stack_allocate as i64,
-    );
-    register_function(stack_allocate_def);
-
     let stack_overflow_def = GuestFunctionDefinition::new(
         "StackOverflow".to_string(),
         Vec::from(&[ParameterType::Int]),

Original file line number	Diff line number	Diff line change
`@@ -49,16 +49,16 @@ fn main() {`
`49`	`49`	`/// if anything failed.`
`50`	`50`	`fn do_calls(mut ctx: MultiUseGuestCallContext) -> Result<MultiUseSandbox> {`
`51`	`51`	`{`
`52`		`- let res1: i32 = {`
	`52`	`+ let res1: String = {`
`53`	`53`	`let rv = ctx.call(`
`54`		`- "StackAllocate",`
	`54`	`+ "Echo",`
`55`	`55`	`ReturnType::Int,`
`56`		`- Some(vec![ParameterValue::Int(1)]),`
	`56`	`+ Some(vec![ParameterValue::String("hello".to_string())]),`
`57`	`57`	`)?;`
`58`	`58`	`rv.try_into()`
`59`	`59`	`}`
`60`		`- .map_err(\|e\| new_error!("failed to get StackAllocate result: {}", e))?;`
`61`		`- println!("got StackAllocate res: {res1}");`
	`60`	`+ .map_err(\|e\| new_error!("failed to get Echo result: {}", e))?;`
	`61`	`+ println!("got Echo res: {res1}");`
`62`	`62`	`}`
`63`	`63`	`{`
`64`	`64`	`let res2: i32 = {`
Original file line number	Diff line number	Diff line change
`@@ -289,9 +289,9 @@ mod tests {`
`289`	`289`
`290`	`290`	`for _ in 0..1000 {`
`291`	`291`	`ctx.call(`
`292`		`- "StackAllocate",`
`293`		`- ReturnType::Int,`
`294`		`- Some(vec![ParameterValue::Int(1)]),`
	`292`	`+ "Echo",`
	`293`	`+ ReturnType::String,`
	`294`	`+ Some(vec![ParameterValue::String("hello".to_string())]),`
`295`	`295`	`)`
`296`	`296`	`.unwrap();`
`297`	`297`	`}`