Clippy updates (#672)

* Run a more exhasutive clippy check

Signed-off-by: Mark Rossett <marosset@microsoft.com>

* Updating dep_rust.yaml to run clippy and other code checks as a seperate
job

Signed-off-by: Mark Rossett <marosset@microsoft.com>

* Fix clippy warnings for hyperlight-guest

Signed-off-by: Mark Rossett <marosset@microsoft.com>

* More clippy fixes for hyperlight_guest_bin

Signed-off-by: Mark Rossett <marosset@microsoft.com>

---------

Signed-off-by: Mark Rossett <marosset@microsoft.com>

Author: marosset

.github/workflows/dep_rust.yml

@@ -31,6 +31,49 @@ defaults:
     shell: bash
 
 jobs:
+  code-checks:
+    if: ${{ inputs.docs_only == 'false' }}
+    timeout-minutes: 60
+    strategy:
+      fail-fast: true
+      matrix:
+        hypervisor: ['hyperv-ws2025', kvm]
+        config: [debug, release]
+    runs-on: ${{ fromJson(
+      format('["self-hosted", "{0}", "X64", "1ES.Pool=hld-{1}-amd"]', 
+        (matrix.hypervisor == 'hyperv-ws2025') && 'Windows' || 'Linux', 
+        matrix.hypervisor == 'hyperv-ws2025' && 'win2025' || 'kvm')) }}
+    steps:
+      - uses: actions/checkout@v4
+
+      - uses: hyperlight-dev/ci-setup-workflow@v1.5.0
+        with:
+          rust-toolchain: "1.85" 
+        env:
+          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+
+      # Does not check for updated Cargo.lock files for test rust guests as this causes an issue with this checkwhen deoendabot updates dependencies in common crates
+      - name: Ensure up-to-date Cargo.lock
+        run: |
+          cargo fetch --locked
+
+      - name: fmt
+        run: just fmt-check
+
+      - name: clippy
+        if: ${{ (runner.os == 'Windows' )}}
+        run: |
+          just clippy ${{ matrix.config }}
+          just clippy-guests ${{ matrix.config }}
+
+      - name: clippy exhaustive check
+        if: ${{ (runner.os == 'Linux' )}}
+        run: |
+          just clippy-exhaustive ${{ matrix.config }}
+
+      - name: Verify MSRV
+        run: ./dev/verify-msrv.sh hyperlight-host hyperlight-guest hyperlight-guest-bin hyperlight-common
+
   build:
     if: ${{ inputs.docs_only == 'false' }}
     timeout-minutes: 60
@@ -61,19 +104,6 @@ jobs:
         env:
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
 
-      - name: fmt
-        run: just fmt-check
-
-      - name: clippy
-        run: |
-          just clippy ${{ matrix.config }}
-          just clippy-guests ${{ matrix.config }}
-
-      # Does not check for updated Cargo.lock files for test rust guests as this causes an issue with this checkwhen deoendabot updates dependencies in common crates
-      - name: Ensure up-to-date Cargo.lock
-        run: |
-          cargo fetch --locked
-
       - name: Get gh action service name
         if: ${{ (runner.os == 'Windows' )}}
         run: (Get-Service actions.runner.*) | Foreach { $_.Name, $_.UserName, $_.ServiceType }
@@ -93,9 +123,6 @@ jobs:
       - name: Build
         run: just build ${{ matrix.config }}
 
-      - name: Verify MSRV
-        run: ./dev/verify-msrv.sh hyperlight-host hyperlight-guest hyperlight-guest-bin hyperlight-common
-
       - name: Run Rust tests
         env:
           CARGO_TERM_COLOR: always

Justfile

@@ -176,6 +176,19 @@ clippy-apply-fix-unix:
 clippy-apply-fix-windows:
     cargo clippy --target x86_64-pc-windows-msvc --fix --all 
 
+# Run clippy with feature combinations for all packages
+clippy-exhaustive target=default-target: (witguest-wit)
+    ./hack/clippy-package-features.sh hyperlight-host {{ target }}
+    ./hack/clippy-package-features.sh hyperlight-guest {{ target }}
+    ./hack/clippy-package-features.sh hyperlight-guest-bin {{ target }}
+    ./hack/clippy-package-features.sh hyperlight-common {{ target }}
+    ./hack/clippy-package-features.sh hyperlight-testing {{ target }}
+    just clippy-guests {{ target }}
+
+# Test a specific package with all feature combinations
+clippy-package package target=default-target: (witguest-wit)
+    ./hack/clippy-package-features.sh {{ package }} {{ target }}
+
 # Verify Minimum Supported Rust Version
 verify-msrv:
     ./dev/verify-msrv.sh hyperlight-host hyperlight-guest hyperlight-guest-lib hyperlight-common

hack/clippy-package-features.sh

@@ -0,0 +1,71 @@
+#!/usr/bin/env bash
+
+set -euo pipefail
+
+# Check for required arguments
+if [[ $# -lt 2 ]]; then
+    echo "Usage: $0 <package> <target>" >&2
+    echo "Example: $0 hyperlight-host debug" >&2
+    exit 1
+fi
+
+PACKAGE="$1"
+TARGET="$2"
+
+# Convert target for cargo profile
+PROFILE=$([ "$TARGET" = "debug" ] && echo "dev" || echo "$TARGET")
+
+# Required features needed so the rust packages can compile
+if [[ "$PACKAGE" == "hyperlight-host" ]]; then
+    REQUIRED_FEATURES=("kvm" "mshv3")
+elif [[ "$PACKAGE" == "hyperlight-guest-bin" ]]; then
+    REQUIRED_FEATURES=("printf")
+else 
+    REQUIRED_FEATURES=()
+fi
+
+# Get all features for the package (excluding default and required features)
+# Always exclude "default", and exclude any required features using jq
+features=$(cargo metadata --format-version 1 --no-deps | jq -r --arg pkg "$PACKAGE" '.packages[] | select(.name == $pkg) | .features | keys[] | select(. != "default" and (IN($ARGS.positional[])|not))' --args "${REQUIRED_FEATURES[@]}" || true)
+
+# Convert required features array to comma-separated string for cargo
+if [[ ${#REQUIRED_FEATURES[@]} -gt 0 ]]; then
+    required_features_str=$(IFS=,; echo "${REQUIRED_FEATURES[*]}")
+else
+    required_features_str=""
+fi
+
+# Test with minimal features
+if [[ ${#REQUIRED_FEATURES[@]} -gt 0 ]]; then
+    echo "Testing $PACKAGE with required features only ($required_features_str)..."
+    (set -x; cargo clippy -p "$PACKAGE" --all-targets --no-default-features --features "$required_features_str" --profile="$PROFILE" -- -D warnings)
+else
+    echo "Testing $PACKAGE with no features..."
+    (set -x; cargo clippy -p "$PACKAGE" --all-targets --no-default-features --profile="$PROFILE" -- -D warnings)
+fi
+
+echo "Testing $PACKAGE with default features..."
+(set -x; cargo clippy -p "$PACKAGE" --all-targets --profile="$PROFILE" -- -D warnings)
+
+# Test each additional feature individually
+for feature in $features; do
+    if [[ ${#REQUIRED_FEATURES[@]} -gt 0 ]]; then
+        echo "Testing $PACKAGE with feature: $required_features_str,$feature"
+        (set -x; cargo clippy -p "$PACKAGE" --all-targets --no-default-features --features "$required_features_str,$feature" --profile="$PROFILE" -- -D warnings)
+    else
+        echo "Testing $PACKAGE with feature: $feature"
+        (set -x; cargo clippy -p "$PACKAGE" --all-targets --no-default-features --features "$feature" --profile="$PROFILE" -- -D warnings)
+    fi
+done
+
+# Test all features together
+if [[ -n "$features" ]]; then
+    all_features=$(echo $features | tr '\n' ',' | sed 's/,$//')
+    if [[ ${#REQUIRED_FEATURES[@]} -gt 0 ]]; then
+        echo "Testing $PACKAGE with all features: $required_features_str,$all_features"
+        (set -x; cargo clippy -p "$PACKAGE" --all-targets --no-default-features --features "$required_features_str,$all_features" --profile="$PROFILE" -- -D warnings)
+    else
+        echo "Testing $PACKAGE with all features: $all_features"
+        (set -x; cargo clippy -p "$PACKAGE" --all-targets --no-default-features --features "$all_features" --profile="$PROFILE" -- -D warnings)
+    fi
+fi

src/hyperlight_guest/src/guest_handle/handle.rs

@@ -25,7 +25,7 @@ use hyperlight_common::mem::HyperlightPEB;
 ///
 /// Guests are expected to initialize this and store it. For example, you
 /// could store it in a global variable.
-#[derive(Debug, Clone, Copy)]
+#[derive(Debug, Clone, Copy, Default)]
 pub struct GuestHandle {
     peb: Option<*mut HyperlightPEB>,
 }

src/hyperlight_guest/src/guest_handle/host_comm.rs

@@ -41,13 +41,13 @@ impl GuestHandle {
         let user_memory_region_size = unsafe { (*peb_ptr).init_data.size };
 
         if num > user_memory_region_size {
-            return Err(HyperlightGuestError::new(
+            Err(HyperlightGuestError::new(
                 ErrorCode::GuestError,
                 format!(
                     "Requested {} bytes from user memory, but only {} bytes are available",
                     num, user_memory_region_size
                 ),
-            ));
+            ))
         } else {
             let user_memory_region_slice =
                 unsafe { core::slice::from_raw_parts(user_memory_region_ptr, num as usize) };
@@ -142,7 +142,7 @@ impl GuestHandle {
     /// Write an error to the shared output data buffer.
     pub fn write_error(&self, error_code: ErrorCode, message: Option<&str>) {
         let guest_error: GuestError = GuestError::new(
-            error_code.clone(),
+            error_code,
             message.map_or("".to_string(), |m| m.to_string()),
         );
         let guest_error_buffer: Vec<u8> = (&guest_error)

src/hyperlight_guest_bin/src/exceptions/handler.rs

@@ -21,8 +21,6 @@ use hyperlight_common::flatbuffer_wrappers::guest_error::ErrorCode;
 use hyperlight_common::outb::Exception;
 use hyperlight_guest::exit::abort_with_code_and_message;
 
-use crate::paging;
-
 /// See AMD64 Architecture Programmer's Manual, Volume 2
 ///     §8.9.3 Interrupt Stack Frame, pp. 283--284
 ///       Figure 8-14: Long-Mode Stack After Interrupt---Same Privilege,
@@ -56,9 +54,9 @@ const _: () = assert!(size_of::<Context>() == 152 + 512);
 
 // TODO: This will eventually need to end up in a per-thread context,
 // when there are threads.
-pub static handlers: [core::sync::atomic::AtomicU64; 31] =
+pub static HANDLERS: [core::sync::atomic::AtomicU64; 31] =
     [const { core::sync::atomic::AtomicU64::new(0) }; 31];
-type handler_t = fn(n: u64, info: *mut ExceptionInfo, ctx: *mut Context, pf_addr: u64) -> bool;
+pub type HandlerT = fn(n: u64, info: *mut ExceptionInfo, ctx: *mut Context, pf_addr: u64) -> bool;
 
 /// Exception handler
 #[unsafe(no_mangle)]
@@ -89,15 +87,12 @@ pub extern "C" fn hl_exception_handler(
     // vectors (0-31)
     if exception_number < 31 {
         let handler =
-            handlers[exception_number as usize].load(core::sync::atomic::Ordering::Acquire);
+            HANDLERS[exception_number as usize].load(core::sync::atomic::Ordering::Acquire);
         if handler != 0
             && unsafe {
-                core::mem::transmute::<_, handler_t>(handler)(
-                    exception_number,
-                    exn_info,
-                    ctx,
-                    page_fault_address,
-                )
+                core::mem::transmute::<u64, fn(u64, *mut ExceptionInfo, *mut Context, u64) -> bool>(
+                    handler,
+                )(exception_number, exn_info, ctx, page_fault_address)
             }
         {
             return;

src/hyperlight_guest_bin/src/exceptions/idtr.rs

@@ -49,6 +49,7 @@ pub(crate) unsafe fn load_idt() {
 
         // Use &raw mut to get a mutable raw pointer, then dereference it
         // this is to avoid the clippy warning "shared reference to mutable static"
+        #[allow(clippy::deref_addrof)]
         let idtr = &mut *(&raw mut IDTR);
         idtr.init(expected_base, idt_size as u16);
         idtr.load();

src/hyperlight_guest_bin/src/guest_function/call.rs

@@ -42,6 +42,7 @@ pub(crate) fn call_guest_function(function_call: FunctionCall) -> Result<Vec<u8>
     // Find the function definition for the function call.
     // Use &raw const to get an immutable reference to the static HashMap
     // this is to avoid the clippy warning "shared reference to mutable static"
+    #[allow(clippy::deref_addrof)]
     if let Some(registered_function_definition) =
         unsafe { (*(&raw const REGISTERED_GUEST_FUNCTIONS)).get(&function_call.function_name) }
     {
@@ -90,7 +91,7 @@ fn internal_dispatch_function() -> Result<()> {
         .expect("Function call deserialization failed");
 
     let result_vec = call_guest_function(function_call).inspect_err(|e| {
-        handle.write_error(e.kind.clone(), Some(e.message.as_str()));
+        handle.write_error(e.kind, Some(e.message.as_str()));
     })?;
 
     handle.push_shared_output_data(result_vec)

src/hyperlight_guest_bin/src/host_comm.rs

@@ -101,7 +101,7 @@ pub fn print_output_with_host_print(function_call: &FunctionCall) -> Result<Vec<
 pub unsafe extern "C" fn _putchar(c: c_char) {
     let handle = unsafe { GUEST_HANDLE };
     let char = c as u8;
-    let mut message_buffer = unsafe { &mut MESSAGE_BUFFER };
+    let message_buffer = unsafe { &mut MESSAGE_BUFFER };
 
     // Extend buffer capacity if it's empty (like `with_capacity` in lazy_static).
     // TODO: replace above Vec::new() with Vec::with_capacity once it's stable in const contexts.
@@ -113,12 +113,12 @@ pub unsafe extern "C" fn _putchar(c: c_char) {
 
     if message_buffer.len() == BUFFER_SIZE || char == b'\0' {
         let str = if char == b'\0' {
-            CStr::from_bytes_until_nul(&message_buffer)
+            CStr::from_bytes_until_nul(message_buffer)
                 .expect("No null byte in buffer")
                 .to_string_lossy()
                 .into_owned()
         } else {
-            String::from_utf8(mem::take(&mut message_buffer))
+            String::from_utf8(mem::take(message_buffer))
                 .expect("Failed to convert buffer to string")
         };
 

src/hyperlight_guest_bin/src/lib.rs

@@ -104,7 +104,7 @@ pub extern "C" fn entrypoint(peb_address: u64, seed: u64, ops: u64, max_log_leve
             #[allow(static_mut_refs)]
             let peb_ptr = GUEST_HANDLE.peb().unwrap();
 
-            let srand_seed = ((peb_address << 8 ^ seed >> 4) >> 32) as u32;
+            let srand_seed = (((peb_address << 8) ^ (seed >> 4)) >> 32) as u32;
 
             // Set the seed for the random number generator for C code using rand;
             srand(srand_seed);