diff --git a/.github/skills/cswin32-com/SKILL.md b/.github/skills/cswin32-com/SKILL.md index 72b7fd71c7c..e183a25fe7c 100644 --- a/.github/skills/cswin32-com/SKILL.md +++ b/.github/skills/cswin32-com/SKILL.md @@ -8,257 +8,194 @@ argument-hint: 'Describe the COM interface or activation pattern you are working Struct-based COM interop using CsWin32 patterns — AOT-compatible, no `[ComImport]` or built-in marshalling. -**Paired skill:** [cswin32-interop](../cswin32-interop/SKILL.md) covers general P/Invoke (`[DllImport]` migration, `FEATURE_WINDOWSINTEROP` gating, blittable signature rules that apply to both `[DllImport]` and COM vtables, `BufferScope`, source-build verification). This file covers only the COM-specific layer on top. +**Paired skill:** [cswin32-interop](../cswin32-interop/SKILL.md) covers general P/Invoke and the blittable signature rules that apply to both `[DllImport]` and COM vtables. This file covers only the COM-specific layer. ## Workflow -1. **Determine if the interface is in Win32 metadata.** If yes, add the name to `src/Framework/NativeMethods.txt` — CsWin32 generates it. If no (e.g. WMI), define a manual struct (see below). -2. **Create a `ComScope`** for lifetime management: `using ComScope scope = new();` -3. **Activate the COM object** via `ComClassFactory.TryCreate(CLSID, ...)` or `PInvoke.CoCreateInstance` with `IID.Get()`. -4. **Call methods** via `scope.Pointer->Method(...)`. Pass `ComScope` directly as `T**` output parameters. -5. **Guard with `#if FEATURE_WINDOWSINTEROP`** — add `&& NET` only when the struct uses - the static-abstract `IComIID` form *exclusively*. The dual-target pattern below works - on net472 without `&& NET`. - -## COM Interfaces in Win32 Metadata - -Add the interface name to `src/Framework/NativeMethods.txt` → CsWin32 generates it → use `ComScope`: - -```csharp -#if FEATURE_WINDOWSINTEROP -using ComScope rot = new(); -HRESULT hr = PInvoke.GetRunningObjectTable(0, rot); -if (hr.Failed) return; -rot.Pointer->SomeMethod(...); -#endif -``` +1. **Interface in Win32 metadata?** Add the name to `src/Framework/NativeMethods.txt` → CsWin32 generates it. Not in metadata (WMI, Fusion, Setup Configuration) → define a manual struct under its own folder, excluded from source builds via ``. +2. **Lifetime: `using ComScope scope = new();`** for every transient COM pointer (`CoCreateInstance`, `QueryInterface`, `IEnumXxx::Next`, factory output, app-local `STDAPI Get*`, etc.). Never write `T* local; try { ... } finally { local->Release(); }` — that's the pre-`ComScope` shape that leaks on every early return. Same for `BSTR` out-params: `using BSTR x = default;`. +3. **Activate** via `ComClassFactory.TryCreate(CLSID, ...)` (AOT-compatible) or `PInvoke.CoCreateInstance` with `IID.Get()` — not `&localGuid`. +4. **Call** via `scope.Pointer->Method(...)`. Pass `ComScope` directly where the API expects `T**` / `void**` — the implicit operator does the address-of. +5. **Match the caller's error contract.** If the top-level consumer swallows COM failure ("no result" == "absent"), helpers return `default` / `false` instead of throwing a `COMException` that's immediately discarded. `ThrowOnFailure` only when the exception will actually propagate or assert a should-never-happen. +6. **Guard with `#if FEATURE_WINDOWSINTEROP`** — add `&& NET` only when the struct uses the static-abstract `IComIID` form exclusively (no net472 dual-target). ## Manual COM Structs (Not in Metadata) -For interfaces not in Win32 metadata (e.g. WMI, Fusion), define struct-based implementations in their own files, excluded via `` in source builds. Guard with `#if FEATURE_WINDOWSINTEROP && NET` when the struct uses **only** the static-abstract `IComIID` member (only emitted on .NET 7+). If the struct needs to compile on net472, provide **both** the static-abstract member (gated `#if NET`) and an instance member (gated `#else`) so the right one binds against the per-target `IComIID` shape — see Fusion structs in `src/Tasks/AssemblyDependency/Fusion/` for this pattern. +Define each interface in its own file under e.g. `src/Tasks/AssemblyDependency/Fusion/`, `src/Framework/Shared/VisualStudio/`, `src/Framework/Utilities/Wmi/`. Exclude from source builds. Pattern (dual-target — works on net472 + .NET): ```csharp -[SupportedOSPlatform("windows6.1")] -internal unsafe struct IWbemLocator : IComIID +internal unsafe struct IAssemblyCache : IComIID { - public static Guid Guid { get; } = new(0xDC12A687, ...); + public static readonly Guid IID_IAssemblyCache = new(0xE707DCDE, ...); - // .NET 7+ static abstract IComIID implementation +#if NET static ref readonly Guid IComIID.Guid { [MethodImpl(MethodImplOptions.AggressiveInlining)] - get - { - ReadOnlySpan data = [ /* 16 GUID bytes */ ]; - return ref Unsafe.As(ref MemoryMarshal.GetReference(data)); - } + get => ref Unsafe.AsRef(in IID_IAssemblyCache); } +#else + readonly Guid IComIID.Guid => IID_IAssemblyCache; +#endif private readonly void** _lpVtbl; - // IUnknown (vtable 0-2) + interface methods at correct indices - public HRESULT ConnectServer(char* strNetworkResource, ...) { - fixed (IWbemLocator* pThis = &this) - return ((delegate* unmanaged[Stdcall])_lpVtbl[3])(pThis, ...); + // IUnknown at indices 0-2, then interface methods at 3+ + public HRESULT QueryInterface(Guid* riid, void** ppvObject) + { + fixed (IAssemblyCache* pThis = &this) + return ((delegate* unmanaged[Stdcall])_lpVtbl[0])(pThis, riid, ppvObject); } - - public static Guid CLSID { get; } = new(0x4590F811, ...); + // AddRef @1, Release @2, then interface methods at correct slot indices. } ``` -**Requirements:** -- `delegate* unmanaged[Stdcall]` for the function-pointer cast -- Static-abstract `IComIID` on .NET 7+ (gate manual structs with `#if NET`); the net472 polyfill is instance-based and is **not** attached to CsWin32-generated structs, so `ComScope` over generated COM types is .NET-only -- Use the CsWin32-generated `PCWSTR` / `PWSTR` for wide string parameters (add the type to `NativeMethods.txt`); raw `char*` only when no typed equivalent exists -- CS0592 prevents `[SupportedOSPlatform]` on structs — put on individual methods instead +**Rules:** +- `delegate* unmanaged[Stdcall]` for the function-pointer cast — IDL `STDMETHODCALLTYPE` ⇒ `__stdcall` on Win32. Picking the wrong convention silently corrupts the stack. +- Static-abstract `IComIID` is .NET 7+ only. For .NET-only structs (WMI), drop the `#else` branch and gate the whole file `#if NET`. For dual-target structs (Fusion), keep both as shown — see `src/Tasks/AssemblyDependency/Fusion/` for the canonical example. +- Use CsWin32-generated `PCWSTR` / `PWSTR` for wide strings (add to `NativeMethods.txt`); raw `char*` only when no typed equivalent exists. +- Vtable slots are exact: index 0 = `QueryInterface`, 1 = `AddRef`, 2 = `Release`, 3+ = interface methods in IDL order. When inheriting, **add the parent interface's method count** before counting your own (e.g. `ISetupConfiguration2.EnumAllInstances` is at slot 6 = 3 IUnknown + 3 v1 + 0). +- CS0592 prevents `[SupportedOSPlatform]` on structs — put it on individual methods. -### Dual-target manual structs (net472 + .NET) +## Lifetime & Access -When a manual COM struct must compile on both net472 and .NET, declare both forms of the `IComIID` member in the same struct so the right one binds against the per-target `IComIID` shape (CsWin32-generated static-abstract on .NET, polyfill instance member on net472): +**A raw `T*` (COM struct) is forbidden as a field on a non-`ref` type.** Allowed locations: locals in `unsafe` methods, parameters, fields of a `ref struct`. Anywhere else (instance fields of `class` / non-`ref` `struct`) → use `AgileComPointer` — a raw pointer field is an apartment-agility hazard and leaks on finalize-without-Dispose. + +### `ComScope` — transient pointers + +`ref struct`, `using`'d, calls `Release` on dispose. **The default for everything that doesn't survive the current method.** Implicitly converts to `T**` and `void**`, so out-params write into the scope directly: ```csharp -internal unsafe struct IAssemblyCache : IComIID +using ComScope scope = new(); +Guid clsid = SomeStruct.CLSID; +Guid iid = IID.Get(); +PInvoke.CoCreateInstance(&clsid, null, CLSCTX.CLSCTX_INPROC_SERVER, &iid, scope).ThrowOnFailure(); +scope.Pointer->DoThing(...); + +using ComScope other = new(); +Guid otherIid = IOther.IID_IOther; +scope.Pointer->QueryInterface(&otherIid, other).ThrowOnFailure(); +``` + +- Access methods via `scope.Pointer->Method(...)`. Null-check with `scope.IsNull`. +- Applies to *every* COM out-param: `CoCreateInstance`, `QueryInterface`, `IEnumXxx::Next`, factory methods, app-local `STDAPI Get*` (declare the `[DllImport]` with `T** pp`, not `out T*`, so the implicit operator binds). + +**Ownership transfer out of a helper.** A helper that acquires a COM pointer can return `ComScope` directly; intermediate pointers stay in their own `using` scopes and Release on the helper's `return`. `default` `ComScope` is null and its `Dispose` is a no-op, so callers don't need an extra null guard: + +```csharp +private static ComScope AcquireSetupConfig() { - public static readonly Guid IID_IAssemblyCache = new(0xE707DCDE, ...); + Guid clsid = SetupConfiguration.CLSID_SetupConfiguration; + Guid iid = ISetupConfiguration2.IID_ISetupConfiguration2; -#if NET - static ref readonly Guid IComIID.Guid - { - [MethodImpl(MethodImplOptions.AggressiveInlining)] - get => ref Unsafe.AsRef(in IID_IAssemblyCache); - } -#else - readonly Guid IComIID.Guid => IID_IAssemblyCache; -#endif + ComScope config = new(); + HRESULT hr = PInvoke.CoCreateInstance(&clsid, null, CLSCTX.CLSCTX_INPROC_SERVER, &iid, config); + if (hr.Succeeded) return config; // ownership flows to caller + if (hr != HRESULT.REGDB_E_CLASSNOTREG) return default; - private readonly void** _lpVtbl; - // ... IUnknown + interface methods + using ComScope v1 = new(); // intermediate, Released on return + if (SetupConfiguration.GetSetupConfiguration(v1, IntPtr.Zero) < 0 || v1.IsNull) return default; + return v1.Pointer->QueryInterface(&iid, config).Succeeded ? config : default; } + +// caller: +using ComScope config = AcquireSetupConfig(); +if (!config.IsNull) { /* use config.Pointer */ } ``` -See `src/Tasks/AssemblyDependency/Fusion/` for the full pattern. - -### Blittable vtable signatures - -COM vtable methods must be blittable for the same reason `[DllImport]` signatures must be — -the runtime does no marshalling work. **Follow the general blittable rules in -[cswin32-interop](../cswin32-interop/SKILL.md#blittable-signatures)** (return `HRESULT`, -`.ThrowOnFailure()`, `T**` not `out T*`, `void*` for opaque, `PCWSTR` / `PWSTR` for wide -strings, typed flag enums, no managed reference types). COM-vtable-specific additions: - -- **Function-pointer cast must match the native calling convention** — for COM vtables that - is almost always `delegate* unmanaged[Stdcall]` (the IDL `STDMETHODCALLTYPE` macro expands to - `__stdcall` on Win32). The general form is `delegate* unmanaged[Cc]` where `Cc` matches the - native side: `Cdecl` for varargs / printf-style APIs, `Thiscall` for C++ instance method - pointers, `Fastcall` for rare classic 32-bit APIs. Picking the wrong convention silently - corrupts the stack on the call. The compiler emits an IL `calli` instruction either way and - works on net472. -- **`[ComImport]` `PreserveSig` defaults the opposite way from `[DllImport]`** — `false` vs - `true`. When migrating a `[ComImport]` interface, every method that lacked an explicit - `[PreserveSig]` was previously throwing on failure HRESULTs; the new struct-based method - must call `.ThrowOnFailure()` at every call site to preserve that contract. See - "Error-Handling Parity When Migrating" below. -- **Vtable indices are exact** — unused slots may be omitted as long as the indices of the - ones you expose are correct relative to the native interface layout (count from 3 after - `QueryInterface` / `AddRef` / `Release`, and add the parent interface's method count when - inheriting). - -### Strongly-typed handle / token wrappers - -When the native side `typedef`s a primitive into a family of "same shape, different meaning" -aliases — e.g. `corhdr.h`'s `typedef ULONG32 mdToken; typedef mdToken mdAssembly; typedef mdToken mdAssemblyRef;` — -mirror that hierarchy with distinct `readonly struct` wrappers (same pattern CsWin32 uses for -`HANDLE` / `HWND` / `HMODULE`). Each wrapper holds a single field of the underlying primitive -so it stays blittable and ABI-compatible with the native type; the `delegate*` cast and any -array (`MdAssemblyRef[]`) marshal at zero cost. - -Conversion follows the typedef hierarchy: - -- **Implicit** widening from a specific type to the generic base (`MdAssembly` → `MdToken`). - Always safe — every `mdAssembly` is an `mdToken` at the C level — and lets specific tokens - flow naturally into APIs that accept the generic base (e.g. `GetCustomAttributeByName`). -- **Explicit** narrowing from the base to a specific type (`(MdAssembly)token`). The C side - can't enforce that the value really is the claimed kind — the runtime validates on use, not - at the cast site — so the cast must be opt-in. - -**Check the native header for the canonical validation primitives** before defining `IsNil` / -`IsValid` etc. on the wrapper. Encoding details often surface only in macros that don't show -up on a casual grep. For `mdToken` the encoding is `(TableType << 24) | Rid` with these -helpers in `corhdr.h`: - -| C macro / constant | What it really means | -|---|---| -| `TypeFromToken(tk) = tk & 0xff000000` | Table-type tag (high byte) → `CorTokenType` enum | -| `RidFromToken(tk) = tk & 0x00ffffff` | Row id (low 24 bits) | -| `IsNilToken(tk) = RidFromToken(tk) == 0` | Nil check — **row id half, not the whole value** | -| `mdAssemblyNil = mdtAssembly = 0x20000000` | Per-type nil is the table-type tag, **not 0** | +### `BSTR` — COM strings -A naive `IsNil => Value == 0` would silently misclassify a valid "no assembly in this scope" -return from `GetAssemblyFromScope` (which writes `0x20000000`) as a non-nil token. Mirror the -macro: `IsNil => Rid == 0`. +CsWin32-generated; extended in [`Windows/Win32/Foundation/BSTR.cs`](../../../src/Framework/Windows/Win32/Foundation/BSTR.cs) with `IDisposable`. `Dispose` calls `SysFreeString` and zeroes the field in place. **Scope every COM `BSTR` out-param with `using BSTR x = default;`** — no manual `try/finally + SysFreeString`: -Example: [`src/Tasks/AssemblyDependency/Metadata/Tokens.cs`](../../../src/Tasks/AssemblyDependency/Metadata/Tokens.cs) -defines `MdToken`, `MdAssembly`, `MdAssemblyRef`, `MdFile` with the implicit/explicit -conversions and exposes `Kind` (`CorTokenType`), `Rid`, `IsNil`, and `IsValid` on each. -The `CorTokenType` enum itself lives in -[`CorTokenType.cs`](../../../src/Tasks/AssemblyDependency/Metadata/CorTokenType.cs). +```csharp +using BSTR versionBstr = default; +if (instance->GetInstallationVersion(&versionBstr).Failed) return false; +string version = versionBstr.ToString(); +// SysFreeString runs at scope exit. +``` + +In-place-only: `BSTR.Dispose` writes through `Unsafe.AsRef(in this)`, so `using` must be on the storage location, not on a method-returned copy. Same rule as `ComScope`. + +### `AgileComPointer` — fields outliving a method + +Finalizable managed class. Use whenever a COM pointer is stored in a class field (the only legal way — raw `T*` fields are forbidden, see top of section). + +- Registers in the Global Interface Table (thread-agile); finalizer releases if `Dispose` is missed. +- Access via `using ComScope scope = agile.GetInterface();` then `scope.Pointer->Method(...)`. `GetInterface()` round-trips through the GIT — hoist a single scope to the top of a method when several calls share it. +- Constructor `takeOwnership`: pass `false` when the raw pointer is also held by a `ComScope` that will Release (GIT AddRefs independently); pass `true` only when no other code path will Release. +- Dispose via the owner's `Dispose` / `DisposeManagedResources` (`AgileComPointer` is managed, not unmanaged — its finalizer is the safety net, not the primary cleanup). ## Activation ```csharp -// Via ComClassFactory (AOT-compatible) +// AOT-compatible if (ComClassFactory.TryCreate(IWbemLocator.CLSID, out var factory, out HRESULT hr)) using ComScope instance = factory.TryCreateInstance(out hr); -// Via CoCreateInstance — use IID.Get() for the IID +// CoCreateInstance — IID.Get(), not &localGuid Guid clsid = IWbemLocator.CLSID; using ComScope locator = new(); hr = PInvoke.CoCreateInstance(&clsid, null, CLSCTX.CLSCTX_INPROC_SERVER, IID.Get(), locator); ``` -**Key points:** -- Use `IID.Get()` — do not take `&localGuid` -- Initialize `ComScope` with `new()`. It implicitly converts to `T**` / `void**` output parameters - ## Error-Handling Parity When Migrating -Struct-based COM returns raw `HRESULT`; `[ComImport]` and built-in activation threw automatically. Preserving the old throw-vs-return behavior is part of the migration. +Struct-based COM returns raw `HRESULT`; `[ComImport]` and built-in activation threw automatically. Preserve the old throw-vs-return at each call site: -| Old shape | Threw on failure via | Migrated shape | +| Old shape | Threw via | Migrated shape | |---|---|---| -| `new SomeCoClass()` | built-in interop activation | `PInvoke.CoCreateInstance(...).ThrowOnFailure()` | -| `(IFoo)rcw` cast | `InvalidCastException` on QI failure | `QueryInterface(&iid, scope).ThrowOnFailure()` | -| `[ComImport]` method (default `PreserveSig=false`) | marshaller throws on `FAILED(hr)` | `.ThrowOnFailure()` at the call site | +| `new SomeCoClass()` | built-in activation | `PInvoke.CoCreateInstance(...).ThrowOnFailure()` | +| `(IFoo)rcw` cast | `InvalidCastException` on QI | `QueryInterface(&iid, scope).ThrowOnFailure()` | +| `[ComImport]` method, default `PreserveSig=false` | marshaller throws on `FAILED(hr)` | `.ThrowOnFailure()` at the call site | | `[ComImport]` method with `[PreserveSig]` | caller inspects return | mirror the existing `hr` branch — don't start throwing | -Factory-method exception: when the old code's contract was "return null for invalid input" (e.g. `Create(path)`), keep null-return only for the operation that legitimately rejects input; environment-level failures (`CoCreateInstance`, QI for guaranteed-implemented interfaces) still throw. Example: [`MetadataReader.cs`](../../../src/Tasks/ManifestUtil/MetadataReader.cs) throws on activation and QI, returns null on `OpenScope` failure. +**Factory-method exception:** when the old contract was "return null for invalid input" (e.g. `Create(path)`), keep null-return only for the legitimate-rejection path; activation / QI failures still throw. See [`MetadataReader.cs`](../../../src/Tasks/ManifestUtil/MetadataReader.cs) (throws on activation/QI, returns null on `OpenScope`). -## IComIID Polyfill for net472 +**Don't throw when the caller swallows it.** If the top-level consumer wraps the whole chain in `catch (COMException) { }`, inner helpers must return `default` / `false` / empty instead of constructing a `COMException` only to have it discarded. Throwing here allocates the exception, walks the stack, and hides the HRESULT for no benefit. See [`VisualStudioLocationHelper.AcquireSetupConfiguration2`](../../../src/Framework/VisualStudioLocationHelper.cs). -CsWin32 emits `IComIID` (with static-abstract `Guid`) and attaches it to every generated COM struct **only on .NET 7+**. On older targets (net472, netstandard2.0): +## Strongly-typed handle / token wrappers -- The `IComIID` interface itself is missing — provide an instance-based version at `src/Framework/Polyfills/IComIID.cs`. -- Generated COM structs do not have `IComIID` in their base list — provide a partial struct that adds it. +When the native side `typedef`s a primitive into a family of "same shape, different meaning" aliases (`corhdr.h`'s `typedef ULONG32 mdToken; typedef mdToken mdAssembly; ...`), mirror that hierarchy with distinct `readonly struct` wrappers holding the single underlying primitive — same pattern as CsWin32's `HANDLE` / `HWND` / `HMODULE`. Stays blittable; `delegate*` casts and arrays marshal at zero cost. -This is a **known case that always requires polyfilling** for .NET Framework support. Pattern in [`src/Framework/Polyfills/IComIIDPolyfills.cs`](../../../src/Framework/Polyfills/IComIIDPolyfills.cs): +Conversions follow the typedef hierarchy: **implicit** widening to the base (`MdAssembly` → `MdToken`, always safe), **explicit** narrowing (`(MdAssembly)token`, opt-in because the C side can't enforce the kind at the cast site). -```csharp -namespace Windows.Win32.System.Com; - -internal partial struct IRunningObjectTable : IComIID -{ - readonly Guid IComIID.Guid => IID_Guid; // CsWin32-emitted field, always present -} -``` - -When a new CsWin32-generated COM type is used through `ComScope`, add a partial entry to that file. Modeled after [winforms `IDataObject.cs`](https://github.com/dotnet/winforms/blob/main/src/System.Private.Windows.Core/src/Framework/Windows/Win32/System/Com/IDataObject.cs). - -Both polyfill files are gated with `#if !NET` so they compile on net472/netstandard2.0 and become empty on .NET (where CsWin32 emits its own static-abstract `IComIID` and attaches it to generated structs). - -For manual COM structs (WMI, Setup Configuration, etc.) that already use the static-abstract `IComIID` form, the polyfill approach won't compile on net472 — those structs stay .NET-only via ``. +**Check the native header for the canonical validation primitives** before defining `IsNil` / `IsValid`. Encoding details often hide in macros that don't grep cleanly. For `mdToken` the encoding is `(TableType << 24) | Rid`: -## Lifetime & Access - -**A raw `T*` (where `T` is a COM struct) must never appear as a field of a non-`ref` type.** Allowed locations for a raw `T*`: +| C macro | Meaning | +|---|---| +| `TypeFromToken(tk) = tk & 0xff000000` | Table-type tag (high byte) → `CorTokenType` | +| `RidFromToken(tk) = tk & 0x00ffffff` | Row id (low 24 bits) | +| `IsNilToken(tk) = RidFromToken(tk) == 0` | Nil check — **rid half, not the whole value** | +| `mdAssemblyNil = mdtAssembly = 0x20000000` | Per-type nil is the table-type tag, **not 0** | -- locals inside an `unsafe` method, -- parameters, -- fields of a `ref struct` (whose lifetime is statically bounded to a stack frame). +A naive `IsNil => Value == 0` would misclassify the `0x20000000` "no assembly in this scope" return from `GetAssemblyFromScope`. Mirror the macro: `IsNil => Rid == 0`. Reference: [`Tokens.cs`](../../../src/Tasks/AssemblyDependency/Metadata/Tokens.cs), [`CorTokenType.cs`](../../../src/Tasks/AssemblyDependency/Metadata/CorTokenType.cs). -Anywhere else — instance fields of a `class` or non-`ref` `struct`, including `internal`/`private` ones — use `AgileComPointer`. A raw pointer field in a managed object is an apartment-agility hazard (the field can be observed from any thread, but the underlying interface may be apartment-bound) and leaks the ref count whenever the owner is finalized without `Dispose`. +## IComIID Polyfill for net472 -- `ComScope` — `ref struct`, use with `using`. Releases on dispose. **The preferred way to scope any COM pointer that doesn't survive the current method**, including transient pointers received from `CoCreateInstance`, `QueryInterface`, `OpenScope`, factory methods, etc. - - **Receive output parameters directly into the `ComScope`.** `ComScope` implicitly converts to `T**` and `void**`, so pass the scope itself where the API expects a `T** ppvObject` / `void** ppv`. The call writes into the scope and the `using` Releases on scope exit. No `T* local; ...->Method(&local); try {...} finally { Release(local); }` patterns. +CsWin32 emits `IComIID` (static-abstract `Guid`) and attaches it to generated COM structs **only on .NET 7+**. On net472 / netstandard2.0: - ```csharp - Guid clsid = SomeStruct.CLSID; - Guid iid = IID.Get(); - using ComScope scope = new(); - PInvoke.CoCreateInstance(&clsid, null, CLSCTX.CLSCTX_INPROC_SERVER, &iid, scope).ThrowOnFailure(); - scope.Pointer->DoThing(...); +- The interface is provided instance-based at [`src/Framework/Polyfills/IComIID.cs`](../../../src/Framework/Polyfills/IComIID.cs). +- Generated structs do **not** carry `IComIID` in their base list — add a partial in [`src/Framework/Polyfills/IComIIDPolyfills.cs`](../../../src/Framework/Polyfills/IComIIDPolyfills.cs): - using ComScope other = new(); - Guid otherIid = IOther.IID_IOther; - scope.Pointer->QueryInterface(&otherIid, other).ThrowOnFailure(); - ``` - - Access methods via `scope.Pointer->Method(...)`. Check for null with `scope.IsNull`. + ```csharp + namespace Windows.Win32.System.Com; + internal partial struct IRunningObjectTable : IComIID + { + readonly Guid IComIID.Guid => IID_Guid; // CsWin32-emitted field + } + ``` -- `AgileComPointer` — finalizable managed class. Use for **every COM pointer that outlives a single method call**, i.e. anything stored in a class field. - - Registers in the Global Interface Table (thread-agile) and releases via the finalizer if `Dispose` is missed. - - Access via `using ComScope scope = agile.GetInterface();` then `scope.Pointer->Method(...)`. Each `GetInterface()` round-trips through the GIT, so hoist a single scope to the top of a method when several calls share it. - - **Constructor `takeOwnership`**: pass `false` when the raw pointer was just received into a `ComScope` that will Release on dispose — the GIT registration AddRefs independently, so two owners is correct. Pass `true` only when no other code path will Release the raw pointer (e.g. handing off a pointer that has no `ComScope` wrapper). - - Dispose via the owner's `Dispose` / `DisposeManagedResources` (`AgileComPointer` is a managed object, not an unmanaged resource — it has its own finalizer). +Both files gated `#if !NET`. When you start using a new CsWin32-generated COM type through `ComScope`, add a partial. Manual structs (WMI, Setup Configuration) that already use the static-abstract form stay .NET-only via `` — the polyfill won't compile against them. ## File Organization | Location | Contents | -|----------|----------| -| `src/Framework/Windows/Win32/System/Com/` | `ComScope.cs`, `ComClassFactory.cs`, `AgileComPointer.cs`, `GlobalInterfaceTable.cs` | +|---|---| +| `src/Framework/Windows/Win32/System/Com/` | `ComScope`, `ComClassFactory`, `AgileComPointer`, `GlobalInterfaceTable` | | `src/Framework/Windows/Win32/IID.cs` | Generic IID lookup | -| `src/Framework/Utilities/Wmi/` | Manual WMI structs (.NET-only — use static-abstract `IComIID`) | -| `src/Framework/Polyfills/IComIID.cs` | net472/netstandard2.0 instance-based `IComIID` polyfill (`#if !NET`) | -| `src/Framework/Polyfills/IComIIDPolyfills.cs` | Per-struct partials attaching `IComIID` to CsWin32-generated COM types on net472/netstandard2.0 | +| `src/Framework/Polyfills/IComIID*.cs` | net472 / netstandard2.0 polyfills (`#if !NET`) | +| `src/Framework/Utilities/Wmi/`, `src/Framework/Shared/VisualStudio/`, `src/Tasks/AssemblyDependency/Fusion/`, `src/Tasks/AssemblyDependency/Metadata/`, `src/Tasks/TypeLib/` | Manual COM struct interfaces — own folder per API surface | ## CS3016 CLS Compliance -CsWin32 COM structs trigger CS3016 under `[assembly: CLSCompliant(true)]`. Handled via `[CLSCompliant(false)]` partial declarations in `GeneratedInteropClsCompliance.cs`. CS3019 warnings suppressed in `.editorconfig` for `{**/Windows/**/*.cs}` — do not add per-file suppressions. See https://github.com/dotnet/roslyn/issues/68526. \ No newline at end of file +CsWin32 COM structs trigger CS3016 under `[assembly: CLSCompliant(true)]`. Handled via `[CLSCompliant(false)]` partial declarations in `GeneratedInteropClsCompliance.cs`; CS3019 suppressed in `.editorconfig` for `{**/Windows/**/*.cs}`. Don't add per-file suppressions. See https://github.com/dotnet/roslyn/issues/68526. diff --git a/src/Build.UnitTests/BackEnd/TargetUpToDateChecker_Tests.cs b/src/Build.UnitTests/BackEnd/TargetUpToDateChecker_Tests.cs index 8fee963e09b..6cb71ec69a0 100644 --- a/src/Build.UnitTests/BackEnd/TargetUpToDateChecker_Tests.cs +++ b/src/Build.UnitTests/BackEnd/TargetUpToDateChecker_Tests.cs @@ -16,6 +16,8 @@ using Microsoft.Win32.SafeHandles; using Xunit; #if FEATURE_WINDOWSINTEROP +using Windows.Win32; +using Windows.Win32.Foundation; using Windows.Win32.Storage.FileSystem; #endif @@ -1005,26 +1007,36 @@ public void NewSymlinkNewDestinationIsNotUpToDate() expectedOutOfDate: true); } - [DllImport("kernel32.dll")] - [return: MarshalAs(UnmanagedType.Bool)] [SupportedOSPlatform("windows6.1")] - private static extern bool CreateSymbolicLink(string lpSymlinkFileName, string lpTargetFileName, UInt32 dwFlags); + private static unsafe bool CreateSymbolicLink(string lpSymlinkFileName, string lpTargetFileName, uint dwFlags) + { + fixed (char* pSym = lpSymlinkFileName) + { + fixed (char* pTarget = lpTargetFileName) + { + return PInvoke.CreateSymbolicLink(new PCWSTR(pSym), new PCWSTR(pTarget), (SYMBOLIC_LINK_FLAGS)dwFlags) != 0; + } + } + } - [DllImport("kernel32.dll", SetLastError = true)] [SupportedOSPlatform("windows6.1")] - private static extern bool SetFileTime(SafeFileHandle hFile, ref long creationTime, - ref long lastAccessTime, ref long lastWriteTime); + private static unsafe SafeFileHandle CreateFileForSymlink(string path) + { + HANDLE h; + fixed (char* pPath = path) + { + h = PInvoke.CreateFile( + new PCWSTR(pPath), + (uint)FILE_ACCESS_RIGHTS.FILE_GENERIC_READ | 0x100 /* FILE_WRITE_ATTRIBUTES */, + FILE_SHARE_MODE.FILE_SHARE_READ, + lpSecurityAttributes: null, + FILE_CREATION_DISPOSITION.OPEN_EXISTING, + FILE_FLAGS_AND_ATTRIBUTES.FILE_ATTRIBUTE_NORMAL | FILE_FLAGS_AND_ATTRIBUTES.FILE_FLAG_OPEN_REPARSE_POINT, + hTemplateFile: default); + } - [DllImport("kernel32.dll", CharSet = CharSet.Unicode, SetLastError = true, EntryPoint = "CreateFileW")] - [SupportedOSPlatform("windows6.1")] - private static extern SafeFileHandle CreateFileForSymlink( - string lpFileName, - uint dwDesiredAccess, - uint dwShareMode, - IntPtr lpSecurityAttributes, - uint dwCreationDisposition, - uint dwFlagsAndAttributes, - IntPtr hTemplateFile); + return new SafeFileHandle((IntPtr)h.Value, ownsHandle: true); + } [SupportedOSPlatform("windows6.1")] private void SimpleSymlinkInputCheck(DateTime symlinkWriteTime, DateTime targetWriteTime, @@ -1052,14 +1064,7 @@ private void SimpleSymlinkInputCheck(DateTime symlinkWriteTime, DateTime targetW // File.SetLastWriteTime on the symlink sets the target write time, // so set the symlink's write time the hard way - using (SafeFileHandle handle = CreateFileForSymlink( - inputSymlink, - (uint)FILE_ACCESS_RIGHTS.FILE_GENERIC_READ | 0x100 /* FILE_WRITE_ATTRIBUTES */, - (uint)FILE_SHARE_MODE.FILE_SHARE_READ, - IntPtr.Zero, - (uint)FILE_CREATION_DISPOSITION.OPEN_EXISTING, - (uint)(FILE_FLAGS_AND_ATTRIBUTES.FILE_ATTRIBUTE_NORMAL | FILE_FLAGS_AND_ATTRIBUTES.FILE_FLAG_OPEN_REPARSE_POINT), - IntPtr.Zero)) + using (SafeFileHandle handle = CreateFileForSymlink(inputSymlink)) { if (handle.IsInvalid) { @@ -1067,9 +1072,13 @@ private void SimpleSymlinkInputCheck(DateTime symlinkWriteTime, DateTime targetW } long symlinkWriteTimeTicks = symlinkWriteTime.ToFileTimeUtc(); + System.Runtime.InteropServices.ComTypes.FILETIME ft = new() + { + dwLowDateTime = (int)(symlinkWriteTimeTicks & 0xFFFFFFFF), + dwHighDateTime = (int)(symlinkWriteTimeTicks >> 32), + }; - if (!SetFileTime(handle, ref symlinkWriteTimeTicks, ref symlinkWriteTimeTicks, - ref symlinkWriteTimeTicks)) + if (!PInvoke.SetFileTime((HANDLE)handle.DangerousGetHandle(), ft, ft, ft)) { Marshal.ThrowExceptionForHR(Marshal.GetHRForLastWin32Error()); } diff --git a/src/Build.UnitTests/Microsoft.Build.Engine.UnitTests.csproj b/src/Build.UnitTests/Microsoft.Build.Engine.UnitTests.csproj index 386ce335f62..48b02992818 100644 --- a/src/Build.UnitTests/Microsoft.Build.Engine.UnitTests.csproj +++ b/src/Build.UnitTests/Microsoft.Build.Engine.UnitTests.csproj @@ -17,6 +17,7 @@ $(NoWarn);MSB3270;CS0436 ..\Shared\UnitTests\App.config + true diff --git a/src/Build/BackEnd/Node/NativeMethods.cs b/src/Build/BackEnd/Node/NativeMethods.cs deleted file mode 100644 index 31054f8cbd2..00000000000 --- a/src/Build/BackEnd/Node/NativeMethods.cs +++ /dev/null @@ -1,229 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System; -using System.Runtime.InteropServices; - -namespace Microsoft.Build.BackEnd -{ - /// - /// Native methods used by the backend. This was copied from the oldOM so we can make it stylecop compliant and allow - /// easier deletion of the native code in the old OM - /// - internal static class NativeMethods - { - /// - /// Null Pointer - /// - internal static readonly IntPtr NullPtr = IntPtr.Zero; - - /// - /// Invalid Handle - /// - internal static readonly IntPtr InvalidHandle = new IntPtr(-1); - - /// - /// Start the process with a normal priority class - /// - internal const uint NORMALPRIORITYCLASS = 0x0020; - - /// - /// Do not create a window - /// - internal const uint CREATENOWINDOW = 0x08000000; - - /// - /// Use the standard handles - /// - internal const Int32 STARTFUSESTDHANDLES = 0x00000100; - - /// - /// Create a new console. - /// - internal const Int32 CREATE_NEW_CONSOLE = 0x00000010; - - /// - /// Indicates that the environment block uses Unicode characters. - /// - internal const uint CREATE_UNICODE_ENVIRONMENT = 0x00000400; - - /// - /// Create a new process - /// - [DllImport("kernel32.dll", CharSet = CharSet.Unicode, SetLastError = true)] - [return: MarshalAs(UnmanagedType.Bool)] - internal static extern bool CreateProcess( - string lpApplicationName, - string lpCommandLine, - ref SECURITY_ATTRIBUTES lpProcessAttributes, - ref SECURITY_ATTRIBUTES lpThreadAttributes, - [In, MarshalAs(UnmanagedType.Bool)] - bool bInheritHandles, - uint dwCreationFlags, - IntPtr lpEnvironment, - string lpCurrentDirectory, - [In] ref STARTUP_INFO lpStartupInfo, - out PROCESS_INFORMATION lpProcessInformation); - - /// - /// Structure that contains the startupinfo - /// Represents STARTUP_INFO in win32 - /// - [StructLayout(LayoutKind.Sequential, CharSet = CharSet.Unicode)] - internal struct STARTUP_INFO - { - /// - /// The size of the structure, in bytes. - /// - internal Int32 cb; - - /// - /// Reserved; must be NULL - /// - internal string lpReserved; - - /// - /// The name of the desktop, or the name of both the desktop and window station for this process. - /// A backslash in the string indicates that the string includes both the desktop and window station names - /// - internal string lpDesktop; - - /// - /// For console processes, this is the title displayed in the title bar if a new console window is created. - /// If NULL, the name of the executable file is used as the window title instead. - /// This parameter must be NULL for GUI or console processes that do not create a new console window - /// - internal string lpTitle; - - /// - /// If dwFlags specifies STARTF_USEPOSITION, this member is the x offset of the upper left corner of a window if a new window is created, in pixels. Otherwise, this member is ignored - /// - internal Int32 dwX; - - /// - /// If dwFlags specifies STARTF_USEPOSITION, this member is the y offset of the upper left corner of a window if a new window is created, in pixels. Otherwise, this member is ignored. - /// - internal Int32 dwY; - - /// - /// If dwFlags specifies STARTF_USESIZE, this member is the width of the window if a new window is created, in pixels. Otherwise, this member is ignored. - /// - internal Int32 dwXSize; - - /// - /// If dwFlags specifies STARTF_USESIZE, this member is the height of the window if a new window is created, in pixels. Otherwise, this member is ignored. - /// - internal Int32 dwYSize; - - /// - /// If dwFlags specifies STARTF_USECOUNTCHARS, if a new console window is created in a console process, this member specifies the screen buffer width, in character columns. Otherwise, this member is ignored. - /// - internal Int32 dwXCountChars; - - /// - /// If dwFlags specifies STARTF_USECOUNTCHARS, if a new console window is created in a console process, this member specifies the screen buffer height, in character rows. Otherwise, this member is ignored.dwFillAttribute - /// - internal Int32 dwYCountChars; - - /// - /// If dwFlags specifies STARTF_USEFILLATTRIBUTE, this member is the initial text and background colors if a new console window is created in a console application. Otherwise, this member is ignored. - /// - internal Int32 dwFillAttribute; - - /// - /// A bit field that determines whether certain STARTUPINFO members are used when the process creates a window - /// - internal Int32 dwFlags; - - /// - /// If dwFlags specifies STARTF_USESHOWWINDOW, this member can be any of the SW_ constants defined in Winuser.h. Otherwise, this member is ignored. - /// - internal Int16 wShowWindow; - - /// - /// Reserved for use by the C Run-time; must be zero. - /// - internal Int16 cbReserved2; - - /// - /// Reserved for use by the C Run-time; must be NULL. - /// - internal IntPtr lpReserved2; - - /// - /// If dwFlags specifies STARTF_USESTDHANDLES, this member is the standard input handle for the process. Otherwise, this member is ignored and the default for standard input is the keyboard buffer. - /// - internal IntPtr hStdInput; - - /// - /// If dwFlags specifies STARTF_USESTDHANDLES, this member is the standard output handle for the process. Otherwise, this member is ignored and the default for standard output is the console window's buffer. - /// - internal IntPtr hStdOutput; - - /// - /// If dwFlags specifies STARTF_USESTDHANDLES, this member is the standard error handle for the process. Otherwise, this member is ignored and the default for standard error is the console window's buffer. - /// - internal IntPtr hStdError; - } - - /// - /// Structure to contain security attributes from the create process call represents - /// SECURITY_ATTRIBUTE in win32 - /// - [StructLayout(LayoutKind.Sequential)] - internal struct SECURITY_ATTRIBUTES - { - /// - /// The size, in bytes, of this structure. Set this value to the size of the SECURITY_ATTRIBUTES structure - /// - public int nLength; - - /// - /// A pointer to a security descriptor for the object that controls the sharing of it. - /// If NULL is specified for this member, the object is assigned the default security descriptor of the calling process. - /// This is not the same as granting access to everyone by assigning a NULL discretionary access control list (DACL). - /// The default security descriptor is based on the default DACL of the access token belonging to the calling process. - /// By default, the default DACL in the access token of a process allows access only to the user represented by the access token. - /// If other users must access the object, you can either create a security descriptor with the appropriate access, - /// or add ACEs to the DACL that grants access to a group of users. - /// - public IntPtr lpSecurityDescriptor; - - /// - /// A Boolean value that specifies whether the returned handle is inherited when a new process is created. - /// If this member is TRUE, the new process inherits the handle. - /// - public int bInheritHandle; - } - - /// - /// Process information from the create process call - /// Represents PROCESS_INFORMATION in win32 - /// - [StructLayout(LayoutKind.Sequential)] - internal struct PROCESS_INFORMATION - { - /// - /// A handle to the newly created process. The handle is used to specify the process in all functions that perform operations on the process object. - /// - public IntPtr hProcess; - - /// - /// A handle to the primary thread of the newly created process. The handle is used to specify the thread in all functions that perform operations on the thread object - /// - public IntPtr hThread; - - /// - /// A value that can be used to identify a process. - /// The value is valid from the time the process is created until all handles to the process are closed and - /// the process object is freed; at this point, the identifier may be reused. - /// - public int dwProcessId; - - /// - /// A value that can be used to identify a thread. The value is valid from the time the thread is created until all handles to the thread are closed and the thread object is freed; at this point, the identifier may be reused. - /// - public int dwThreadId; - } - } -} diff --git a/src/Build/Microsoft.Build.csproj b/src/Build/Microsoft.Build.csproj index d851eee77b8..34effaabafc 100644 --- a/src/Build/Microsoft.Build.csproj +++ b/src/Build/Microsoft.Build.csproj @@ -64,9 +64,6 @@ - - - diff --git a/src/Framework.UnitTests/FileSystem/WindowsNative_Tests.cs b/src/Framework.UnitTests/FileSystem/WindowsNative_Tests.cs new file mode 100644 index 00000000000..2d1e6531f9d --- /dev/null +++ b/src/Framework.UnitTests/FileSystem/WindowsNative_Tests.cs @@ -0,0 +1,151 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.IO; +using System.Linq; +using System.Runtime.Versioning; +using Microsoft.Build.Shared.FileSystem; +using Microsoft.Build.UnitTests.Shared; +using Shouldly; +using Xunit; + +namespace Microsoft.Build.UnitTests +{ + /// + /// Covers 's migration from `[DllImport]`-based + /// FindFirstFileW / FindNextFileW / FindClose / PathMatchSpecExW + /// to CsWin32 (PInvoke.FindFirstFile / FindNextFile / FindClose / + /// PathMatchSpecEx), plus the Win32FindData adapter built from the + /// blittable WIN32_FIND_DATAW. + /// + [SupportedOSPlatform("windows6.0.6000")] + public sealed class WindowsNative_Tests : IDisposable + { + private readonly string _tempDir; + + public WindowsNative_Tests() + { + _tempDir = Path.Combine(Path.GetTempPath(), "msb-WindowsNative_Tests-" + Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(_tempDir); + } + + public void Dispose() + { + try + { + if (Directory.Exists(_tempDir)) + { + Directory.Delete(_tempDir, recursive: true); + } + } + catch + { + // Best-effort: leave it to the temp folder cleanup if something locks a file. + } + } + + [WindowsOnlyFact] + public void FindFirstFile_ReturnsExpectedFileName() + { + // Single concrete file looked up by its exact name. CFileName must come back + // through the WIN32_FIND_DATAW.cFileName fixed buffer -> managed string adapter. + string filePath = Path.Combine(_tempDir, "alpha.txt"); + File.WriteAllText(filePath, "test"); + + using SafeFindFileHandle handle = WindowsNative.FindFirstFileW(filePath, out WindowsNative.Win32FindData data); + handle.IsInvalid.ShouldBeFalse(); + data.CFileName.ShouldBe("alpha.txt"); + } + + [WindowsOnlyFact] + public void FindNextFile_EnumeratesAllEntries() + { + // FindFirst + repeated FindNext should walk every file in the directory exactly once. + // "." and ".." come back too — exclude them, then compare to the canonical + // Directory.GetFiles enumeration of the same directory. + string[] names = ["a.txt", "b.txt", "c.txt"]; + foreach (string n in names) + { + File.WriteAllText(Path.Combine(_tempDir, n), n); + } + + using SafeFindFileHandle handle = WindowsNative.FindFirstFileW(Path.Combine(_tempDir, "*"), out WindowsNative.Win32FindData data); + handle.IsInvalid.ShouldBeFalse(); + + var found = new System.Collections.Generic.List(); + do + { + if (data.CFileName is not "." and not "..") + { + found.Add(data.CFileName); + } + } + while (WindowsNative.FindNextFileW(handle, out data)); + + found.OrderBy(s => s).ShouldBe(names.OrderBy(s => s)); + } + + [WindowsOnlyFact] + public void Win32FindData_AttributesAndTimes_MatchManagedAPIs() + { + // The adapter must preserve every field the callers depend on. Round-trip the + // attributes and last-write time against the standard managed file-system APIs. + string filePath = Path.Combine(_tempDir, "props.txt"); + File.WriteAllText(filePath, "x"); + File.SetAttributes(filePath, FileAttributes.ReadOnly | FileAttributes.Normal); + + using SafeFindFileHandle handle = WindowsNative.FindFirstFileW(filePath, out WindowsNative.Win32FindData data); + handle.IsInvalid.ShouldBeFalse(); + + File.GetAttributes(filePath).ShouldBe(data.DwFileAttributes); + + long fileTimeFromFindData = + ((long)(uint)data.FtLastWriteTime.dwHighDateTime << 32) + | (uint)data.FtLastWriteTime.dwLowDateTime; + DateTime.FromFileTimeUtc(fileTimeFromFindData) + .ShouldBe(File.GetLastWriteTimeUtc(filePath), TimeSpan.FromMilliseconds(10)); + + // nFileSizeHigh:nFileSizeLow is the 64-bit size. + ((long)data.NFileSizeHigh << 32 | data.NFileSizeLow).ShouldBe(1L); + } + + [WindowsOnlyFact] + public void FindFirstFile_NonExistentDirectory_ReturnsInvalidHandle() + { + // Bad path must surface as a SafeHandle whose IsInvalid is true (FindFirstFile + // returns INVALID_HANDLE_VALUE in this case). The SafeHandle must not throw on + // Dispose for an invalid handle either. + string bogus = Path.Combine(_tempDir, "does-not-exist", "*"); + using SafeFindFileHandle handle = WindowsNative.FindFirstFileW(bogus, out _); + handle.IsInvalid.ShouldBeTrue(); + } + + [WindowsOnlyFact] + public void PathMatchSpecExW_MatchAndNonMatch() + { + // Win32 PathMatchSpecEx returns S_OK (ErrorSuccess) on a match and S_FALSE + // on a non-match. The wrapper preserves that as the raw int. + WindowsNative.PathMatchSpecExW("foo.txt", "*.txt", WindowsNative.DwFlags.PmsfNormal) + .ShouldBe(WindowsNative.ErrorSuccess); + WindowsNative.PathMatchSpecExW("foo.bin", "*.txt", WindowsNative.DwFlags.PmsfNormal) + .ShouldNotBe(WindowsNative.ErrorSuccess); + } + + [WindowsOnlyFact] + public void SafeFindFileHandle_Dispose_IsIdempotent() + { + // SafeFindFileHandle's ReleaseHandle calls PInvoke.FindClose. Double-dispose and + // implicit-dispose at scope exit must both be no-ops. + string filePath = Path.Combine(_tempDir, "z.txt"); + File.WriteAllText(filePath, "z"); + + SafeFindFileHandle handle = WindowsNative.FindFirstFileW(filePath, out _); + handle.IsInvalid.ShouldBeFalse(); + + handle.Dispose(); + Should.NotThrow(() => handle.Dispose()); + handle.IsClosed.ShouldBeTrue(); + } + } +} diff --git a/src/Framework.UnitTests/Microsoft.Build.Framework.UnitTests.csproj b/src/Framework.UnitTests/Microsoft.Build.Framework.UnitTests.csproj index b2196587455..5d140de8c2b 100644 --- a/src/Framework.UnitTests/Microsoft.Build.Framework.UnitTests.csproj +++ b/src/Framework.UnitTests/Microsoft.Build.Framework.UnitTests.csproj @@ -9,7 +9,6 @@ - diff --git a/src/Framework.UnitTests/VisualStudioLocationHelper_Tests.cs b/src/Framework.UnitTests/VisualStudioLocationHelper_Tests.cs new file mode 100644 index 00000000000..240e46e8c62 --- /dev/null +++ b/src/Framework.UnitTests/VisualStudioLocationHelper_Tests.cs @@ -0,0 +1,125 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.IO; +using System.Linq; +using System.Runtime.Versioning; +using Microsoft.Build.Shared; +using Microsoft.Build.UnitTests.Shared; +using Shouldly; +using Xunit; + +namespace Microsoft.Build.UnitTests +{ + /// + /// Baseline behavioural contract for — the helper + /// that queries the Visual Studio Setup Configuration COM API + /// (Microsoft.VisualStudio.Setup.Configuration.Native.dll) for installed VS + /// instances on Windows .NET Framework. + /// + /// + /// These tests intentionally make no assumption about whether VS is installed on the + /// machine they run on — CI agents may have zero, one, or many VS instances. The contract + /// asserted here is: + /// + /// The call itself never throws (any underlying COMException / + /// DllNotFoundException is swallowed by design). + /// The returned list is non-null. + /// Every returned has a non-empty Name, + /// a non-empty Path, and a Version at major 15 or higher (the + /// ISetupConfiguration API does not surface VS < 15 instances). + /// Repeated calls return an equivalent set of instances (the helper is a pure + /// read of installer state; there is no per-call side effect). + /// + /// Tests run only on .NET Framework with VS Setup Configuration enabled — the + /// FEATURE_VISUALSTUDIOSETUP gate inside the helper means the non-net472 build + /// returns an empty list unconditionally and there is no native interop to exercise. + /// + public sealed class VisualStudioLocationHelper_Tests + { + [WindowsOnlyFact] + public void GetInstances_ReturnsNonNullList() + { + // The helper's contract is that it never throws and never returns null. An empty + // list is the legitimate "no VS installed" outcome — that's still a valid result, + // not an error. + var instances = VisualStudioLocationHelper.GetInstances(); + instances.ShouldNotBeNull(); + } + + [WindowsOnlyFact] + public void GetInstances_DoesNotThrowOnRepeatedCalls() + { + // The underlying ISetupConfiguration COM API is reentrant. The helper must be + // safe to invoke multiple times in the same process and should return the same + // set of installation paths each time. We compare paths because the same physical + // install produces a fresh COM enumeration object on each call. + var first = VisualStudioLocationHelper.GetInstances(); + var second = VisualStudioLocationHelper.GetInstances(); + + first.Select(i => i.Path).OrderBy(p => p) + .ShouldBe(second.Select(i => i.Path).OrderBy(p => p)); + } + + [WindowsOnlyFact] + public void GetInstances_AllResults_HaveValidShape() + { + // For every instance the helper surfaces, the three carried fields (Name, Path, + // Version) must be populated. Path doesn't have to exist on disk in the tests's + // sandbox (the user might have moved the install), but it must be a non-empty + // string. Version must be >= 15.0 because ISetupConfiguration was introduced + // for "VS 15" (Visual Studio 2017) and never reports earlier products. + var instances = VisualStudioLocationHelper.GetInstances(); + + foreach (var instance in instances) + { + instance.Name.ShouldNotBeNullOrEmpty(); + instance.Path.ShouldNotBeNullOrEmpty(); + instance.Version.ShouldNotBeNull(); + instance.Version.Major.ShouldBeGreaterThanOrEqualTo(15); + } + } + + [WindowsOnlyFact] + public void GetInstances_PathsAreUnique() + { + // Each install lives at its own physical location. The helper must not duplicate + // an instance — if the same path shows up twice that would indicate the + // enumeration didn't terminate or the same ISetupInstance was added twice. + var paths = VisualStudioLocationHelper.GetInstances().Select(i => i.Path).ToList(); + paths.ShouldBeUnique(); + } + + [WindowsOnlyFact] + public void GetInstances_ExistingPaths_AreAbsoluteDirectories() + { + // Real installations report rooted directories. If the path actually exists on + // disk it must be a directory (not a file). Paths that don't exist are still + // allowed (the user may have manually deleted an install without uninstalling). + foreach (var instance in VisualStudioLocationHelper.GetInstances()) + { + Path.IsPathRooted(instance.Path).ShouldBeTrue($"VS install path '{instance.Path}' is not rooted"); + if (Directory.Exists(instance.Path)) + { + // Sanity: when present, it's a directory, not a file masquerading as one. + File.Exists(instance.Path).ShouldBeFalse(); + } + } + } + + [Fact] + public void VisualStudioInstance_Constructor_AssignsAllProperties() + { + // Trivial data-shape check on the value-type wrapper. The constructor is the only + // surface code on this class (no validation, no normalization) — verifying it + // here guards against an accidental change to property order or copy-paste error. + var v = new Version(17, 8, 0, 0); + var instance = new VisualStudioInstance("VS 2022 Enterprise", @"C:\Program Files\Microsoft Visual Studio\2022\Enterprise", v); + + instance.Name.ShouldBe("VS 2022 Enterprise"); + instance.Path.ShouldBe(@"C:\Program Files\Microsoft Visual Studio\2022\Enterprise"); + instance.Version.ShouldBe(v); + } + } +} diff --git a/src/Framework/FileSystem/SafeFileHandle.cs b/src/Framework/FileSystem/SafeFileHandle.cs index 1ba5c2dc3e9..313adfd37ba 100644 --- a/src/Framework/FileSystem/SafeFileHandle.cs +++ b/src/Framework/FileSystem/SafeFileHandle.cs @@ -1,28 +1,32 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System; using Microsoft.Win32.SafeHandles; - +using Windows.Win32; +using Windows.Win32.Foundation; namespace Microsoft.Build.Shared.FileSystem { /// - /// Handle for a volume iteration as returned by WindowsNative.FindFirstVolumeW /> + /// Handle for a FindFirstFileW enumeration. Owns the FindClose call on Dispose. /// internal sealed class SafeFindFileHandle : SafeHandleZeroOrMinusOneIsInvalid { - /// - /// Private constructor for the PInvoke marshaller. - /// - private SafeFindFileHandle() + // Default ctor is used internally by SetHandle below; we don't want the P/Invoke + // marshaller path here anymore since SafeFindFileHandle is constructed manually + // from the HANDLE returned by PInvoke.FindFirstFile. + internal SafeFindFileHandle(IntPtr handle) : base(ownsHandle: true) { + SetHandle(handle); } /// + [System.Runtime.Versioning.SupportedOSPlatform("windows5.1.2600")] protected override bool ReleaseHandle() { - return WindowsNative.FindClose(handle); + return PInvoke.FindClose(new HANDLE(handle)); } } } diff --git a/src/Framework/FileSystem/WindowsNative.cs b/src/Framework/FileSystem/WindowsNative.cs index d50e4d56870..66c01d0af8c 100644 --- a/src/Framework/FileSystem/WindowsNative.cs +++ b/src/Framework/FileSystem/WindowsNative.cs @@ -8,13 +8,12 @@ using System.IO; using System.Runtime.InteropServices; - namespace Microsoft.Build.Shared.FileSystem { /// /// Native implementation of file system operations /// - internal static class WindowsNative + internal static unsafe class WindowsNative { /// /// Maximum path length. @@ -254,26 +253,74 @@ public struct Win32FindData } /// - [DllImport("kernel32.dll", SetLastError = true, CharSet = CharSet.Unicode)] - [SuppressMessage("Microsoft.Interoperability", "CA1401:PInvokesShouldNotBeVisible", Justification = "Needed for custom enumeration.")] - public static extern SafeFindFileHandle FindFirstFileW( - string lpFileName, - out Win32FindData lpFindFileData); + [System.Runtime.Versioning.SupportedOSPlatform("windows5.1.2600")] + public static SafeFindFileHandle FindFirstFileW(string lpFileName, out Win32FindData lpFindFileData) + { + Windows.Win32.Foundation.HANDLE h = Windows.Win32.PInvoke.FindFirstFile(lpFileName, out Windows.Win32.Storage.FileSystem.WIN32_FIND_DATAW raw); + lpFindFileData = ConvertFindData(in raw); + return new SafeFindFileHandle((nint)h.Value); + } /// - [DllImport("kernel32.dll", SetLastError = true, CharSet = CharSet.Unicode)] - [return: MarshalAs(UnmanagedType.Bool)] - [SuppressMessage("Microsoft.Interoperability", "CA1401:PInvokesShouldNotBeVisible", Justification = "Needed for custom enumeration.")] - public static extern bool FindNextFileW(SafeHandle hFindFile, out Win32FindData lpFindFileData); + [System.Runtime.Versioning.SupportedOSPlatform("windows5.1.2600")] + public static bool FindNextFileW(SafeHandle hFindFile, out Win32FindData lpFindFileData) + { + // DangerousAddRef pins the SafeHandle's ref count for the duration of the + // native call so a concurrent Dispose() cannot invalidate the handle mid-call. + // Today's enumeration callers are single-threaded but the cost is negligible. + bool refAdded = false; + try + { + hFindFile.DangerousAddRef(ref refAdded); + bool ok = Windows.Win32.PInvoke.FindNextFile( + new Windows.Win32.Foundation.HANDLE((void*)hFindFile.DangerousGetHandle()), + out Windows.Win32.Storage.FileSystem.WIN32_FIND_DATAW raw); + lpFindFileData = ConvertFindData(in raw); + return ok; + } + finally + { + if (refAdded) + { + hFindFile.DangerousRelease(); + } + } + } /// - [DllImport("shlwapi.dll", CharSet = CharSet.Unicode)] - [SuppressMessage("Microsoft.Interoperability", "CA1401:PInvokesShouldNotBeVisible", Justification = "Needed for creating symlinks.")] - public static extern int PathMatchSpecExW([In] string pszFileParam, [In] string pszSpec, [In] int flags); + [System.Runtime.Versioning.SupportedOSPlatform("windows6.0.6000")] + public static int PathMatchSpecExW(string pszFileParam, string pszSpec, int flags) + { + return Windows.Win32.PInvoke.PathMatchSpecEx(pszFileParam, pszSpec, (uint)flags); + } /// - [DllImport("kernel32.dll", SetLastError = true)] - [return: MarshalAs(UnmanagedType.Bool)] - internal static extern bool FindClose(IntPtr findFileHandle); + [System.Runtime.Versioning.SupportedOSPlatform("windows5.1.2600")] + internal static bool FindClose(IntPtr findFileHandle) + { + return Windows.Win32.PInvoke.FindClose(new Windows.Win32.Foundation.HANDLE((void*)findFileHandle)); + } + + // Adapter from CsWin32's WIN32_FIND_DATAW (fixed-size char buffers, no string marshalling) + // to the legacy Win32FindData shape used by callers that read CFileName as a managed string. + private static Win32FindData ConvertFindData(in Windows.Win32.Storage.FileSystem.WIN32_FIND_DATAW raw) + { + fixed (Windows.Win32.Storage.FileSystem.WIN32_FIND_DATAW* p = &raw) + { + return new Win32FindData + { + DwFileAttributes = (FileAttributes)p->dwFileAttributes, + FtCreationTime = p->ftCreationTime, + FtLastAccessTime = p->ftLastAccessTime, + FtLastWriteTime = p->ftLastWriteTime, + NFileSizeHigh = p->nFileSizeHigh, + NFileSizeLow = p->nFileSizeLow, + DwReserved0 = p->dwReserved0, + DwReserved1 = p->dwReserved1, + CFileName = new string((char*)&p->cFileName), + CAlternate = new string((char*)&p->cAlternateFileName), + }; + } + } } } diff --git a/src/Framework/Microsoft.Build.Framework.csproj b/src/Framework/Microsoft.Build.Framework.csproj index 92d39aa2e0b..55f424e11b1 100644 --- a/src/Framework/Microsoft.Build.Framework.csproj +++ b/src/Framework/Microsoft.Build.Framework.csproj @@ -43,7 +43,6 @@ - @@ -91,8 +90,11 @@ + + + diff --git a/src/Framework/NativeMethods.txt b/src/Framework/NativeMethods.txt index 3508948fd36..d0909183f98 100644 --- a/src/Framework/NativeMethods.txt +++ b/src/Framework/NativeMethods.txt @@ -16,12 +16,13 @@ CreateItemMoniker CreatePipe CreateProcess CreateSymbolicLink +DebugCreate DEBUG_PROC_DESC_NO_MTS_PACKAGES DEBUG_PROC_DESC_NO_PATHS DEBUG_PROC_DESC_NO_SERVICES DEBUG_PROC_DESC_NO_SESSION_ID DEBUG_PROC_DESC_NO_USER_NAME -DebugCreate +DefineDosDevice EndUpdateResource EOLE_AUTHENTICATION_CAPABILITIES FACILITY_CODE @@ -31,6 +32,10 @@ FILE_FLAGS_AND_ATTRIBUTES FILE_MAP FILE_SHARE_MODE FILETIME +FindClose +FindFirstFile +FindNextFile +FindResource FreeEnvironmentStrings FreeEnvironmentStringsW FreeLibrary @@ -70,7 +75,11 @@ INVALID_FILE_ATTRIBUTES INVALID_HANDLE_VALUE IRunningObjectTable KNOWN_FOLDER_FLAG +LOAD_LIBRARY_FLAGS LoadLibrary +LoadLibraryEx +LoadResource +LockResource MapViewOfFile MAX_PATH MEMORYSTATUSEX @@ -79,6 +88,7 @@ MoveFileEx NtQueryInformationProcess OpenProcess PAGE_PROTECTION_FLAGS +PathMatchSpecEx PROCESS_ACCESS_RIGHTS PROCESS_BASIC_INFORMATION PROCESS_CREATION_FLAGS @@ -86,6 +96,8 @@ PROCESS_INFORMATION PropVariantClear PWSTR PCWSTR +PCSTR +QueryDosDevice ReadFile REGDB_E_CLASSNOTREG RM_APP_TYPE @@ -107,8 +119,10 @@ SECURITY_ATTRIBUTES SetConsoleMode SetCurrentDirectory SetEnvironmentVariable +SetFileTime SetThreadErrorMode SHGetKnownFolderPath +SizeofResource STARTUPINFOW STARTUPINFOW_FLAGS SYMBOLIC_LINK_FLAGS diff --git a/src/Framework/Shared/VisualStudio/IEnumSetupInstances.cs b/src/Framework/Shared/VisualStudio/IEnumSetupInstances.cs new file mode 100644 index 00000000000..bb60c1d162f --- /dev/null +++ b/src/Framework/Shared/VisualStudio/IEnumSetupInstances.cs @@ -0,0 +1,85 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +#if FEATURE_WINDOWSINTEROP && FEATURE_VISUALSTUDIOSETUP + +using System; +#if NET +using System.Runtime.CompilerServices; +#endif +using Windows.Win32; +using Windows.Win32.Foundation; + +namespace Microsoft.Build.Shared.VisualStudio; + +/// +/// Standard COM enumerator over pointers. +/// +/// +/// Declared in Setup.Configuration.idl. IID = {6380BCFF-41D3-4B2E-8B2E-BF8A6810C848}. +/// +internal unsafe struct IEnumSetupInstances : IComIID +{ + public static readonly Guid IID_IEnumSetupInstances = new(0x6380BCFF, 0x41D3, 0x4B2E, 0x8B, 0x2E, 0xBF, 0x8A, 0x68, 0x10, 0xC8, 0x48); + +#if NET + static ref readonly Guid IComIID.Guid + { + [MethodImpl(MethodImplOptions.AggressiveInlining)] + get => ref Unsafe.AsRef(in IID_IEnumSetupInstances); + } +#else + readonly Guid IComIID.Guid => IID_IEnumSetupInstances; +#endif + + private readonly void** _lpVtbl; + + // IUnknown methods (vtable indices 0-2) + + public HRESULT QueryInterface(Guid* riid, void** ppvObject) + { + fixed (IEnumSetupInstances* pThis = &this) + { + return ((delegate* unmanaged[Stdcall])_lpVtbl[0])(pThis, riid, ppvObject); + } + } + + public uint AddRef() + { + fixed (IEnumSetupInstances* pThis = &this) + { + return ((delegate* unmanaged[Stdcall])_lpVtbl[1])(pThis); + } + } + + public uint Release() + { + fixed (IEnumSetupInstances* pThis = &this) + { + return ((delegate* unmanaged[Stdcall])_lpVtbl[2])(pThis); + } + } + + // IEnumSetupInstances vtable layout (indices 3-6): + // 3 = Next (ULONG celt, ISetupInstance** rgelt, ULONG* pceltFetched) <-- Used + // 4 = Skip (ULONG celt) + // 5 = Reset () + // 6 = Clone (out IEnumSetupInstances**) + + /// + /// Retrieve up to instances from the enumerator. Returns + /// when exactly items were + /// produced and when fewer (including zero) were + /// available. always reflects the actual count. + /// + public HRESULT Next(uint celt, ISetupInstance** rgelt, uint* pceltFetched) + { + fixed (IEnumSetupInstances* pThis = &this) + { + return ((delegate* unmanaged[Stdcall])_lpVtbl[3])( + pThis, celt, rgelt, pceltFetched); + } + } +} + +#endif diff --git a/src/Framework/Shared/VisualStudio/ISetupConfiguration.cs b/src/Framework/Shared/VisualStudio/ISetupConfiguration.cs new file mode 100644 index 00000000000..3edc16b1721 --- /dev/null +++ b/src/Framework/Shared/VisualStudio/ISetupConfiguration.cs @@ -0,0 +1,66 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +#if FEATURE_WINDOWSINTEROP && FEATURE_VISUALSTUDIOSETUP + +using System; +#if NET +using System.Runtime.CompilerServices; +#endif +using Windows.Win32; +using Windows.Win32.Foundation; + +namespace Microsoft.Build.Shared.VisualStudio; + +/// +/// Top-level Visual Studio Setup Configuration query interface (v1). MSBuild only needs +/// the IUnknown surface here so it can QI to ; the +/// v1-specific vtable slots (3–5) are intentionally not exposed. +/// +/// +/// Declared in Setup.Configuration.idl. IID = {42843719-DB4C-46C2-8E7C-64F1816EFD5B}. +/// +internal unsafe struct ISetupConfiguration : IComIID +{ + public static readonly Guid IID_ISetupConfiguration = new(0x42843719, 0xDB4C, 0x46C2, 0x8E, 0x7C, 0x64, 0xF1, 0x81, 0x6E, 0xFD, 0x5B); + +#if NET + static ref readonly Guid IComIID.Guid + { + [MethodImpl(MethodImplOptions.AggressiveInlining)] + get => ref Unsafe.AsRef(in IID_ISetupConfiguration); + } +#else + readonly Guid IComIID.Guid => IID_ISetupConfiguration; +#endif + + private readonly void** _lpVtbl; + + // IUnknown methods (vtable indices 0-2) + + public HRESULT QueryInterface(Guid* riid, void** ppvObject) + { + fixed (ISetupConfiguration* pThis = &this) + { + return ((delegate* unmanaged[Stdcall])_lpVtbl[0])(pThis, riid, ppvObject); + } + } + + public uint AddRef() + { + fixed (ISetupConfiguration* pThis = &this) + { + return ((delegate* unmanaged[Stdcall])_lpVtbl[1])(pThis); + } + } + + public uint Release() + { + fixed (ISetupConfiguration* pThis = &this) + { + return ((delegate* unmanaged[Stdcall])_lpVtbl[2])(pThis); + } + } +} + +#endif diff --git a/src/Framework/Shared/VisualStudio/ISetupConfiguration2.cs b/src/Framework/Shared/VisualStudio/ISetupConfiguration2.cs new file mode 100644 index 00000000000..495e528e4a5 --- /dev/null +++ b/src/Framework/Shared/VisualStudio/ISetupConfiguration2.cs @@ -0,0 +1,87 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +#if FEATURE_WINDOWSINTEROP && FEATURE_VISUALSTUDIOSETUP + +using System; +#if NET +using System.Runtime.CompilerServices; +#endif +using Windows.Win32; +using Windows.Win32.Foundation; + +namespace Microsoft.Build.Shared.VisualStudio; + +/// +/// v2 of the Visual Studio Setup Configuration top-level query interface. Derives from +/// ; adds for enumerating +/// every installed instance (including incomplete / partial installations). +/// +/// +/// Declared in Setup.Configuration.idl. IID = {26AAB78C-4A60-49D6-AF3B-3C35BC93365D}. +/// +internal unsafe struct ISetupConfiguration2 : IComIID +{ + public static readonly Guid IID_ISetupConfiguration2 = new(0x26AAB78C, 0x4A60, 0x49D6, 0xAF, 0x3B, 0x3C, 0x35, 0xBC, 0x93, 0x36, 0x5D); + +#if NET + static ref readonly Guid IComIID.Guid + { + [MethodImpl(MethodImplOptions.AggressiveInlining)] + get => ref Unsafe.AsRef(in IID_ISetupConfiguration2); + } +#else + readonly Guid IComIID.Guid => IID_ISetupConfiguration2; +#endif + + private readonly void** _lpVtbl; + + // IUnknown methods (vtable indices 0-2) + + public HRESULT QueryInterface(Guid* riid, void** ppvObject) + { + fixed (ISetupConfiguration2* pThis = &this) + { + return ((delegate* unmanaged[Stdcall])_lpVtbl[0])(pThis, riid, ppvObject); + } + } + + public uint AddRef() + { + fixed (ISetupConfiguration2* pThis = &this) + { + return ((delegate* unmanaged[Stdcall])_lpVtbl[1])(pThis); + } + } + + public uint Release() + { + fixed (ISetupConfiguration2* pThis = &this) + { + return ((delegate* unmanaged[Stdcall])_lpVtbl[2])(pThis); + } + } + + // ISetupConfiguration vtable layout (indices 3-5): + // 3 = EnumInstances (out IEnumSetupInstances**) + // 4 = GetInstanceForCurrentProcess(out ISetupInstance**) + // 5 = GetInstanceForPath (LPCWSTR, out ISetupInstance**) + + // ISetupConfiguration2 vtable layout (added at index 6): + // 6 = EnumAllInstances (out IEnumSetupInstances**) <-- Used + + /// + /// Enumerate every installed Visual Studio instance the installer knows about, + /// including ones whose InstanceState is incomplete. Use this in preference + /// to EnumInstances when the caller wants to surface partial installs. + /// + public HRESULT EnumAllInstances(IEnumSetupInstances** ppEnumInstances) + { + fixed (ISetupConfiguration2* pThis = &this) + { + return ((delegate* unmanaged[Stdcall])_lpVtbl[6])(pThis, ppEnumInstances); + } + } +} + +#endif diff --git a/src/Framework/Shared/VisualStudio/ISetupInstance.cs b/src/Framework/Shared/VisualStudio/ISetupInstance.cs new file mode 100644 index 00000000000..b8b3fe22098 --- /dev/null +++ b/src/Framework/Shared/VisualStudio/ISetupInstance.cs @@ -0,0 +1,107 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +#if FEATURE_WINDOWSINTEROP && FEATURE_VISUALSTUDIOSETUP + +using System; +#if NET +using System.Runtime.CompilerServices; +#endif +using Windows.Win32; +using Windows.Win32.Foundation; + +namespace Microsoft.Build.Shared.VisualStudio; + +/// +/// A single installed Visual Studio instance. MSBuild only reads three identity fields +/// (installation path, display name, installation version); the rest of the v1 vtable +/// is intentionally not exposed. +/// +/// +/// Declared in Setup.Configuration.idl. IID = {B41463C3-8866-43B5-BC33-2B0676F7F42E}. +/// String returns are ; callers own the BSTR and must release it via +/// SysFreeString. +/// +internal unsafe struct ISetupInstance : IComIID +{ + public static readonly Guid IID_ISetupInstance = new(0xB41463C3, 0x8866, 0x43B5, 0xBC, 0x33, 0x2B, 0x06, 0x76, 0xF7, 0xF4, 0x2E); + +#if NET + static ref readonly Guid IComIID.Guid + { + [MethodImpl(MethodImplOptions.AggressiveInlining)] + get => ref Unsafe.AsRef(in IID_ISetupInstance); + } +#else + readonly Guid IComIID.Guid => IID_ISetupInstance; +#endif + + private readonly void** _lpVtbl; + + // IUnknown methods (vtable indices 0-2) + + public HRESULT QueryInterface(Guid* riid, void** ppvObject) + { + fixed (ISetupInstance* pThis = &this) + { + return ((delegate* unmanaged[Stdcall])_lpVtbl[0])(pThis, riid, ppvObject); + } + } + + public uint AddRef() + { + fixed (ISetupInstance* pThis = &this) + { + return ((delegate* unmanaged[Stdcall])_lpVtbl[1])(pThis); + } + } + + public uint Release() + { + fixed (ISetupInstance* pThis = &this) + { + return ((delegate* unmanaged[Stdcall])_lpVtbl[2])(pThis); + } + } + + // ISetupInstance vtable layout (indices 3-10): + // 3 = GetInstanceId (out BSTR) + // 4 = GetInstallDate (out FILETIME) + // 5 = GetInstallationName (out BSTR) + // 6 = GetInstallationPath (out BSTR) <-- Used + // 7 = GetInstallationVersion (out BSTR) <-- Used + // 8 = GetDisplayName (LCID, out BSTR) <-- Used + // 9 = GetDescription (LCID, out BSTR) + // 10 = ResolvePath (LPCWSTR, out BSTR) + + /// Get the absolute installation path of this instance (typically ...\Microsoft Visual Studio\YYYY\Edition). + public HRESULT GetInstallationPath(BSTR* pbstrInstallationPath) + { + fixed (ISetupInstance* pThis = &this) + { + return ((delegate* unmanaged[Stdcall])_lpVtbl[6])(pThis, pbstrInstallationPath); + } + } + + /// Get the installation version (e.g. "17.8.0.0"); parseable by . + public HRESULT GetInstallationVersion(BSTR* pbstrInstallationVersion) + { + fixed (ISetupInstance* pThis = &this) + { + return ((delegate* unmanaged[Stdcall])_lpVtbl[7])(pThis, pbstrInstallationVersion); + } + } + + /// Get the localized display name (e.g. "Visual Studio Enterprise 2022"). + /// LCID for localization; 0 for the current user UI culture. + /// Receives the localized display name as a freshly-allocated BSTR; the caller owns it and must release it via SysFreeString. + public HRESULT GetDisplayName(uint lcid, BSTR* pbstrDisplayName) + { + fixed (ISetupInstance* pThis = &this) + { + return ((delegate* unmanaged[Stdcall])_lpVtbl[8])(pThis, lcid, pbstrDisplayName); + } + } +} + +#endif diff --git a/src/Framework/Shared/VisualStudio/ISetupInstance2.cs b/src/Framework/Shared/VisualStudio/ISetupInstance2.cs new file mode 100644 index 00000000000..9dcfb2c8fa8 --- /dev/null +++ b/src/Framework/Shared/VisualStudio/ISetupInstance2.cs @@ -0,0 +1,90 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +#if FEATURE_WINDOWSINTEROP && FEATURE_VISUALSTUDIOSETUP + +using System; +#if NET +using System.Runtime.CompilerServices; +#endif +using Windows.Win32; +using Windows.Win32.Foundation; + +namespace Microsoft.Build.Shared.VisualStudio; + +/// +/// v2 of . Derives from ; adds +/// installer-state queries. MSBuild only needs here to filter out +/// partial installations. +/// +/// +/// Declared in Setup.Configuration.idl. IID = {89143C9A-05AF-49B0-B717-72E218A2185C}. +/// +internal unsafe struct ISetupInstance2 : IComIID +{ + public static readonly Guid IID_ISetupInstance2 = new(0x89143C9A, 0x05AF, 0x49B0, 0xB7, 0x17, 0x72, 0xE2, 0x18, 0xA2, 0x18, 0x5C); + +#if NET + static ref readonly Guid IComIID.Guid + { + [MethodImpl(MethodImplOptions.AggressiveInlining)] + get => ref Unsafe.AsRef(in IID_ISetupInstance2); + } +#else + readonly Guid IComIID.Guid => IID_ISetupInstance2; +#endif + + private readonly void** _lpVtbl; + + // IUnknown methods (vtable indices 0-2) + + public HRESULT QueryInterface(Guid* riid, void** ppvObject) + { + fixed (ISetupInstance2* pThis = &this) + { + return ((delegate* unmanaged[Stdcall])_lpVtbl[0])(pThis, riid, ppvObject); + } + } + + public uint AddRef() + { + fixed (ISetupInstance2* pThis = &this) + { + return ((delegate* unmanaged[Stdcall])_lpVtbl[1])(pThis); + } + } + + public uint Release() + { + fixed (ISetupInstance2* pThis = &this) + { + return ((delegate* unmanaged[Stdcall])_lpVtbl[2])(pThis); + } + } + + // ISetupInstance vtable layout (indices 3-10): see ISetupInstance.cs. + // ISetupInstance2 vtable layout (added at indices 11+): + // 11 = GetState (out InstanceState) <-- Used + // 12 = GetPackages (out SAFEARRAY of ISetupPackageReference*) + // 13 = GetProduct (out ISetupPackageReference**) + // 14 = GetProductPath (out BSTR) + // 15 = GetErrors (out ISetupErrorState**) + // 16 = IsLaunchable (out VARIANT_BOOL) + // 17 = IsComplete (out VARIANT_BOOL) + // 18 = GetProperties (out ISetupPropertyStore**) + // 19 = GetEnginePath (out BSTR) + + /// + /// Get the installer-state bitfield. Compare against + /// to filter out partial installations that the user should not be invoking against. + /// + public HRESULT GetState(InstanceState* pState) + { + fixed (ISetupInstance2* pThis = &this) + { + return ((delegate* unmanaged[Stdcall])_lpVtbl[11])(pThis, pState); + } + } +} + +#endif diff --git a/src/Framework/Shared/VisualStudio/InstanceState.cs b/src/Framework/Shared/VisualStudio/InstanceState.cs new file mode 100644 index 00000000000..2fd5870e8fc --- /dev/null +++ b/src/Framework/Shared/VisualStudio/InstanceState.cs @@ -0,0 +1,45 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +#if FEATURE_WINDOWSINTEROP && FEATURE_VISUALSTUDIOSETUP + +using System; + +namespace Microsoft.Build.Shared.VisualStudio; + +/// +/// Bitfield describing how complete the installer considers a given Visual Studio +/// instance. Only an instance whose state equals (all known +/// state bits set) is safe for tooling to invoke against. +/// +/// +/// Declared in Setup.Configuration.idl as InstanceState : DWORD. +/// +[Flags] +internal enum InstanceState : uint +{ + /// No state set yet; the install record is still being populated. + None = 0, + + /// The instance's files exist on disk under its installation path. + Local = 1, + + /// The instance is registered with the OS (Add/Remove Programs, COM, etc.). + Registered = 2, + + /// The instance does not need a reboot before it can be used. + NoRebootRequired = 4, + + /// The instance has no recorded installer errors. + NoErrors = 8, + + /// + /// Sentinel "everything is set" value (MAXUINT in the IDL). Matches the + /// InstanceState.Complete constant in the managed + /// Microsoft.VisualStudio.Setup.Configuration.Interop RCW wrapper that this + /// enum replaces. + /// + Complete = uint.MaxValue, +} + +#endif diff --git a/src/Framework/Shared/VisualStudio/SetupConfiguration.cs b/src/Framework/Shared/VisualStudio/SetupConfiguration.cs new file mode 100644 index 00000000000..7d2ed2c8f9c --- /dev/null +++ b/src/Framework/Shared/VisualStudio/SetupConfiguration.cs @@ -0,0 +1,47 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +// Manually-defined Visual Studio Setup Configuration COM structs following CsWin32 +// struct-based COM patterns. The Setup Configuration COM API is not in Win32 metadata +// (it ships with the VS installer redistributable as +// Microsoft.VisualStudio.Setup.Configuration.Native.dll); declarations from the +// Setup.Configuration type library. + +#if FEATURE_WINDOWSINTEROP && FEATURE_VISUALSTUDIOSETUP + +using System; +using System.Runtime.InteropServices; + +namespace Microsoft.Build.Shared.VisualStudio; + +/// +/// Activation entry points and well-known IIDs for the Visual Studio Setup Configuration +/// COM API exposed by Microsoft.VisualStudio.Setup.Configuration.Native.dll. +/// +internal static unsafe class SetupConfiguration +{ + /// + /// CLSID of the SetupConfiguration coclass (the entry-point activatable object). + /// CoCreate this CLSID against / + /// to obtain the top-level query interface. + /// + public static readonly Guid CLSID_SetupConfiguration = new(0x177F0C4A, 0x1CD3, 0x4DE7, 0xA3, 0x2C, 0x71, 0xDB, 0xBB, 0x9F, 0xA3, 0x6D); + + /// + /// Fallback activation entry point that lives next to the coclass when the CLSID is + /// not registered with COM (which happens when the helper DLL has not been registered + /// on the machine — e.g. fresh CI agent without the VS installer). Returns a raw + /// pointer suitable for QI'ing to + /// . + /// + /// + /// Signature from Setup.Configuration.idl: + /// STDAPI GetSetupConfiguration(_Outptr_ ISetupConfiguration **ppConfiguration, _Reserved_ LPVOID lpReserved);. + /// The ISetupConfiguration** parameter accepts ComScope<ISetupConfiguration> + /// directly via its implicit T** operator. + /// + [DllImport("Microsoft.VisualStudio.Setup.Configuration.Native.dll", ExactSpelling = true, PreserveSig = true)] + public static extern int GetSetupConfiguration(ISetupConfiguration** ppConfiguration, IntPtr lpReserved); +} + +#endif diff --git a/src/Framework/VisualStudioLocationHelper.cs b/src/Framework/VisualStudioLocationHelper.cs index 1110cdee745..6e296f22b39 100644 --- a/src/Framework/VisualStudioLocationHelper.cs +++ b/src/Framework/VisualStudioLocationHelper.cs @@ -3,20 +3,27 @@ using System; using System.Collections.Generic; -#if FEATURE_VISUALSTUDIOSETUP -using System.Runtime.InteropServices; -using Microsoft.VisualStudio.Setup.Configuration; +#if FEATURE_WINDOWSINTEROP && FEATURE_VISUALSTUDIOSETUP +using Microsoft.Build.Shared.VisualStudio; +using Windows.Win32; using Windows.Win32.Foundation; +using Windows.Win32.System.Com; #endif namespace Microsoft.Build.Shared { /// - /// Helper class to wrap the Microsoft.VisualStudio.Setup.Configuration.Interop API to query - /// Visual Studio setup for instances installed on the machine. - /// Code derived from sample: https://code.msdn.microsoft.com/Visual-Studio-Setup-0cedd331 + /// Helper class that queries the Visual Studio Setup Configuration COM API for instances + /// of Visual Studio installed on the machine. Will not include anything before VS "15". /// - internal class VisualStudioLocationHelper + /// + /// Uses manually-defined struct-based COM declarations under Shared/VisualStudio/ + /// (matching the CsWin32 struct-based COM pattern used elsewhere in the repo) so this + /// project does not need a reference to the legacy + /// Microsoft.VisualStudio.Setup.Configuration.Interop RCW package. All COM pointer + /// lifetimes are managed via ComScope<T>. + /// + internal static unsafe class VisualStudioLocationHelper { /// /// Query the Visual Studio setup API to get instances of Visual Studio installed @@ -27,84 +34,159 @@ internal static IList GetInstances() { var validInstances = new List(); -#if FEATURE_VISUALSTUDIOSETUP +#if FEATURE_WINDOWSINTEROP && FEATURE_VISUALSTUDIOSETUP + // No try/catch around the COM call chain by design: AcquireSetupConfiguration2 + // and PopulateInstances check HRESULTs inline and return gracefully on failure. + // If a future maintainer introduces a ThrowOnFailure() in this path, add the + // matching catch — "empty list" is the documented behaviour for absent VS. try { - // This code is not obvious. See the sample (link above) for reference. - var query = (ISetupConfiguration2)GetQuery(); - var e = query.EnumAllInstances(); - - int fetched; - var instances = new ISetupInstance[1]; - do + using ComScope config = AcquireSetupConfiguration2(); + if (!config.IsNull) { - // Call e.Next to query for the next instance (single item or nothing returned). - e.Next(1, instances, out fetched); - if (fetched <= 0) - { - continue; - } - - var instance = instances[0]; - var state = ((ISetupInstance2)instance).GetState(); - Version version; - - try - { - version = new Version(instance.GetInstallationVersion()); - } - catch (FormatException) - { - continue; - } - - // If the install was complete and a valid version, consider it. - if (state == InstanceState.Complete) - { - validInstances.Add(new VisualStudioInstance( - instance.GetDisplayName(), - instance.GetInstallationPath(), - version)); - } - } while (fetched > 0); + PopulateInstances(config.Pointer, validInstances); + } } - catch (COMException) - { } catch (DllNotFoundException) { - // This is OK, VS "15" or greater likely not installed. + // The Setup Configuration native helper isn't on disk — VS "15" or newer + // is likely not installed. Fall through with whatever has been collected. } #endif + return validInstances; } -#if FEATURE_VISUALSTUDIOSETUP - private static ISetupConfiguration GetQuery() +#if FEATURE_WINDOWSINTEROP && FEATURE_VISUALSTUDIOSETUP + /// + /// Try to obtain a top-level pointer. First attempts + /// CoCreateInstance on the registered coclass; on + /// falls back to the app-local + /// GetSetupConfiguration entry point exported by + /// Microsoft.VisualStudio.Setup.Configuration.Native.dll (which requires that + /// helper DLL to be loadable but doesn't require COM registration). Any other COM + /// failure is returned as a null scope — the caller treats absent VS Setup as an + /// empty result, not an error. + /// + private static ComScope AcquireSetupConfiguration2() { - try + Guid clsid = SetupConfiguration.CLSID_SetupConfiguration; + Guid iidConfig2 = ISetupConfiguration2.IID_ISetupConfiguration2; + + ComScope config2 = new(); + HRESULT hr = PInvoke.CoCreateInstance(&clsid, pUnkOuter: null, CLSCTX.CLSCTX_INPROC_SERVER, &iidConfig2, config2); + if (hr.Succeeded) + { + return config2; + } + + if (hr != HRESULT.REGDB_E_CLASSNOTREG) + { + // Some other COM failure. The caller swallows errors, so signal "no result" + // by returning an empty scope rather than constructing a COMException only + // to have it discarded. CoCreateInstance must null the out-parameter on + // failure per the COM spec, but Dispose defensively so the ownership model + // is explicit even against a buggy COM implementation. + config2.Dispose(); + return default; + } + + // App-local fallback: the helper DLL is present but not COM-registered. + using ComScope config1 = new(); + int rawHr = SetupConfiguration.GetSetupConfiguration(config1, IntPtr.Zero); + if (rawHr < 0 || config1.IsNull) + { + return default; + } + + HRESULT qiHr = config1.Pointer->QueryInterface(&iidConfig2, config2); + if (qiHr.Failed) { - // Try to CoCreate the class object. - return new SetupConfiguration(); + config2.Dispose(); + return default; } - catch (COMException ex) when (ex.ErrorCode == HRESULT.REGDB_E_CLASSNOTREG) + + return config2; + } + + /// + /// Enumerate every instance the configuration exposes and append the complete ones + /// to . Per-instance failures are silent — a partial/broken + /// install simply does not appear in the output. + /// + private static void PopulateInstances(ISetupConfiguration2* config, List results) + { + using ComScope enumInstances = new(); + if (config->EnumAllInstances(enumInstances).Failed || enumInstances.IsNull) { - // Try to get the class object using app-local call. - ISetupConfiguration query; - var result = GetSetupConfiguration(out query, IntPtr.Zero); + return; + } - if (result < 0) + while (true) + { + using ComScope instance = new(); + uint fetched = 0; + if (enumInstances.Pointer->Next(1, instance, &fetched).Failed + || fetched == 0 + || instance.IsNull) { - throw new COMException("Failed to get query", result); + break; } - return query; + if (TryReadInstance(instance.Pointer, out VisualStudioInstance vs)) + { + results.Add(vs); + } } } - [DllImport("Microsoft.VisualStudio.Setup.Configuration.Native.dll", ExactSpelling = true, PreserveSig = true)] - private static extern int GetSetupConfiguration( - [MarshalAs(UnmanagedType.Interface), Out] out ISetupConfiguration configuration, - IntPtr reserved); + /// + /// Read one : filter by , + /// parse the version string, and capture the display name and installation path. Returns + /// when the instance should be skipped (incomplete install, + /// missing v2 interface, or unparseable version). + /// + private static bool TryReadInstance(ISetupInstance* instance, out VisualStudioInstance result) + { + result = null!; + + // QI to ISetupInstance2 so we can read the install state. Instances that lack v2 are + // ignored — only VS 15 and newer ship the v2 interface, which matches what the old + // RCW-based code required for `GetState`. + using ComScope instance2 = new(); + Guid iidInstance2 = ISetupInstance2.IID_ISetupInstance2; + if (instance->QueryInterface(&iidInstance2, instance2).Failed || instance2.IsNull) + { + return false; + } + + InstanceState state = default; + if (instance2.Pointer->GetState(&state).Failed || state != InstanceState.Complete) + { + return false; + } + + // BSTR is IDisposable: `using` calls SysFreeString in place at scope exit, so we + // don't need a try/finally around each of the three out-params. + using BSTR versionBstr = default; + using BSTR nameBstr = default; + using BSTR pathBstr = default; + + if (instance->GetInstallationVersion(&versionBstr).Failed + || !Version.TryParse(versionBstr.ToString(), out Version version)) + { + return false; + } + + if (instance->GetDisplayName(0, &nameBstr).Failed + || instance->GetInstallationPath(&pathBstr).Failed) + { + return false; + } + + result = new VisualStudioInstance(nameBstr.ToString(), pathBstr.ToString(), version); + return true; + } #endif } diff --git a/src/Shared/InprocTrackingNativeMethods.cs b/src/Shared/InprocTrackingNativeMethods.cs index fd8293d841d..fb162044b5f 100644 --- a/src/Shared/InprocTrackingNativeMethods.cs +++ b/src/Shared/InprocTrackingNativeMethods.cs @@ -4,222 +4,229 @@ #if FEATURE_FILE_TRACKER using System; +using System.Buffers; using System.IO; using System.Runtime.InteropServices; -#if FEATURE_CONSTRAINED_EXECUTION -using System.Runtime.ConstrainedExecution; -#endif using System.Security; +using System.Text; using Microsoft.Build.Shared.FileSystem; -#if FEATURE_RESOURCE_EXPOSURE -using System.Runtime.Versioning; -#endif +using Windows.Win32; +using Windows.Win32.Foundation; #nullable disable namespace Microsoft.Build.Shared { /// - /// Methods that are invoked on FileTracker.dll in order to handle inproc tracking + /// Managed shim over FileTracker.dll — the native Detours-based file-access tracker + /// shipped alongside MSBuild on .NET Framework. /// - /// - /// We want to P/Invoke to the FileTracker methods, but FileTracker.dll is not guaranteed to be on PATH (since it's - /// in the MSBuild directory), and there is no DefaultDllImportSearchPath that explicitly points to us. Thus, we - /// are sneaking around P/Invoke by manually acquiring the method pointers and calling them ourselves. The vast - /// majority of this code was lifted from ndp\fx\src\CLRCompression\ZLibNative.cs, which does the same thing for - /// that assembly. - /// - internal static class InprocTrackingNativeMethods + /// + /// FileTracker.dll is not on PATH (it lives in the MSBuild tools directory) and there is + /// no DllImportSearchPath that explicitly points to it, so the module is loaded by + /// absolute path via LoadLibrary and each export is resolved through + /// GetProcAddress into a typed delegate* unmanaged[Stdcall] function pointer. + /// Slot table from FileTracker.def: StartTrackingContext @2, + /// StartTrackingContextWithRoot @3, EndTrackingContext @4, + /// StopTrackingAndCleanup @5, SuspendTracking @6, ResumeTracking @7, + /// WriteAllTLogs @8, WriteContextTLogs @9, SetThreadCount @10. + /// All exports are HRESULT WINAPI Name(...) with Unicode LPCTSTR arguments + /// (FileTracker.dll is built UNICODE), so the function-pointer signatures take + /// and return . + /// + internal static unsafe class InprocTrackingNativeMethods { -#pragma warning disable format // region formatting is different in net7.0 and net472, and cannot be fixed for both - #region Delegates for the tracking functions - - [UnmanagedFunctionPointer(CallingConvention.StdCall)] -#if FEATURE_SECURITY_PERMISSIONS - [SuppressUnmanagedCodeSecurity] -#endif - private delegate int StartTrackingContextDelegate([In, MarshalAs(UnmanagedType.LPWStr)] string intermediateDirectory, [In, MarshalAs(UnmanagedType.LPWStr)] string taskName); - - [UnmanagedFunctionPointer(CallingConvention.StdCall)] -#if FEATURE_SECURITY_PERMISSIONS - [SuppressUnmanagedCodeSecurity] -#endif - private delegate int StartTrackingContextWithRootDelegate([In, MarshalAs(UnmanagedType.LPWStr)] string intermediateDirectory, [In, MarshalAs(UnmanagedType.LPWStr)] string taskName, [In, MarshalAs(UnmanagedType.LPWStr)] string rootMarker); - - [UnmanagedFunctionPointer(CallingConvention.StdCall)] -#if FEATURE_SECURITY_PERMISSIONS - [SuppressUnmanagedCodeSecurity] -#endif - private delegate int EndTrackingContextDelegate(); - - [UnmanagedFunctionPointer(CallingConvention.StdCall)] -#if FEATURE_SECURITY_PERMISSIONS - [SuppressUnmanagedCodeSecurity] -#endif - private delegate int StopTrackingAndCleanupDelegate(); - - [UnmanagedFunctionPointer(CallingConvention.StdCall)] -#if FEATURE_SECURITY_PERMISSIONS - [SuppressUnmanagedCodeSecurity] -#endif - private delegate int SuspendTrackingDelegate(); - - [UnmanagedFunctionPointer(CallingConvention.StdCall)] -#if FEATURE_SECURITY_PERMISSIONS - [SuppressUnmanagedCodeSecurity] -#endif - private delegate int ResumeTrackingDelegate(); - - [UnmanagedFunctionPointer(CallingConvention.StdCall)] -#if FEATURE_SECURITY_PERMISSIONS - [SuppressUnmanagedCodeSecurity] -#endif - private delegate int WriteAllTLogsDelegate([In, MarshalAs(UnmanagedType.LPWStr)] string intermediateDirectory, [In, MarshalAs(UnmanagedType.LPWStr)] string tlogRootName); - - [UnmanagedFunctionPointer(CallingConvention.StdCall)] -#if FEATURE_SECURITY_PERMISSIONS - [SuppressUnmanagedCodeSecurity] -#endif - private delegate int WriteContextTLogsDelegate([In, MarshalAs(UnmanagedType.LPWStr)] string intermediateDirectory, [In, MarshalAs(UnmanagedType.LPWStr)] string tlogRootName); - - [UnmanagedFunctionPointer(CallingConvention.StdCall)] -#if FEATURE_SECURITY_PERMISSIONS - [SuppressUnmanagedCodeSecurity] -#endif - private delegate int SetThreadCountDelegate(int threadCount); - - #endregion // Delegates for the tracking functions - #region Public API - internal static void StartTrackingContext([In, MarshalAs(UnmanagedType.LPWStr)] string intermediateDirectory, [In, MarshalAs(UnmanagedType.LPWStr)] string taskName) + /// + /// Begin a new tracking context on the current thread. File accesses on this thread + /// (and any worker threads pre-allocated via ) will + /// accumulate into per-context read/write/delete TLog lists keyed by + /// until is called. + /// + /// Directory in which TLog files for this context will be written when / is called. + /// Logical task name; used as the TLog file-name prefix and as the key in the context stack. + internal static void StartTrackingContext(string intermediateDirectory, string taskName) { - int hresult = FileTrackerDllStub.startTrackingContextDelegate(intermediateDirectory, taskName); - Marshal.ThrowExceptionForHR(hresult, new IntPtr(-1)); + fixed (char* pDir = intermediateDirectory) + { + fixed (char* pTask = taskName) + { + FileTrackerDllStub.StartTrackingContext(new PCWSTR(pDir), new PCWSTR(pTask)).ThrowOnFailure(); + } + } } - internal static void StartTrackingContextWithRoot([In, MarshalAs(UnmanagedType.LPWStr)] string intermediateDirectory, [In, MarshalAs(UnmanagedType.LPWStr)] string taskName, [In, MarshalAs(UnmanagedType.LPWStr)] string rootMarker) + /// + /// Same as but also supplies a response file from + /// which the rooting marker (the canonical "this is the build root" identifier + /// embedded in TLog headers) is read. + /// + /// Output directory for TLogs. + /// Logical task name (TLog file-name prefix). + /// Path to a response file containing the rooting-marker string. + internal static void StartTrackingContextWithRoot(string intermediateDirectory, string taskName, string rootMarker) { - int hresult = FileTrackerDllStub.startTrackingContextWithRootDelegate(intermediateDirectory, taskName, rootMarker); - Marshal.ThrowExceptionForHR(hresult, new IntPtr(-1)); + fixed (char* pDir = intermediateDirectory) + { + fixed (char* pTask = taskName) + { + fixed (char* pRoot = rootMarker) + { + FileTrackerDllStub.StartTrackingContextWithRoot(new PCWSTR(pDir), new PCWSTR(pTask), new PCWSTR(pRoot)).ThrowOnFailure(); + } + } + } } + /// + /// Pop the top-most tracking context off the current thread's context stack. After + /// this returns, file accesses on the thread fall through to the next context on the + /// stack (if any) or stop being tracked. + /// internal static void EndTrackingContext() { - int hresult = FileTrackerDllStub.endTrackingContextDelegate(); - Marshal.ThrowExceptionForHR(hresult, new IntPtr(-1)); + FileTrackerDllStub.EndTrackingContext().ThrowOnFailure(); } + /// + /// Tear down every tracking context on every tracked thread and release the + /// process-wide tracking data structures. Tracking is fully disabled after this call + /// until a fresh reactivates it. The FileTracker + /// module itself remains loaded (it must — see remarks on the class). + /// internal static void StopTrackingAndCleanup() { - int hresult = FileTrackerDllStub.stopTrackingAndCleanupDelegate(); - Marshal.ThrowExceptionForHR(hresult, new IntPtr(-1)); + FileTrackerDllStub.StopTrackingAndCleanup().ThrowOnFailure(); } + /// + /// Temporarily pause recording into the current thread's active tracking context. + /// File accesses between this call and are not added to + /// the context's TLog lists. + /// internal static void SuspendTracking() { - int hresult = FileTrackerDllStub.suspendTrackingDelegate(); - Marshal.ThrowExceptionForHR(hresult, new IntPtr(-1)); + FileTrackerDllStub.SuspendTracking().ThrowOnFailure(); } + /// + /// Resume recording into the current thread's active tracking context after a prior + /// . + /// internal static void ResumeTracking() { - int hresult = FileTrackerDllStub.resumeTrackingDelegate(); - Marshal.ThrowExceptionForHR(hresult, new IntPtr(-1)); + FileTrackerDllStub.ResumeTracking().ThrowOnFailure(); } - internal static void WriteAllTLogs([In, MarshalAs(UnmanagedType.LPWStr)] string intermediateDirectory, [In, MarshalAs(UnmanagedType.LPWStr)] string tlogRootName) + /// + /// Flush the accumulated read/write/delete TLogs for every tracking context across + /// every tracked thread to disk under , using + /// as the file-name prefix. The contexts remain live + /// and continue to accumulate entries after this call. + /// + /// Output directory for the TLogs. + /// File-name prefix for the produced *.read.tlog / *.write.tlog / *.delete.tlog files. + internal static void WriteAllTLogs(string intermediateDirectory, string tlogRootName) { - int hresult = FileTrackerDllStub.writeAllTLogsDelegate(intermediateDirectory, tlogRootName); - Marshal.ThrowExceptionForHR(hresult, new IntPtr(-1)); + fixed (char* pDir = intermediateDirectory) + { + fixed (char* pRoot = tlogRootName) + { + FileTrackerDllStub.WriteAllTLogs(new PCWSTR(pDir), new PCWSTR(pRoot)).ThrowOnFailure(); + } + } } - internal static void WriteContextTLogs([In, MarshalAs(UnmanagedType.LPWStr)] string intermediateDirectory, [In, MarshalAs(UnmanagedType.LPWStr)] string tlogRootName) + /// + /// Flush the read/write/delete TLogs for just the current thread's active tracking + /// context to disk and clear those in-memory lists (so subsequent activity in the same + /// context is recorded into freshly empty lists). + /// + /// Output directory for the TLogs. + /// File-name prefix for the produced TLog files. + internal static void WriteContextTLogs(string intermediateDirectory, string tlogRootName) { - int hresult = FileTrackerDllStub.writeContextTLogsDelegate(intermediateDirectory, tlogRootName); - Marshal.ThrowExceptionForHR(hresult, new IntPtr(-1)); + fixed (char* pDir = intermediateDirectory) + { + fixed (char* pRoot = tlogRootName) + { + FileTrackerDllStub.WriteContextTLogs(new PCWSTR(pDir), new PCWSTR(pRoot)).ThrowOnFailure(); + } + } } + /// + /// Declare to FileTracker how many worker threads the host intends to use. FileTracker + /// pre-allocates per-thread TLS slots / counters from this value so that worker + /// threads can attach their own tracking contexts without taking a global lock at + /// runtime. + /// + /// Maximum concurrent worker-thread count. internal static void SetThreadCount(int threadCount) { - int hresult = FileTrackerDllStub.setThreadCountDelegate(threadCount); - Marshal.ThrowExceptionForHR(hresult, new IntPtr(-1)); + FileTrackerDllStub.SetThreadCount(threadCount).ThrowOnFailure(); } #endregion // Public API -#pragma warning restore format + private static class FileTrackerDllStub { - private static readonly Lazy fileTrackerDllName = new Lazy(() => RuntimeInformation.ProcessArchitecture == Architecture.Arm64 ? "FileTrackerA4.dll" : (IntPtr.Size == sizeof(Int32)) ? "FileTracker32.dll" : "FileTracker64.dll"); - - // Handle for FileTracker.dll itself - [SecurityCritical] - private static SafeHandle s_fileTrackerDllHandle; -#pragma warning disable format // region formatting is different in net7.0 and net472, and cannot be fixed for both - #region Function pointers to native functions - - internal static StartTrackingContextDelegate startTrackingContextDelegate; - - internal static StartTrackingContextWithRootDelegate startTrackingContextWithRootDelegate; - - internal static EndTrackingContextDelegate endTrackingContextDelegate; - - internal static StopTrackingAndCleanupDelegate stopTrackingAndCleanupDelegate; - - internal static SuspendTrackingDelegate suspendTrackingDelegate; - - internal static ResumeTrackingDelegate resumeTrackingDelegate; - - internal static WriteAllTLogsDelegate writeAllTLogsDelegate; - - internal static WriteContextTLogsDelegate writeContextTLogsDelegate; - - internal static SetThreadCountDelegate setThreadCountDelegate; - - #endregion // Function pointers to native functions - - #region Declarations of Windows API needed to load the native library - - [DllImport("kernel32.dll", CharSet = CharSet.Ansi, BestFitMapping = false)] -#if FEATURE_RESOURCE_EXPOSURE - [ResourceExposure(ResourceScope.Process)] -#endif + // Architecture-specific FileTracker DLL name. The native module is built per + // architecture (x86 / x64 / arm64); the host process must load the variant whose + // bitness and ISA match its own so that Detours can hook the right import table. + private static readonly Lazy s_fileTrackerDllName = new(() => + RuntimeInformation.ProcessArchitecture == Architecture.Arm64 + ? "FileTrackerA4.dll" + : IntPtr.Size == sizeof(int) + ? "FileTracker32.dll" + : "FileTracker64.dll"); + + // Handle for FileTracker.dll itself. The HMODULE is intentionally never freed: + // FileTracker detours ExitProcess and forcibly unloading it would corrupt CLR + // shutdown (matching the historical SafeLibraryHandle.ReleaseHandle no-op). [SecurityCritical] - private static extern IntPtr GetProcAddress(SafeHandle moduleHandle, String procName); - - [DllImport("kernel32.dll", CharSet = CharSet.Unicode, SetLastError = true)] -#if FEATURE_RESOURCE_EXPOSURE - [ResourceExposure(ResourceScope.Machine)] -#endif - [SecurityCritical] - private static extern SafeLibraryHandle LoadLibrary(String libPath); - - #endregion // Declarations of Windows API needed to load the native library - - #region Initialization code + private static HMODULE s_fileTrackerDllHandle; + + // Function-pointer slots, populated by InitDelegates() from the LoadLibrary'd + // module via GetProcAddress. Signatures track the native exports verbatim: + // every export is __stdcall HRESULT-returning and takes Unicode LPCWSTR arguments + // (FileTracker.dll is built UNICODE). + internal static delegate* unmanaged[Stdcall] StartTrackingContext; + internal static delegate* unmanaged[Stdcall] StartTrackingContextWithRoot; + internal static delegate* unmanaged[Stdcall] EndTrackingContext; + internal static delegate* unmanaged[Stdcall] StopTrackingAndCleanup; + internal static delegate* unmanaged[Stdcall] SuspendTracking; + internal static delegate* unmanaged[Stdcall] ResumeTracking; + internal static delegate* unmanaged[Stdcall] WriteAllTLogs; + internal static delegate* unmanaged[Stdcall] WriteContextTLogs; + internal static delegate* unmanaged[Stdcall] SetThreadCount; /// - /// Loads FileTracker.dll into a handle that we can use subsequently to grab the exported methods we're interested in. + /// Loads FileTracker.dll from the MSBuild tools directory into + /// . /// private static void LoadFileTrackerDll() { - // Get the FileTracker in our directory that matches the currently running process string buildToolsPath = FrameworkLocationHelper.GeneratePathToBuildToolsForToolsVersion(MSBuildConstants.CurrentToolsVersion, DotNetFrameworkArchitecture.Current); - string fileTrackerPath = Path.Combine(buildToolsPath, fileTrackerDllName.Value); + string fileTrackerPath = Path.Combine(buildToolsPath, s_fileTrackerDllName.Value); if (!FileSystems.Default.FileExists(fileTrackerPath)) { - throw new DllNotFoundException(fileTrackerDllName.Value); + throw new DllNotFoundException(s_fileTrackerDllName.Value); } - SafeLibraryHandle handle = LoadLibrary(fileTrackerPath); + HMODULE handle; + fixed (char* pPath = fileTrackerPath) + { + handle = PInvoke.LoadLibrary(new PCWSTR(pPath)); + } - if (handle.IsInvalid) + if (handle.IsNull) { - Int32 hresult = Marshal.GetHRForLastWin32Error(); - Marshal.ThrowExceptionForHR(hresult, new IntPtr(-1)); + Marshal.ThrowExceptionForHR(Marshal.GetHRForLastWin32Error(), new IntPtr(-1)); - // If Marshal.ThrowExceptionForHR did not throw, we still need to make sure to throw: + // If Marshal.ThrowExceptionForHR did not throw, force a throw here. throw new InvalidOperationException(); } @@ -227,44 +234,70 @@ private static void LoadFileTrackerDll() } /// - /// Generic code to grab the function pointer for a function exported by FileTracker.dll, given - /// that function's name, and transform that function pointer into a callable delegate. + /// Resolve from the loaded FileTracker module + /// and return its address as an unmanaged function pointer. /// [SecurityCritical] - private static DT CreateDelegate
(String entryPointName) + private static void* GetExport(string entryPointName) { - IntPtr entryPoint = GetProcAddress(s_fileTrackerDllHandle, entryPointName); + // GetProcAddress takes a system-default-code-page (ANSI) string. Encode through + // the real encoder so DBCS / multi-byte characters in an unusual export name + // are handled correctly. Rent from ArrayPool to avoid a per-call allocation. + // + // Encoding.Default returns the system ANSI code page on .NET Framework + // (correct for GetProcAddress) but UTF-8 on .NET (Core). FEATURE_FILE_TRACKER + // is only defined for net472 so this branch is correct as-is; if the #if + // guard ever broadens to .NET (Core), switch to a CodePagesEncodingProvider + // ANSI encoding instead. + Encoding ansi = Encoding.Default; + int maxByteCount = ansi.GetMaxByteCount(entryPointName.Length) + 1; + byte[] buffer = ArrayPool.Shared.Rent(maxByteCount); + try + { + int written = ansi.GetBytes(entryPointName, 0, entryPointName.Length, buffer, 0); + buffer[written] = 0; + + FARPROC entryPoint; + fixed (byte* pName = buffer) + { + entryPoint = PInvoke.GetProcAddress(s_fileTrackerDllHandle, new PCSTR(pName)); + } + + if (entryPoint.IsNull) + { + throw new EntryPointNotFoundException(s_fileTrackerDllName.Value + "!" + entryPointName); + } - if (IntPtr.Zero == entryPoint) + return (void*)entryPoint.Value; + } + finally { - throw new EntryPointNotFoundException(fileTrackerDllName.Value + "!" + entryPointName); + ArrayPool.Shared.Return(buffer); } - - return (DT)(Object)Marshal.GetDelegateForFunctionPointer(entryPoint, typeof(DT)); } /// - /// Actually generate all of the delegates that will be called by our public (or rather, internal) surface area methods. + /// Resolve every export FileTracker.dll publishes that the managed shim cares + /// about, populating the function-pointer fields. /// private static void InitDelegates() { - ErrorUtilities.VerifyThrow(s_fileTrackerDllHandle != null, "fileTrackerDllHandle should not be null"); - ErrorUtilities.VerifyThrow(!s_fileTrackerDllHandle.IsInvalid, "Handle for FileTracker.dll should not be invalid"); - - startTrackingContextDelegate = CreateDelegate("StartTrackingContext"); - startTrackingContextWithRootDelegate = CreateDelegate("StartTrackingContextWithRoot"); - endTrackingContextDelegate = CreateDelegate("EndTrackingContext"); - stopTrackingAndCleanupDelegate = CreateDelegate("StopTrackingAndCleanup"); - suspendTrackingDelegate = CreateDelegate("SuspendTracking"); - resumeTrackingDelegate = CreateDelegate("ResumeTracking"); - writeAllTLogsDelegate = CreateDelegate("WriteAllTLogs"); - writeContextTLogsDelegate = CreateDelegate("WriteContextTLogs"); - setThreadCountDelegate = CreateDelegate("SetThreadCount"); + ErrorUtilities.VerifyThrow(!s_fileTrackerDllHandle.IsNull, "Handle for FileTracker.dll should not be null"); + + StartTrackingContext = (delegate* unmanaged[Stdcall])GetExport(nameof(StartTrackingContext)); + StartTrackingContextWithRoot = (delegate* unmanaged[Stdcall])GetExport(nameof(StartTrackingContextWithRoot)); + EndTrackingContext = (delegate* unmanaged[Stdcall])GetExport(nameof(EndTrackingContext)); + StopTrackingAndCleanup = (delegate* unmanaged[Stdcall])GetExport(nameof(StopTrackingAndCleanup)); + SuspendTracking = (delegate* unmanaged[Stdcall])GetExport(nameof(SuspendTracking)); + ResumeTracking = (delegate* unmanaged[Stdcall])GetExport(nameof(ResumeTracking)); + WriteAllTLogs = (delegate* unmanaged[Stdcall])GetExport(nameof(WriteAllTLogs)); + WriteContextTLogs = (delegate* unmanaged[Stdcall])GetExport(nameof(WriteContextTLogs)); + SetThreadCount = (delegate* unmanaged[Stdcall])GetExport(nameof(SetThreadCount)); } /// - /// Static constructor -- generates the delegates for all of the export methods from - /// FileTracker.dll that we care about. + /// Type initializer: loads the native module and resolves every export once + /// before any of the function-pointer fields are read. /// [SecuritySafeCritical] static FileTrackerDllStub() @@ -272,35 +305,6 @@ static FileTrackerDllStub() LoadFileTrackerDll(); InitDelegates(); } - - #endregion // Initialization code -#pragma warning restore format - // Specialized handle to make sure we free FileTracker.dll - [SecurityCritical] - private class SafeLibraryHandle : SafeHandle - { - internal SafeLibraryHandle() - : base(IntPtr.Zero, true) - { - } - - public override bool IsInvalid - { - [SecurityCritical] - get - { return IntPtr.Zero == handle; } - } -#if FEATURE_CONSTRAINED_EXECUTION - [ReliabilityContract(Consistency.WillNotCorruptState, Cer.MayFail)] -#endif - [SecurityCritical] - protected override bool ReleaseHandle() - { - // FileTracker expects to continue to exist even through ExitProcess -- if we forcibly unload it now, - // bad things can happen when the CLR attempts to call the (still detoured?) ExitProcess. - return true; - } - } // private class SafeLibraryHandle } } } diff --git a/src/Tasks.UnitTests/AddToWin32Manifest_Tests.cs b/src/Tasks.UnitTests/AddToWin32Manifest_Tests.cs index c724deb8221..bff1015a2f5 100644 --- a/src/Tasks.UnitTests/AddToWin32Manifest_Tests.cs +++ b/src/Tasks.UnitTests/AddToWin32Manifest_Tests.cs @@ -14,6 +14,7 @@ #if FEATURE_WINDOWSINTEROP using Windows.Win32; using Windows.Win32.Foundation; +using Windows.Win32.System.LibraryLoader; #endif namespace Microsoft.Build.Tasks.UnitTests @@ -157,40 +158,36 @@ public void E2EScenarioTests(string? manifestName, bool expectedResult) [SupportedOSPlatform("windows6.1")] internal sealed class AssemblyNativeResourceManager { - public enum LoadLibraryFlags : uint { LOAD_LIBRARY_AS_DATAFILE = 2 }; - - [DllImport("Kernel32.dll", CharSet = CharSet.Auto, SetLastError = true)] - public static extern IntPtr LoadLibrary(string lpFileName, IntPtr hReservedNull, LoadLibraryFlags dwFlags); - - [DllImport("kernel32.dll", SetLastError = true)] - public static extern IntPtr FindResource(IntPtr hModule, string lpName, string lpType); - - [DllImport("kernel32.dll", SetLastError = true)] - public static extern IntPtr LoadResource(IntPtr hModule, IntPtr hResInfo); - - [DllImport("kernel32.dll", SetLastError = true)] - public static extern IntPtr LockResource(IntPtr hResData); - - [DllImport("kernel32.dll", SetLastError = true)] - public static extern uint SizeofResource(IntPtr hModule, IntPtr hResInfo); - - public static byte[]? GetResourceFromExecutable(string assembly, string lpName, string lpType) + public static unsafe byte[]? GetResourceFromExecutable(string assembly, string lpName, string lpType) { - IntPtr hModule = LoadLibrary(assembly, IntPtr.Zero, LoadLibraryFlags.LOAD_LIBRARY_AS_DATAFILE); + HMODULE hModule; + fixed (char* pAssembly = assembly) + { + hModule = PInvoke.LoadLibraryEx(new PCWSTR(pAssembly), default, LOAD_LIBRARY_FLAGS.LOAD_LIBRARY_AS_DATAFILE); + } + try { - if (hModule != IntPtr.Zero) + if (!hModule.IsNull) { - IntPtr hResource = FindResource(hModule, lpName, lpType); - if (hResource != IntPtr.Zero) + HRSRC hResource; + fixed (char* pName = lpName) + { + fixed (char* pType = lpType) + { + hResource = PInvoke.FindResource(hModule, new PCWSTR(pName), new PCWSTR(pType)); + } + } + + if (!hResource.IsNull) { - uint resSize = SizeofResource(hModule, hResource); - IntPtr resData = LoadResource(hModule, hResource); - if (resData != IntPtr.Zero) + uint resSize = PInvoke.SizeofResource(hModule, hResource); + HGLOBAL resData = PInvoke.LoadResource(hModule, hResource); + if (!resData.IsNull) { byte[] uiBytes = new byte[resSize]; - IntPtr ipMemorySource = LockResource(resData); - Marshal.Copy(ipMemorySource, uiBytes, 0, (int)resSize); + void* pMemorySource = PInvoke.LockResource(resData); + Marshal.Copy((IntPtr)pMemorySource, uiBytes, 0, (int)resSize); return uiBytes; } @@ -199,7 +196,7 @@ public enum LoadLibraryFlags : uint { LOAD_LIBRARY_AS_DATAFILE = 2 }; } finally { - PInvoke.FreeLibrary((HMODULE)hModule); + PInvoke.FreeLibrary(hModule); } return null; diff --git a/src/Tasks.UnitTests/AssemblyInformation_Tests.cs b/src/Tasks.UnitTests/AssemblyInformation_Tests.cs new file mode 100644 index 00000000000..ab8a93506f4 --- /dev/null +++ b/src/Tasks.UnitTests/AssemblyInformation_Tests.cs @@ -0,0 +1,93 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.IO; +using System.Reflection; +using System.Runtime.Versioning; +using Microsoft.Build.Tasks; +using Microsoft.Build.Tasks.AssemblyDependency; +using Microsoft.Build.UnitTests; +using Shouldly; +using Xunit; + +namespace Microsoft.Build.Tasks.UnitTests +{ + /// + /// Targeted real-file tests for — the CLR-metadata-backed + /// path that the CsWin32 struct-based COM migration affects most directly. These exercise the + /// CoCreateInstance / OpenScope / QueryInterface / GetAssemblyProps / ASSEMBLYMETADATA / + /// GetCustomAttributeByName / GetPEKind call chain end-to-end on net472, and the + /// System.Reflection.Metadata fallback path on net10. + /// + public sealed class AssemblyInformation_Tests + { + /// Path of this test assembly — guaranteed to exist and to be a managed PE. + private static string ThisAssemblyPath => typeof(AssemblyInformation_Tests).Assembly.Location; + + [WindowsOnlyFact] + public void GetTargetFrameworkAttribute_RealAssembly_MatchesReflection() + { + // Every modern SDK-built assembly carries [assembly: TargetFrameworkAttribute("...")]; + // the value returned by AssemblyInformation must match what reflection sees on the + // exact same file. This covers the borrowed IMetaDataImport2* path that lives outside + // GetAssemblyMetadata() (the FrameworkNameAttribute property). + var attribute = typeof(AssemblyInformation_Tests).Assembly + .GetCustomAttribute(); + attribute.ShouldNotBeNull(); + + FrameworkName actual = AssemblyInformation.GetTargetFrameworkAttribute(ThisAssemblyPath); + actual.ShouldNotBeNull(); + actual.FullName.ShouldBe(new FrameworkName(attribute.FrameworkName).FullName); + } + +#if !FEATURE_ASSEMBLYLOADCONTEXT + [WindowsOnlyFact] + public void GetAssemblyMetadata_RealAssembly_PopulatesAllFields() + { + // net472-only: exercises the full CLR-metadata COM path end-to-end: + // CoCreateInstance(CorMetaDataDispenser) -> OpenScope(IMetaDataImport2) + // -> QueryInterface(IMetaDataAssemblyImport) -> GetAssemblyFromScope + // -> GetAssemblyProps (name buffer + ASSEMBLYMETADATA + flags + public key) + // -> GetCustomAttributeByName x4 (description / TFM / guid / interop) + // -> GetPEKind. + // Compares the COM-sourced data against AssemblyName.GetAssemblyName(...) and the + // TargetFrameworkAttribute on the same file — these have to agree. + using var info = new AssemblyInformation(ThisAssemblyPath); + AssemblyAttributes attributes = info.GetAssemblyMetadata(); + + attributes.ShouldNotBeNull(); + attributes.IsAssembly.ShouldBeTrue(); + attributes.AssemblyFullPath.ShouldBe(ThisAssemblyPath); + + // Identity parity with AssemblyName.GetAssemblyName on the same file. + AssemblyName reflectedName = AssemblyName.GetAssemblyName(ThisAssemblyPath); + attributes.AssemblyName.ShouldBe(reflectedName.Name); + attributes.DefaultAlias.ShouldBe(reflectedName.Name); + attributes.MajorVersion.ShouldBe((ushort)reflectedName.Version.Major); + attributes.MinorVersion.ShouldBe((ushort)reflectedName.Version.Minor); + attributes.BuildNumber.ShouldBe((ushort)reflectedName.Version.Build); + attributes.RevisionNumber.ShouldBe((ushort)reflectedName.Version.Revision); + + // PublicHexKey is the full public key (not the token) converted via BitConverter; if the + // test assembly is signed the field is non-empty, otherwise it's the empty string. + byte[] reflectedPublicKey = reflectedName.GetPublicKey(); + if (reflectedPublicKey is { Length: > 0 }) + { + attributes.PublicHexKey.ShouldNotBeNullOrEmpty(); + } + + // TargetFrameworkMoniker comes from [TargetFrameworkAttribute] on the assembly. + var tfa = typeof(AssemblyInformation_Tests).Assembly.GetCustomAttribute(); + tfa.ShouldNotBeNull(); + attributes.TargetFrameworkMoniker.ShouldBe(tfa.FrameworkName); + + // PeKind comes from GetPEKind — any managed PE returns a non-zero combination of + // CorPEKind flags (peIL = 1 at minimum). + attributes.PeKind.ShouldNotBe(0u); + + // RuntimeVersion looks like "v4.0.30319" / "v4.0.0" — non-empty for any real managed PE. + attributes.RuntimeVersion.ShouldNotBeNullOrEmpty(); + } +#endif + } +} diff --git a/src/Tasks.UnitTests/MetadataReader_Tests.cs b/src/Tasks.UnitTests/MetadataReader_Tests.cs index c2c5178cd48..915a9a9f6eb 100644 --- a/src/Tasks.UnitTests/MetadataReader_Tests.cs +++ b/src/Tasks.UnitTests/MetadataReader_Tests.cs @@ -264,5 +264,37 @@ public void ConcurrentPropertyAccess_OnSameInstance_IsThreadSafe() Task.WaitAll(tasks); } + + [Fact] + public void HasAssemblyAttributes_Batch_AgreesWithSingular() + { + // The batch overload was added to share a single GIT acquisition + GetAssemblyFromScope + // across multiple probes on the net472 path. It must return the same per-name result + // the singular `HasAssemblyAttribute(name)` overload returns. Mixes a known-present + // attribute (every SDK-built assembly has TargetFrameworkAttribute) with two clearly + // bogus names so both true and false outcomes are exercised in one call. + using MetadataReader reader = MetadataReader.Create(ThisAssemblyPath); + reader.ShouldNotBeNull(); + + string[] names = + [ + "System.Runtime.Versioning.TargetFrameworkAttribute", + "Definitely.Not.A.Real.Attribute", + "Another.Missing.AttributeName", + ]; + + bool[] batch = new bool[names.Length]; + reader.HasAssemblyAttributes(names, batch); + + for (int i = 0; i < names.Length; i++) + { + batch[i].ShouldBe(reader.HasAssemblyAttribute(names[i]), $"mismatch on '{names[i]}'"); + } + + // Sanity: the well-known one must be present and the bogus ones must be absent. + batch[0].ShouldBeTrue(); + batch[1].ShouldBeFalse(); + batch[2].ShouldBeFalse(); + } } } diff --git a/src/Tasks.UnitTests/Microsoft.Build.Tasks.UnitTests.csproj b/src/Tasks.UnitTests/Microsoft.Build.Tasks.UnitTests.csproj index 1c1696e6440..d46ce18147b 100644 --- a/src/Tasks.UnitTests/Microsoft.Build.Tasks.UnitTests.csproj +++ b/src/Tasks.UnitTests/Microsoft.Build.Tasks.UnitTests.csproj @@ -9,6 +9,7 @@ Microsoft.Build.Tasks.UnitTests true $(DefineConstants);MICROSOFT_BUILD_TASKS_UNITTESTS + true diff --git a/src/Tasks/ManifestUtil/ApplicationManifest.cs b/src/Tasks/ManifestUtil/ApplicationManifest.cs index 22564f94bc6..fa0e7fa37a3 100644 --- a/src/Tasks/ManifestUtil/ApplicationManifest.cs +++ b/src/Tasks/ManifestUtil/ApplicationManifest.cs @@ -960,6 +960,14 @@ private class AssemblyAttributeFlags public readonly bool HasImportedFromTypeLibAttribute; public readonly bool HasSecurityTransparentAttribute; + private static readonly string[] s_attributeNames = + [ + "System.Security.AllowPartiallyTrustedCallersAttribute", + "System.Security.SecurityTransparentAttribute", + "System.Runtime.InteropServices.PrimaryInteropAssemblyAttribute", + "System.Runtime.InteropServices.ImportedFromTypeLibAttribute", + ]; + public AssemblyAttributeFlags(string path) { using (MetadataReader r = MetadataReader.Create(path)) @@ -967,10 +975,15 @@ public AssemblyAttributeFlags(string path) if (r != null) { IsSigned = !String.IsNullOrEmpty(r.PublicKeyToken); - HasAllowPartiallyTrustedCallersAttribute = r.HasAssemblyAttribute("System.Security.AllowPartiallyTrustedCallersAttribute"); - HasSecurityTransparentAttribute = r.HasAssemblyAttribute("System.Security.SecurityTransparentAttribute"); - HasPrimaryInteropAssemblyAttribute = r.HasAssemblyAttribute("System.Runtime.InteropServices.PrimaryInteropAssemblyAttribute"); - HasImportedFromTypeLibAttribute = r.HasAssemblyAttribute("System.Runtime.InteropServices.ImportedFromTypeLibAttribute"); + + // Batched on the net472/COM path to share a single GIT acquisition across + // all four probes; on the netcore path it iterates the cached attribute list. + bool[] results = new bool[s_attributeNames.Length]; + r.HasAssemblyAttributes(s_attributeNames, results); + HasAllowPartiallyTrustedCallersAttribute = results[0]; + HasSecurityTransparentAttribute = results[1]; + HasPrimaryInteropAssemblyAttribute = results[2]; + HasImportedFromTypeLibAttribute = results[3]; } } } diff --git a/src/Tasks/ManifestUtil/MetadataReader.cs b/src/Tasks/ManifestUtil/MetadataReader.cs index 420be57345c..83a481d0b34 100644 --- a/src/Tasks/ManifestUtil/MetadataReader.cs +++ b/src/Tasks/ManifestUtil/MetadataReader.cs @@ -61,6 +61,21 @@ public static MetadataReader Create(string path) } public bool HasAssemblyAttribute(string name) + { + EnsureCustomAttributes(); + return _customAttributes.Contains(name); + } + + public void HasAssemblyAttributes(string[] names, bool[] results) + { + EnsureCustomAttributes(); + for (int i = 0; i < names.Length; i++) + { + results[i] = _customAttributes.Contains(names[i]); + } + } + + private void EnsureCustomAttributes() { if (_customAttributes == null) { @@ -72,8 +87,6 @@ public bool HasAssemblyAttribute(string name) } } } - - return _customAttributes.Contains(name); } public string Name => Attributes[nameof(Name)]; @@ -340,16 +353,39 @@ public bool HasAssemblyAttribute(string name) using ComScope import2 = _import2.GetInterface(); MdAssembly assemblyScope; asmImport.Pointer->GetAssemblyFromScope(&assemblyScope).ThrowOnFailure(); + return HasAssemblyAttribute(import2.Pointer, assemblyScope, name); + } - // The CLR returns S_OK with pcbData=size when the attribute is present, S_FALSE with - // pcbData=0 when it is absent, and an error HRESULT otherwise. Treat anything that is - // not S_OK as "not present" so failure paths cannot leave valueLen indeterminate. + // Batch variant: callers (e.g. AssemblyAttributeFlags) that probe several attributes on + // the same reader pay a single GIT round-trip and a single GetAssemblyFromScope call + // instead of one per attribute. + public void HasAssemblyAttributes(string[] names, bool[] results) + { + using ComScope asmImport = _assemblyImport.GetInterface(); + using ComScope import2 = _import2.GetInterface(); + MdAssembly assemblyScope; + asmImport.Pointer->GetAssemblyFromScope(&assemblyScope).ThrowOnFailure(); + + for (int i = 0; i < names.Length; i++) + { + results[i] = HasAssemblyAttribute(import2.Pointer, assemblyScope, names[i]); + } + } + + // Takes a borrowed IMetaDataImport2* so callers in a hot path can reuse a single GIT + // round-trip instead of paying one per call. + // + // The CLR returns S_OK with pcbData=size when the attribute is present, S_FALSE with + // pcbData=0 when it is absent, and an error HRESULT otherwise. Treat anything that is + // not S_OK as "not present" so failure paths cannot leave valueLen indeterminate. + private static bool HasAssemblyAttribute(IMetaDataImport2* import2, MdAssembly assemblyScope, string name) + { void* valuePtr = null; uint valueLen = 0; HRESULT hr; fixed (char* pName = name) { - hr = import2.Pointer->GetCustomAttributeByName(assemblyScope, pName, &valuePtr, &valueLen); + hr = import2->GetCustomAttributeByName(assemblyScope, pName, &valuePtr, &valueLen); } return hr == HRESULT.S_OK && valueLen != 0; } diff --git a/src/Tasks/Microsoft.Build.Tasks.csproj b/src/Tasks/Microsoft.Build.Tasks.csproj index 4b7de4fb696..d072cd27b3d 100644 --- a/src/Tasks/Microsoft.Build.Tasks.csproj +++ b/src/Tasks/Microsoft.Build.Tasks.csproj @@ -646,9 +646,6 @@ - - - diff --git a/src/UnitTests.Shared/DriveMapping.cs b/src/UnitTests.Shared/DriveMapping.cs index c9034b6902a..41cb7588f39 100644 --- a/src/UnitTests.Shared/DriveMapping.cs +++ b/src/UnitTests.Shared/DriveMapping.cs @@ -4,6 +4,9 @@ #nullable enable using System.Runtime.InteropServices; using System.Runtime.Versioning; +using Windows.Win32; +using Windows.Win32.Foundation; +using Windows.Win32.Storage.FileSystem; namespace Microsoft.Build.UnitTests.Shared; @@ -11,8 +14,8 @@ internal static class DriveMapping { private const int ERROR_FILE_NOT_FOUND = 2; private const int ERROR_INSUFFICIENT_BUFFER = 122; - private const int DDD_REMOVE_DEFINITION = 2; - private const int DDD_NO_FLAG = 0; + private const DEFINE_DOS_DEVICE_FLAGS DDD_REMOVE_DEFINITION = (DEFINE_DOS_DEVICE_FLAGS)2; + private const DEFINE_DOS_DEVICE_FLAGS DDD_NO_FLAG = 0; // extra space for '\??\'. Not counting for long paths support in tests. private const int MAX_PATH = 259; @@ -21,10 +24,10 @@ internal static class DriveMapping ///
/// Drive letter /// Path to be mapped - [SupportedOSPlatform("windows")] + [SupportedOSPlatform("windows5.1.2600")] public static void MapDrive(char letter, string path) { - if (!DefineDosDevice(DDD_NO_FLAG, ToDeviceName(letter), path)) + if (!PInvoke.DefineDosDevice(DDD_NO_FLAG, ToDeviceName(letter), path)) { NativeMethodsShared.ThrowExceptionForErrorCode(Marshal.GetLastWin32Error()); } @@ -34,10 +37,10 @@ public static void MapDrive(char letter, string path) /// Windows specific. Unmaps drive mapping. /// /// Drive letter. - [SupportedOSPlatform("windows")] + [SupportedOSPlatform("windows5.1.2600")] public static void UnmapDrive(char letter) { - if (!DefineDosDevice(DDD_REMOVE_DEFINITION, ToDeviceName(letter), null)) + if (!PInvoke.DefineDosDevice(DDD_REMOVE_DEFINITION, ToDeviceName(letter), null)) { NativeMethodsShared.ThrowExceptionForErrorCode(Marshal.GetLastWin32Error()); } @@ -48,15 +51,28 @@ public static void UnmapDrive(char letter) /// /// Drive letter. /// Path mapped under specified letter. Empty string if mapping not found. - [SupportedOSPlatform("windows")] - public static string GetDriveMapping(char letter) + [SupportedOSPlatform("windows5.1.2600")] + public static unsafe string GetDriveMapping(char letter) { // since this is just for test purposes - let's not overcomplicate with long paths support char[] buffer = new char[MAX_PATH]; + string deviceName = ToDeviceName(letter); + uint length; - while (QueryDosDevice(ToDeviceName(letter), buffer, buffer.Length) == 0) + while (true) { - // Return empty string if the drive is not mapped + fixed (char* pBuf = buffer) + { + fixed (char* pDevice = deviceName) + { + length = PInvoke.QueryDosDevice(new PCWSTR(pDevice), new PWSTR(pBuf), (uint)buffer.Length); + } + } + if (length != 0) + { + break; + } + int err = Marshal.GetLastWin32Error(); if (err == ERROR_FILE_NOT_FOUND) { @@ -71,20 +87,14 @@ public static string GetDriveMapping(char letter) buffer = new char[buffer.Length * 4]; } - // Translate from the native path semantic - starting with '\??\' - return new string(buffer, 4, buffer.Length - 4); + // Translate from the native path semantic - starting with '\??\'. `length` is the + // number of characters QueryDosDevice copied INCLUDING the trailing NUL; trim the + // prefix (4) and the terminator (1) to get the managed string content. + return new string(buffer, 4, (int)length - 5); } private static string ToDeviceName(char letter) { return $"{char.ToUpper(letter)}:"; } - - [DllImport("kernel32.dll", CharSet = CharSet.Auto, SetLastError = true)] - [SupportedOSPlatform("windows")] - private static extern bool DefineDosDevice([In] int flags, [In] string deviceName, [In] string? path); - - [DllImport("kernel32.dll", CharSet = CharSet.Auto, SetLastError = true)] - [SupportedOSPlatform("windows")] - private static extern int QueryDosDevice([In] string deviceName, [Out] char[] buffer, [In] int bufSize); } diff --git a/src/Utilities.UnitTests/Microsoft.Build.Utilities.UnitTests.csproj b/src/Utilities.UnitTests/Microsoft.Build.Utilities.UnitTests.csproj index fc358d0663e..c3d9235a4a8 100644 --- a/src/Utilities.UnitTests/Microsoft.Build.Utilities.UnitTests.csproj +++ b/src/Utilities.UnitTests/Microsoft.Build.Utilities.UnitTests.csproj @@ -6,6 +6,7 @@ Microsoft.Build.Utilities.UnitTests true Microsoft.Build.Utilities.UnitTests + true @@ -28,9 +29,6 @@ - - NativeMethods.cs - App.config diff --git a/src/Utilities.UnitTests/TrackedDependencies/FileTrackerTests.cs b/src/Utilities.UnitTests/TrackedDependencies/FileTrackerTests.cs index 6eae1c71a1a..24b2e58e113 100644 --- a/src/Utilities.UnitTests/TrackedDependencies/FileTrackerTests.cs +++ b/src/Utilities.UnitTests/TrackedDependencies/FileTrackerTests.cs @@ -20,12 +20,170 @@ #endif using Xunit; -using BackEndNativeMethods = Microsoft.Build.BackEnd.NativeMethods; +using Windows.Win32; +using Windows.Win32.Foundation; +using Windows.Win32.System.Threading; // PLEASE NOTE: This is a UNICODE file as it contains UNICODE characters! #nullable disable +namespace Microsoft.Build.UnitTests.FileTracking +{ + /// + /// Test-only CreateProcess fixture used by the (currently skipped) FileTracker tests. + /// Routes through Windows.Win32.PInvoke.CreateProcess; keeps the original API + /// surface that the tests were written against so the call sites read the same way. + /// + internal static class BackEndNativeMethods + { + public static readonly IntPtr NullPtr = IntPtr.Zero; + public static readonly IntPtr InvalidHandle = new IntPtr(-1); + + public const uint NORMALPRIORITYCLASS = 0x0020; + public const uint CREATENOWINDOW = 0x08000000; + public const int STARTFUSESTDHANDLES = 0x00000100; + + [StructLayout(LayoutKind.Sequential, CharSet = CharSet.Unicode)] + internal struct STARTUP_INFO + { + internal int cb; + internal string lpReserved; + internal string lpDesktop; + internal string lpTitle; + internal int dwX; + internal int dwY; + internal int dwXSize; + internal int dwYSize; + internal int dwXCountChars; + internal int dwYCountChars; + internal int dwFillAttribute; + internal int dwFlags; + internal short wShowWindow; + internal short cbReserved2; + internal IntPtr lpReserved2; + internal IntPtr hStdInput; + internal IntPtr hStdOutput; + internal IntPtr hStdError; + } + + [StructLayout(LayoutKind.Sequential)] + internal struct SECURITY_ATTRIBUTES + { + public int nLength; + public IntPtr lpSecurityDescriptor; + public int bInheritHandle; + } + + [StructLayout(LayoutKind.Sequential)] + internal struct PROCESS_INFORMATION + { + public IntPtr hProcess; + public IntPtr hThread; + public int dwProcessId; + public int dwThreadId; + } + + // Forwards to Windows.Win32.PInvoke.CreateProcess with the same parameter shape + // the existing tests use. Note that this code path is currently exercised only by + // tests gated on FEATURE_FILE_TRACKER, all of which are skipped (see issue #649). + public static unsafe bool CreateProcess( + string lpApplicationName, + string lpCommandLine, + ref SECURITY_ATTRIBUTES lpProcessAttributes, + ref SECURITY_ATTRIBUTES lpThreadAttributes, + bool bInheritHandles, + uint dwCreationFlags, + IntPtr lpEnvironment, + string lpCurrentDirectory, + ref STARTUP_INFO lpStartupInfo, + out PROCESS_INFORMATION lpProcessInformation) + { + // The managed STARTUP_INFO above has `string` fields, so it isn't blittable and + // cannot be reinterpret-cast to STARTUPINFOW*. Marshal into a Win32-shaped struct + // for the duration of the call. CreateProcess only consumes lpStartupInfo, so we + // don't need to copy anything back. + Windows.Win32.System.Threading.STARTUPINFOW si = default; + si.cb = (uint)sizeof(Windows.Win32.System.Threading.STARTUPINFOW); + si.dwX = (uint)lpStartupInfo.dwX; + si.dwY = (uint)lpStartupInfo.dwY; + si.dwXSize = (uint)lpStartupInfo.dwXSize; + si.dwYSize = (uint)lpStartupInfo.dwYSize; + si.dwXCountChars = (uint)lpStartupInfo.dwXCountChars; + si.dwYCountChars = (uint)lpStartupInfo.dwYCountChars; + si.dwFillAttribute = (uint)lpStartupInfo.dwFillAttribute; + si.dwFlags = (STARTUPINFOW_FLAGS)lpStartupInfo.dwFlags; + si.wShowWindow = (ushort)lpStartupInfo.wShowWindow; + si.hStdInput = new HANDLE(lpStartupInfo.hStdInput.ToPointer()); + si.hStdOutput = new HANDLE(lpStartupInfo.hStdOutput.ToPointer()); + si.hStdError = new HANDLE(lpStartupInfo.hStdError.ToPointer()); + + Windows.Win32.Security.SECURITY_ATTRIBUTES pSec = new() + { + nLength = (uint)sizeof(Windows.Win32.Security.SECURITY_ATTRIBUTES), + lpSecurityDescriptor = (void*)lpProcessAttributes.lpSecurityDescriptor, + bInheritHandle = lpProcessAttributes.bInheritHandle != 0, + }; + Windows.Win32.Security.SECURITY_ATTRIBUTES tSec = new() + { + nLength = (uint)sizeof(Windows.Win32.Security.SECURITY_ATTRIBUTES), + lpSecurityDescriptor = (void*)lpThreadAttributes.lpSecurityDescriptor, + bInheritHandle = lpThreadAttributes.bInheritHandle != 0, + }; + + Windows.Win32.System.Threading.PROCESS_INFORMATION pi = default; + + // CreateProcessW is documented to be allowed to modify lpCommandLine in place + // (it parses argv[0] out of it on the way in). Pinning a managed string would let + // native code write to immutable string storage, so copy into a fresh char[] — + // sized for the null terminator — and pin that. + char[] cmdLineBuffer; + if (lpCommandLine is null) + { + cmdLineBuffer = null; + } + else + { + cmdLineBuffer = new char[lpCommandLine.Length + 1]; + lpCommandLine.CopyTo(0, cmdLineBuffer, 0, lpCommandLine.Length); + // CLR initializes the array to 0, so cmdLineBuffer[length] is the terminator. + } + + bool ok; + fixed (char* pApp = lpApplicationName) + { + fixed (char* pCmd = cmdLineBuffer) + { + fixed (char* pCwd = lpCurrentDirectory) + { + ok = PInvoke.CreateProcess( + new PCWSTR(pApp), + new PWSTR(pCmd), + &pSec, + &tSec, + bInheritHandles, + (PROCESS_CREATION_FLAGS)dwCreationFlags, + (void*)lpEnvironment, + new PCWSTR(pCwd), + &si, + &pi); + } + } + } + + lpProcessInformation = new PROCESS_INFORMATION + { + hProcess = (IntPtr)pi.hProcess.Value, + hThread = (IntPtr)pi.hThread.Value, + dwProcessId = (int)pi.dwProcessId, + dwThreadId = (int)pi.dwThreadId, + }; + + return ok; + } + } +} + namespace Microsoft.Build.UnitTests.FileTracking { public sealed class FileTrackerTests : IDisposable diff --git a/src/Utilities/Microsoft.Build.Utilities.csproj b/src/Utilities/Microsoft.Build.Utilities.csproj index ac8644ad6cf..a5d1acb9d61 100644 --- a/src/Utilities/Microsoft.Build.Utilities.csproj +++ b/src/Utilities/Microsoft.Build.Utilities.csproj @@ -24,10 +24,6 @@ - - - -