Skip to content
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

refactor: Remove lazy static instances #250

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open
18 changes: 4 additions & 14 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,9 @@ cfg-if = "1.0.0"
clap = "4.5.9"
clap-cargo = "0.14.1"
itertools = "0.13.0"
lazy_static = "1.5.0"
once_cell = { version = "1.20.2", default-features = false, features = [
leon-xd marked this conversation as resolved.
Show resolved Hide resolved
"alloc",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm is alloc required here? this forces there to be an allocator pulled in always. packages without them would fail to compile

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be addressed in separate issue: we should add some samples to the top level test that explicitly test this no alloc available case. The current examples all pull in wdk-alloc but samples should work without it still

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think as it stands, this pr breaks the pure no-std case

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@leon-xd this was marked as resolved, but i believe its still the case?

] }
paste = "1.0.15"
pretty_assertions = "1.4.0"
proc-macro2 = "1.0.86"
Expand Down
1 change: 0 additions & 1 deletion crates/wdk-build/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ cargo_metadata.workspace = true
cfg-if.workspace = true
clap = { workspace = true, features = ["derive"] }
clap-cargo.workspace = true
lazy_static.workspace = true
paste.workspace = true
rustversion.workspace = true
serde = { workspace = true, features = ["derive"] }
Expand Down
12 changes: 3 additions & 9 deletions crates/wdk-build/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ mod utils;

mod bindgen;

use std::{env, path::PathBuf};
use std::{env, path::PathBuf, sync::LazyLock};

use cargo_metadata::MetadataCommand;
use serde::{Deserialize, Serialize};
Expand Down Expand Up @@ -981,14 +981,8 @@ pub fn configure_wdk_binary_build() -> Result<(), ConfigError> {
Config::from_env_auto()?.configure_binary_build()
}

// This currently only exports the driver type, but may export more metadata in
leon-xd marked this conversation as resolved.
Show resolved Hide resolved
// the future. `EXPORTED_CFG_SETTINGS` is a mapping of cfg key to allowed cfg
// values
lazy_static::lazy_static! {
// FIXME: replace lazy_static with std::Lazy once available: https://github.com/rust-lang/rust/issues/109736
static ref EXPORTED_CFG_SETTINGS: Vec<(&'static str, Vec<&'static str>)> =
vec![("DRIVER_MODEL-DRIVER_TYPE", vec!["WDM", "KMDF", "UMDF"])];
}
static EXPORTED_CFG_SETTINGS: LazyLock<Vec<(&'static str, Vec<&'static str>)>> =
LazyLock::new(|| vec![("DRIVER_MODEL-DRIVER_TYPE", vec!["WDM", "KMDF", "UMDF"])]);

#[cfg(test)]
mod tests {
Expand Down
6 changes: 3 additions & 3 deletions crates/wdk-sys/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,15 @@ anyhow.workspace = true
bindgen.workspace = true
cargo_metadata.workspace = true
cc.workspace = true
lazy_static.workspace = true
once_cell.workspace = true
serde_json.workspace = true
thiserror.workspace = true
tracing.workspace = true
tracing-subscriber = { workspace = true, features = ["env-filter"] }
wdk-build.workspace = true

[dependencies]
lazy_static = { workspace = true, features = ["spin_no_std"] }
once_cell.workspace = true
rustversion.workspace = true
wdk-macros.workspace = true

Expand Down Expand Up @@ -74,4 +74,4 @@ redundant_explicit_links = "warn"
unescaped_backticks = "warn"

[package.metadata.cargo-machete]
ignored = ["lazy_static"] # lazy_static is used in code generated by build.rs
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

keep the comment

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this entire line is gone -- there's no reference to lazy_static. why do we need to keep the comment?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is still a machete entry for once_cell. I want a comment documenting why machete needs to ignore once_cell (presumably because its only used in generated code that machete doesn't scan , resulting in a false positive).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

without the comment, machete fails, right?

ignored = ["once_cell"]
99 changes: 65 additions & 34 deletions crates/wdk-sys/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@ use std::{
env,
io::Write,
path::{Path, PathBuf},
sync::LazyLock,
thread,
};

