icd: Do not clear loader environment in extension enumeration

[dgkoch]

What

vkEnumerateInstanceExtensionProperties() takes an icd::EnvironmentOverride, but the function it
calls, Global::EnumerateInstanceExtensionProperties(), only reads the cached instance extension list
and never calls into the Vulkan loader. The override therefore protects no downstream call at this site.

This removes it, and replaces it with a comment explaining why it is absent.

Why it matters

EnvironmentOverride clears the loader environment variables for the whole process while it is held, so
the effect is visible to every thread rather than only the calling one. The Vulkan SC loader resolves
drivers from the process environment on each pre-instance call, so a driver scan issued by another
thread during that window can resolve to a different driver than the application selected. Where a
second Vulkan SC driver is installed, an application using the two-call enumeration idiom from multiple
threads can then receive a count from one driver and a list from another.

Background and the full investigation are in Vulkan SC WG issue #304 (Khronos GitLab, members only).

Effect

Measured with the reproducing case from the Vulkan SC CTS, alternating a build of this branch against an
unmodified build of the same commit one run each, so that between-session variation applies to both:

ICD build	runs in which the mismatch was observed
unmodified	12 / 210
this change	0 / 210

This is a reduction in observed incidence, not a guaranteed fix. The four remaining
EnvironmentOverride sites still modify the process environment for the duration of calls that do use
the Vulkan loader, so the window remains open for other call patterns. With no occurrences in 210 runs
the upper bound on the remaining rate at this entry point is roughly 1.4%, which bounds the improvement
rather than demonstrating elimination.

The underlying rate is also unstable: across sessions on the same machine it varied from 8 in 60 runs to
0 in 150 consecutive runs, and running the system under heavy load suppressed it rather than increasing
it. Absolute rates from any single session should not be relied on, and failing to reproduce does not
indicate the window is closed.

Removing the mutex

The override also acquires the environment mutex, so this drops a lock. Every write to
instance_extension_list_ occurs in the Global constructor, which completes at library load before
any exported entry point can run; the member is private, has no accessor, and no other code path writes
it. The reads at this site therefore need no synchronization.

---

Portions of the investigation and testing behind this change were assisted by Claude (Anthropic).

[aqnuep]

Interestingly, this is the right solution for the wrong reason...

It is, indeed, true, that the environment override in the end is unnecessary in EnumerateInstanceExtensionProperties, because we do cache the extension list and don't actually call down the Vulkan loader.

However, the reason behind this happening to solve the problem is more subtle.

First of all, the whole environment override shenanigans is necessary because of the historical bad decision to not use different environment variable names for Vulkan SC vs Vulkan, that we have to live with. Therefore, the Emulation ICD has to temporarily override them for the duration of calling into the Vulkan loader at points where the loader uses these.

The locking inside the Emulation ICD's environment override utility is to ensure that no other API call is invoked simultaneously that may similarly depend on loader environment variables inside the ICD, although it's more of a guard-rail than actual safety, as the environment overrides can still impact other threads calling into the Vulkan SC loader itself (before even reaching the ICD).

Unfortunately, this entire environment override trick relies on treating the loader in a somewhat white-box fashion (having to know where it reads environment variables and whether that is appropriately guarded against simultaneous calls).

As it turns out, we do indeed have an issue...

Let's look at the places where environment overrides are used:

1. vkEnumerateInstanceExtensionProperties does have an environment override (indeed it's unnecessary per the latest code of the ICD), as ICDs and layers might get loaded at this point by the underlying loader which does involve looking up the affected environment variables
2. Module load does have an environment override, particularly because of the up-front instance extension enumeration (see the earlier point)
3. vkCreateInstance does have an environment override, because it's a point where ICDs and layers might get loaded at this point by the underlying loader which does involve looking up the affected environment variables
4. vkEnumeratePhysicalDevices and vkEnumeratePhysicalDeviceGroups have environment overrides due to the device filtering environment variables

Now the loader does take global locks appropriately in case of (3) and (4).

However, interestingly, indeed it does not lock in case of (1), which explains the misbehavior there (I'm not sure if that changed over time or we just did not notice). Nonetheless, that should be fine with this patch applied, as we anyway cache the instance extension list.

The only remaining case is (2). However, both (1) and (2) could be "problematic" because of the same underlying question: why ICD scanning is not protected by a mutex in the loader?

ICD scanning happens in 3 places:

1. In vkCreateInstance (protected by the normal global lock)
2. In vkEnumerateInstanceExtensionProperties as part of loader_preload_icds (protected by a separate preload-specific global lock)
3. In vkEnumerateInstanceExtensionProperties right after loader_preload_icds, without any locking

It is true that the actual ICD scanning itself does not necessarily require a global lock, although I'd argue it's probably not a great idea to scan and load the same drivers from multiple threads simulatenously.

Anyway, it does not look like this change would entirely eliminate the possible "environment race conditions" without appropriate loader collaboration (it may be worth raising the question to the Vulkan loader folks why ICD scanning isn't mutexed in general), but certainly solves some of the problems.

Honestly, probably, the easiest solution would be if the loader would mutex the vkEnumerateInstanceExtensionProperties call (which is the only other entry point that triggers ICDs loads currently).

Although, because this PR's commit description does not really capture the real issue and the code change contains a somewhat misleading and certainly unnecessary comment, I've created a separate PR to address this problem: #50 (I deemed that this is the easier path than iterating with Claude on the commit message / content).