Skip to content

Clippy updates #672

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 43 additions & 16 deletions .github/workflows/dep_rust.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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/[email protected]
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
Expand Down Expand Up @@ -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 }
Expand All @@ -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
Expand Down
13 changes: 13 additions & 0 deletions Justfile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
71 changes: 71 additions & 0 deletions hack/clippy-package-features.sh
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion src/hyperlight_guest/src/guest_handle/handle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
}
Expand Down
6 changes: 3 additions & 3 deletions src/hyperlight_guest/src/guest_handle/host_comm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) };
Expand Down Expand Up @@ -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)
Expand Down
17 changes: 6 additions & 11 deletions src/hyperlight_guest_bin/src/exceptions/handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)]
Expand Down Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions src/hyperlight_guest_bin/src/exceptions/idtr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
3 changes: 2 additions & 1 deletion src/hyperlight_guest_bin/src/guest_function/call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) }
{
Expand Down Expand Up @@ -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)
Expand Down
6 changes: 3 additions & 3 deletions src/hyperlight_guest_bin/src/host_comm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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")
};

Expand Down
2 changes: 1 addition & 1 deletion src/hyperlight_guest_bin/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Loading
Loading