use anyhow::Context;
use bindgen::CodegenConfig;
use lazy_static::lazy_static;
use tracing::{info, info_span, Span};
use tracing_subscriber::{
filter::{LevelFilter, ParseError},
Expand Down Expand Up @@ -47,35 +47,64 @@ const WDF_FUNCTION_COUNT_DECLARATION_EXTERNAL_SYMBOL: &str = "
const WDF_FUNCTION_COUNT_DECLARATION_TABLE_INDEX: &str = "
let wdf_function_count = crate::_WDFFUNCENUM::WdfFunctionTableNumEntries as usize;";

// FIXME: replace lazy_static with std::Lazy once available: https://github.com/rust-lang/rust/issues/109736
lazy_static! {
static ref WDF_FUNCTION_TABLE_TEMPLATE: String = format!(
r#"
// FIXME: replace lazy_static with std::Lazy once available: https://github.com/rust-lang/rust/issues/109736
#[cfg(any(driver_model__driver_type = "KMDF", driver_model__driver_type = "UMDF"))]
lazy_static::lazy_static! {{
#[allow(missing_docs)]
pub static ref WDF_FUNCTION_TABLE: &'static [crate::WDFFUNC] = {{
// SAFETY: `WdfFunctions` is generated as a mutable static, but is not supposed to be ever mutated by WDF.
let wdf_function_table = unsafe {{ crate::WdfFunctions }};
{WDF_FUNCTION_COUNT_DECLARATION_PLACEHOLDER}

// SAFETY: This is safe because:
// 1. `WdfFunctions` is valid for reads for `{NUM_WDF_FUNCTIONS_PLACEHOLDER}` * `core::mem::size_of::<WDFFUNC>()`
// bytes, and is guaranteed to be aligned and it must be properly aligned.
// 2. `WdfFunctions` points to `{NUM_WDF_FUNCTIONS_PLACEHOLDER}` consecutive properly initialized values of
// type `WDFFUNC`.
// 3. WDF does not mutate the memory referenced by the returned slice for for its entire `'static' lifetime.
// 4. The total size, `{NUM_WDF_FUNCTIONS_PLACEHOLDER}` * `core::mem::size_of::<WDFFUNC>()`, of the slice must be no
// larger than `isize::MAX`. This is proven by the below `debug_assert!`.
unsafe {{
debug_assert!(isize::try_from(wdf_function_count * core::mem::size_of::<crate::WDFFUNC>()).is_ok());
core::slice::from_raw_parts(wdf_function_table, wdf_function_count)
static WDF_FUNCTION_TABLE_TEMPLATE: LazyLock<String> = LazyLock::new(|| {
format!(
r"
extern crate alloc;

/// Struct to encapsulate a function table.
leon-xd marked this conversation as resolved.
Show resolved Hide resolved
/// Lazily initializes on access via index operation.
pub struct FunctionTable {{
inner: once_cell::race::OnceBox<alloc::boxed::Box<[crate::WDFFUNC]>>
leon-xd marked this conversation as resolved.
Show resolved Hide resolved
}}

impl core::ops::Index<usize> for FunctionTable {{
type Output = crate::WDFFUNC;

fn index(&self, index: usize) -> &Self::Output {{
leon-xd marked this conversation as resolved.
Show resolved Hide resolved
&(**(self.get_function_table()))[index]
}}
}}

impl FunctionTable {{
/// Initializes new FunctionTable.
/// Will not contain data until accessed via index operator.
pub const fn new() -> FunctionTable {{
FunctionTable {{
inner: once_cell::race::OnceBox::new()
}}
}};
}}"#
);
static ref CALL_UNSAFE_WDF_BINDING_TEMPLATE: String = format!(
}}

fn get_function_table(&self) -> &alloc::boxed::Box<[crate::WDFFUNC]> {{
self.inner.get_or_init(|| {{
// SAFETY: `WdfFunctions` is generated as a mutable static, but is not supposed to be ever mutated by WDF.
let wdf_function_table = unsafe {{ crate::WdfFunctions }};
{WDF_FUNCTION_COUNT_DECLARATION_PLACEHOLDER}

// SAFETY: This is safe because:
// 1. `WdfFunctions` is valid for reads for `{NUM_WDF_FUNCTIONS_PLACEHOLDER}` * `core::mem::size_of::<WDFFUNC>()`
// bytes, and is guaranteed to be aligned and it must be properly aligned.
// 2. `WdfFunctions` points to `{NUM_WDF_FUNCTIONS_PLACEHOLDER}` consecutive properly initialized values of
// type `WDFFUNC`.
// 3. WDF does not mutate the memory referenced by the returned slice for for its entire `'static' lifetime.
// 4. The total size, `{NUM_WDF_FUNCTIONS_PLACEHOLDER}` * `core::mem::size_of::<WDFFUNC>()`, of the slice must be no
// larger than `isize::MAX`. This is proven by the below `debug_assert!`.
unsafe {{
debug_assert!(isize::try_from(wdf_function_count * core::mem::size_of::<crate::WDFFUNC>()).is_ok());
alloc::boxed::Box::new(core::slice::from_raw_parts(wdf_function_table, wdf_function_count).into())
}}
}})
}}
}}

/// Static instance of the function table to be used throughout generated code.
pub static WDF_FUNCTION_TABLE: FunctionTable = FunctionTable::new();
"
)
});

static CALL_UNSAFE_WDF_BINDING_TEMPLATE: LazyLock<String> = LazyLock::new(|| {
format!(
r#"
/// A procedural macro that allows WDF functions to be called by name.
///
Expand Down Expand Up @@ -125,18 +154,20 @@ macro_rules! call_unsafe_wdf_function_binding {{
)
}}
}}"#
);
static ref TEST_STUBS_TEMPLATE: String = format!(
)
});

static TEST_STUBS_TEMPLATE: LazyLock<String> = LazyLock::new(|| {
format!(
r"
use crate::WDFFUNC;

/// Stubbed version of the symbol that [`WdfFunctions`] links to so that test targets will compile
#[no_mangle]
pub static mut {WDFFUNCTIONS_SYMBOL_NAME_PLACEHOLDER}: *const WDFFUNC = core::ptr::null();
",
);
}

)
});
type GenerateFn = fn(&Path, &Config) -> Result<(), ConfigError>;

const BINDGEN_FILE_GENERATORS_TUPLES: &[(&str, GenerateFn)] = &[
Expand Down
18 changes: 4 additions & 14 deletions examples/sample-kmdf-driver/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 4 additions & 14 deletions examples/sample-umdf-driver/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading