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 19 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 12 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
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
wmmc88 marked this conversation as resolved.
Show resolved Hide resolved
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";
leon-xd marked this conversation as resolved.
Show resolved Hide resolved

const WDF_FUNCTION_COUNT_DECLARATION_EXTERNAL_SYMBOL: &str =
leon-xd marked this conversation as resolved.
Show resolved Hide resolved
"(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.
leon-xd marked this conversation as resolved.
Show resolved Hide resolved
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
leon-xd marked this conversation as resolved.
Show resolved Hide resolved
/// 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)?;
leon-xd marked this conversation as resolved.
Show resolved Hide resolved
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"))]
leon-xd marked this conversation as resolved.
Show resolved Hide resolved
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"));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Put this inside a module named __private, and have the module be #[doc(hidden)]. I don't like our sys crate having a public api that isn't present in the WDK

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 clear, this doesn't actually make it private. the modules still needs to be pub in order for the proc_macro to access it. This just hides it from the generated docs (and rust-analyzer auto-complete) and makes it clear that its an internal implementation detail.

Similar approaches are used in crates like serde to expose internal-only apis to their proc_macros: https://github.com/serde-rs/serde/blob/930401b0dd58a809fce34da091b8aa3d6083cb33/serde/src/lib.rs#L324

We also use a similar approach in our proc_macro

Loading
Loading