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
17 changes: 3 additions & 14 deletions Cargo.lock

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

1 change: 0 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ cfg-if = "1.0.0"
clap = "4.5.9"
clap-cargo = "0.14.1"
itertools = "0.13.0"
lazy_static = "1.5.0"
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
15 changes: 6 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,11 @@ 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"])];
}
/// This currently only exports the driver type, but may export more metadata in
/// the future. `EXPORTED_CFG_SETTINGS` is a mapping of cfg key to allowed cfg
/// values
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
17 changes: 16 additions & 1 deletion crates/wdk-macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -229,9 +229,24 @@ impl DerivedASTFragments {
// arguments for the WDF function is safe befause WDF maintains the strict mapping between the
// function table index and the correct function pointer type.
unsafe {
let wdf_function_table = wdk_sys::WdfFunctions;
let wdf_function_count = wdk_sys::wdf::get_wdf_function_count();

// 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 `const_assert!`.

debug_assert!(isize::try_from(wdf_function_count * core::mem::size_of::<wdk_sys::WDFFUNC>()).is_ok());
let wdf_function_table = core::slice::from_raw_parts(wdf_function_table, wdf_function_count);

core::mem::transmute(
// FIXME: investigate why _WDFFUNCENUM does not have a generated type alias without the underscore prefix
wdk_sys::WDF_FUNCTION_TABLE[wdk_sys::_WDFFUNCENUM::#function_table_index as usize],
wdf_function_table[wdk_sys::_WDFFUNCENUM::#function_table_index as usize],
)
}
);
Expand Down
5 changes: 0 additions & 5 deletions crates/wdk-sys/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,13 @@ anyhow.workspace = true
bindgen.workspace = true
cargo_metadata.workspace = true
cc.workspace = true
lazy_static.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"] }
rustversion.workspace = true
wdk-macros.workspace = true

Expand Down Expand Up @@ -72,6 +70,3 @@ missing_crate_level_docs = "warn"
private_intra_doc_links = "warn"
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?

114 changes: 42 additions & 72 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 All @@ -31,51 +31,30 @@ use wdk_build::{
UmdfConfig,
};

const NUM_WDF_FUNCTIONS_PLACEHOLDER: &str =
"<PLACEHOLDER FOR IDENTIFIER FOR VARIABLE CORRESPONDING TO NUMBER OF WDF FUNCTIONS>";
const WDF_FUNCTION_COUNT_DECLARATION_PLACEHOLDER: &str =
"<PLACEHOLDER FOR DECLARATION OF wdf_function_count VARIABLE>";
const OUT_DIR_PLACEHOLDER: &str =
"<PLACEHOLDER FOR LITERAL VALUE CONTAINING OUT_DIR OF wdk-sys CRATE>";
const WDFFUNCTIONS_SYMBOL_NAME_PLACEHOLDER: &str =
"<PLACEHOLDER FOR LITERAL VALUE CONTAINING WDFFUNCTIONS SYMBOL NAME>";

const WDF_FUNCTION_COUNT_DECLARATION_EXTERNAL_SYMBOL: &str = "
// SAFETY: `crate::WdfFunctionCount` is generated as a mutable static, but is not supposed \
to be ever mutated by WDF.
let wdf_function_count = unsafe { crate::WdfFunctionCount } as usize;";
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 ref CALL_UNSAFE_WDF_BINDING_TEMPLATE: String = format!(
const WDF_FUNCTION_COUNT_PLACEHOLDER: &str =
"<PLACEHOLDER FOR EVALUATION CONTAINING WDF_FUNCTION_COUNT VALUE";

const WDF_FUNCTION_COUNT_DECLARATION_EXTERNAL_SYMBOL: &str =
"(unsafe { crate::WdfFunctionCount }) as usize";

const WDF_FUNCTION_COUNT_DECLARATION_TABLE_INDEX: &str =
"crate::_WDFFUNCENUM::WdfFunctionTableNumEntries as usize";

static WDF_FUNCTION_COUNT_FUNCTION_TEMPLATE: LazyLock<String> = LazyLock::new(|| {
format!(
r"/// function to access the value of the number of functions in the WDF function table.
pub fn get_wdf_function_count() -> usize {{
{WDF_FUNCTION_COUNT_PLACEHOLDER}
}}"
)
});

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 +104,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 Expand Up @@ -259,15 +240,14 @@ fn generate_wdf(out_path: &Path, config: &Config) -> Result<(), ConfigError> {
}
}

/// Generates a `wdf_function_table.rs` file in `OUT_DIR` which contains the
/// definition of `WDF_FUNCTION_TABLE`. This is required to be generated here
/// Generates a `wdf_function_count.rs` file in `OUT_DIR` which contains the
/// definition of `WDF_FUNCTION_COUNT`. This is required to be generated here
/// since the size of the table is derived from either a global symbol
/// (`WDF_FUNCTION_COUNT`) that newer WDF versions expose, or an enum that older
/// versions use.
/// that newer WDF versions expose, or an enum that older versions use.
fn generate_wdf_function_table(out_path: &Path, config: &Config) -> std::io::Result<()> {
const MINIMUM_MINOR_VERSION_TO_GENERATE_WDF_FUNCTION_COUNT: u8 = 25;

let generated_file_path = out_path.join("wdf_function_table.rs");
let generated_file_path = out_path.join("wdf_function_count.rs");
let mut generated_file = std::fs::File::create(generated_file_path)?;

let is_wdf_function_count_generated = match *config {
Expand Down Expand Up @@ -304,26 +284,16 @@ fn generate_wdf_function_table(out_path: &Path, config: &Config) -> std::io::Res
}
};

let wdf_function_table_code_snippet = if is_wdf_function_count_generated {
WDF_FUNCTION_TABLE_TEMPLATE
.replace(NUM_WDF_FUNCTIONS_PLACEHOLDER, "crate::WdfFunctionCount")
.replace(
WDF_FUNCTION_COUNT_DECLARATION_PLACEHOLDER,
WDF_FUNCTION_COUNT_DECLARATION_EXTERNAL_SYMBOL,
)
} else {
WDF_FUNCTION_TABLE_TEMPLATE
.replace(
NUM_WDF_FUNCTIONS_PLACEHOLDER,
"crate::_WDFFUNCENUM::WdfFunctionTableNumEntries",
)
.replace(
WDF_FUNCTION_COUNT_DECLARATION_PLACEHOLDER,
WDF_FUNCTION_COUNT_DECLARATION_TABLE_INDEX,
)
};
let wdf_function_table_count_snippet = WDF_FUNCTION_COUNT_FUNCTION_TEMPLATE.replace(
WDF_FUNCTION_COUNT_PLACEHOLDER,
if is_wdf_function_count_generated {
WDF_FUNCTION_COUNT_DECLARATION_EXTERNAL_SYMBOL
} else {
WDF_FUNCTION_COUNT_DECLARATION_TABLE_INDEX
},
);

generated_file.write_all(wdf_function_table_code_snippet.as_bytes())?;
generated_file.write_all(wdf_function_table_count_snippet.as_bytes())?;
Ok(())
}

Expand Down Expand Up @@ -427,7 +397,7 @@ fn main() -> anyhow::Result<()> {
.expect("Scoped Thread should spawn successfully"),
);

info_span!("wdf_function_table.rs generation").in_scope(|| {
info_span!("wdf_function_count.rs generation").in_scope(|| {
generate_wdf_function_table(&out_path, &config)?;
Ok::<(), std::io::Error>(())
})?;
Expand Down
2 changes: 0 additions & 2 deletions crates/wdk-sys/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@

#![no_std]

#[cfg(any(driver_model__driver_type = "KMDF", driver_model__driver_type = "UMDF"))]
pub use wdf::WDF_FUNCTION_TABLE;
#[cfg(any(
driver_model__driver_type = "WDM",
driver_model__driver_type = "KMDF",
Expand Down
2 changes: 1 addition & 1 deletion crates/wdk-sys/src/wdf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,4 @@ mod bindings {
include!(concat!(env!("OUT_DIR"), "/wdf.rs"));
}

include!(concat!(env!("OUT_DIR"), "/wdf_function_table.rs"));
include!(concat!(env!("OUT_DIR"), "/wdf_function_count.rs"));
Loading
Loading