Improve std::sys::windows::compat

[m-ou-se]

Improves the compat_fn macro in sys::windows, which is used for conditionally loading APIs that might not be available.

• The module (dll) name can now be any string, not just an ident. (Not all Windows api modules are valid Rust identifiers. E.g. WaitOnAddress comes from API-MS-Win-Core-Synch-l1-2-0.dll.)
• Adds FuncName::is_available() for checking if a function is really available without having to do a duplicate lookup.
• Add comment explaining the lack of locking.
• Use $_:block to simplify the macro_rules.
• Apply allow(unused_variables) only to the fallback instead of everything.

---

The second point (is_available()) simplifies code that needs to pick an implementation depening on what is available, like sys/windows/mutex.rs. Before this change, it'd do its own lookup and keep its own AtomicUsize to track the result. Now it can just use c::AcquireSRWLockExclusive::is_available() directly.

This will also be useful when park/unpark/CondVar/etc. get improved implementations (e.g. from parking_lot or something else), as the best APIs for those are not available before Windows 8.

[rust-highfive]

r? @withoutboats

(rust_highfive has picked a reviewer for you, use r? to override)

[dtolnay]

Function pointer equality is not well defined, since we give functions to LLVM as unnamed_addr which means their address is claimed to not be significant to the execution of the program. See #54685. This expression can evaluate to unequal even if lookup failed, and can evaluate to equal even if lookup succeeded. Is there a different way to implement is_available, or is there some reason to believe this wouldn't hit bizarre behavior?

[m-ou-se]

Oh, interesting. Thanks.

If I understand correctly, unnamed_addr allows for merging functions. Meaning that f1 == f2 might give true for different functions, but not that f1 == f1 might give false for the same function. So false positives, but not false negatives. That would be fine here. The functions loaded from GetProcAddress are definitely not merged with fallback (since they come from external dll), and that's the only one that matters. If multiple fallback functions happen to have the same address for example, that's fine.

So the question here is whether 'the address is not significant' means 'not significant but at least constant', or that it may assumef != f.

Disabling unnamed_addr on fallback would also work, but it seems there's no way to do that in Rust.

[m-ou-se]

Ah, @eddyb tells me rustc itself might duplicate functions too. So, in this snippet it could mean that fallback as usize (which is short for fallback as fn() as usize) might result in a copy of fallback. Using a static FALLBACK: fn(..) = fallback; and using that static instead should prevent that. The assumption that that works is already made in core:

rust/library/core/src/fmt/mod.rs

Lines 246 to 267 in 9cba260

	// This guarantees a single stable value for the function pointer associated with
	// indices/counts in the formatting infrastructure.
	//
	// Note that a function defined as such would not be correct as functions are
	// always tagged unnamed_addr with the current lowering to LLVM IR, so their
	// address is not considered important to LLVM and as such the as_usize cast
	// could have been miscompiled. In practice, we never call as_usize on non-usize
	// containing data (as a matter of static generation of the formatting
	// arguments), so this is merely an additional check.
	//
	// We primarily want to ensure that the function pointer at `USIZE_MARKER` has
	// an address corresponding *only* to functions that also take `&usize` as their
	// first argument. The read_volatile here ensures that we can safely ready out a
	// usize from the passed reference and that this address does not point at a
	// non-usize taking function.
	#[unstable(feature = "fmt_internals", reason = "internal to format_args!", issue = "none")]
	static USIZE_MARKER: fn(&usize, &mut Formatter<'_>) -> Result = |ptr, _| {
	// SAFETY: ptr is a reference
	let _v: usize = unsafe { crate::ptr::read_volatile(ptr) };
	loop {}
	};

See also #54685 (comment)

I'll update the PR to do the same.

[m-ou-se]

Rebased and added 63b6007 to fix this issue.

[eddyb]

FWIW this looks fine to me now.

[dtolnay]

@bors r+

[bors]

📌 Commit 63b6007 has been approved by dtolnay