Enable contracts for new holders and refactor Crst initialization

[AaronRobinsonMSFT]

Summary

Enables EE contracts for the holder infrastructure and refactors related lock initialization so that a single, consistent contract vocabulary is used across the VM, GC, and NativeAOT runtimes. Contracts are additionally hardened so the hosted contract machinery is only enabled when code is compiled into or linked against coreclr or the static host, and the legacy fault-contract annotations are removed.

Details

Contract enablement and host gating (inc/contract.h)

• Simplify the macros that gate ENABLE_CONTRACTS_DATA / ENABLE_CONTRACTS / ENABLE_CONTRACTS_IMPL. Contract data no longer depends on JIT_BUILD / CROSS_COMPILE.
• ENABLE_CONTRACTS is now additionally gated on !DBI_COMPILE && !SELF_NO_HOST (alongside the existing !DACCESS_COMPILE), so the hosted contract implementation is only enabled when code is compiled into or linked against coreclr or the static host. DAC/DBI builds continue to reference contract data but not the implementation.
• Move the EEContract vocabulary (GC_TRIGGERS, MODE_COOPERATIVE / MODE_PREEMPTIVE / MODE_ANY, etc.) directly into contract.h and delete vm/eecontract.h; the separate ENABLE_EE_CONTRACTS define is removed.

Holders (inc/holder.h)

• ReleaseHolderTraits::Free now uses a full CONTRACTL block (NOTHROW / GC_TRIGGERS / MODE_PREEMPTIVE) when the EE contract vocabulary is available, falling back to the static contracts otherwise.
• The release-path contract validation is factored out of line into ContractReleaseValidate() (declared in holder.h), with a hosted implementation in vm/util.cpp and a hostless no-op stub in utilcode/hostimpl.cpp.

Remove legacy fault-contract annotations

• Remove all INJECT_FAULT(...), FORBID_FAULT / STATIC_CONTRACT_FORBID_FAULT, and FAULT_NOT_FATAL() annotations throughout src/coreclr.

Standalone IL tools (ilasm / ildasm)

Make the hostless ilasm / ildasm tool chain consistently SELF_NO_HOST so it no longer requires the hosted contract implementation at link time:

• Add SELF_NO_HOST to the tool-only _ppdb metadata libraries (mdcompiler_ppdb, mdruntime_ppdb, mdruntimerw_ppdb) and to mscorpe.
• Add a ceefgen_nohost flavor for ilasm; coreclr keeps the contract-enabled ceefgen.
• Switch ildasm from the contract-enabled _wks metadata flavor to the hostless _ppdb flavor.

Crst initialization

• Remove CrstStatic::InitNoThrow and make Init (and CrstBase::InitWorker) NOTHROW / GC_NOTRIGGER. All callers in the VM, GC, and NativeAOT are updated to call Init. gcenv.sync.h and NativeAOT Crst.h drop their InitNoThrow shims accordingly.

IsIPInModule

• Move the implementation out of the generic utilcode/util.cpp (which cannot depend on EE contracts) into vm/util.cpp, declare it in vm/util.hpp, and expose it to the debugger through the EE debugger interface (IEEDbgInterface::IsIPInModule) instead of the free function.

Metadata (inc/metadata.h)

• Add GC_NOTRIGGER to several lock-acquire contracts.

Include ordering (vm/common.h)

• Pull in contract.h before holder.h so the EE contract vocabulary is visible to the holder definitions.

Note

This PR description was generated with the assistance of GitHub Copilot.

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[dotnet-policy-service]

Tagging subscribers to this area: @agocke
See info in area-owners.md if you want to be subscribed.

[Copilot]

Pull request overview

This PR updates CoreCLR’s contracts and lock initialization patterns to use a more consistent EE contract vocabulary across VM/GC/NativeAOT, and relocates IsIPInModule into the VM while routing debugger usage through EEDebugInterface.

Changes:

• Refactors CrstStatic initialization to remove InitNoThrow and update call sites across VM/GC/NativeAOT to call Init.
• Introduces ENABLE_EE_CONTRACTS (when contracts implementation is enabled) and uses it to upgrade holder contracts in holder.h.
• Moves IsIPInModule from utilcode/util.cpp into vm/util.cpp, adds a VM declaration, and exposes it via EEDebugInterface.

Show a summary per file

File	Description
src/coreclr/vm/util.hpp	Declares VM-level IsIPInModule.
src/coreclr/vm/util.cpp	Adds VM implementation of IsIPInModule.
src/coreclr/vm/eventtrace.cpp	Switches CrstStatic init from InitNoThrow to Init.
src/coreclr/vm/eventing/eventpipe/ep-rt-coreclr.h	Switches EventPipe lock init to Init.
src/coreclr/vm/eedbginterfaceimpl.h	Adds IsIPInModule to the EE debugger interface implementation.
src/coreclr/vm/eedbginterfaceimpl.cpp	Implements EEDbgInterfaceImpl::IsIPInModule forwarding to the helper.
src/coreclr/vm/eedbginterface.h	Adds IsIPInModule to EEDebugInterface.
src/coreclr/vm/eecontract.h	Introduces ENABLE_EE_CONTRACTS under ENABLE_CONTRACTS_IMPL.
src/coreclr/vm/crst.h	Removes CrstStatic::InitNoThrow and standardizes on Init.
src/coreclr/vm/crst.cpp	Changes CrstBase::InitWorker contract and initialization behavior.
src/coreclr/vm/common.h	Adjusts include ordering so EE contract vocabulary is visible to holders.
src/coreclr/utilcode/util.cpp	Removes the old IsIPInModule implementation from utilcode.
src/coreclr/nativeaot/Runtime/GcStressControl.cpp	Updates NativeAOT lock init to Init.
src/coreclr/nativeaot/Runtime/eventpipe/ep-rt-aot.cpp	Updates NativeAOT EventPipe locks to Init.
src/coreclr/nativeaot/Runtime/Crst.h	Removes NativeAOT InitNoThrow shim.
src/coreclr/nativeaot/Runtime/clrgc.enabled.cpp	Updates GC event lock init to Init.
src/coreclr/inc/utilcode.h	Removes utilcode declaration of IsIPInModule.
src/coreclr/inc/metadata.h	Adds GC_NOTRIGGER to several lock-acquire contract blocks.
src/coreclr/inc/holder.h	Uses full CONTRACTL under ENABLE_EE_CONTRACTS for ReleaseHolderTraits::Free.
src/coreclr/inc/contract.h	Simplifies gating for ENABLE_CONTRACTS_DATA / ENABLE_CONTRACTS (DAC excluded from impl).
src/coreclr/gc/handletable.cpp	Switches handle table lock init to Init and removes failure cleanup path.
src/coreclr/gc/env/gcenv.sync.h	Changes GC env shim CrstStatic init API to Init and drops status return.
src/coreclr/debug/ee/debugger.cpp	Routes debugger’s module-IP check through g_pEEInterface->IsIPInModule.

Copilot's findings

Comments suppressed due to low confidence (1)

src/coreclr/vm/crst.cpp:42

• CrstBase::InitWorker relies on _ASSERTE(suc) after minipal_mutex_init. In retail builds _ASSERTE compiles away, so a failed mutex init would still mark the Crst initialized and proceed with an uninitialized m_lock, leading to undefined behavior later. This needs an all-build failure path (fail-fast) rather than a debug-only assert.

    _ASSERTE((flags & CRST_INITIALIZED) == 0);

    bool suc = minipal_mutex_init(&m_lock._mtx);
    _ASSERTE(suc);

• Files reviewed: 23/23 changed files
• Comments generated: 1

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[Copilot]

Copilot's findings

Suppressed comments (4)

src/coreclr/vm/crst.cpp:42

• CrstBase::InitWorker ignores minipal_mutex_init failure in retail builds (only _ASSERTE checks it). If initialization fails (e.g., OOM), we proceed with an uninitialized mutex, which can lead to undefined behavior later. This is especially problematic now that callers no longer have an InitNoThrow fallback path.

    _ASSERTE((flags & CRST_INITIALIZED) == 0);

    bool suc = minipal_mutex_init(&m_lock._mtx);
    _ASSERTE(suc);

src/coreclr/gc/env/gcenv.sync.h:32

• CrstStatic::Init now discards the CLRCriticalSection::Initialize() return value, removing even the prior ability to detect initialization failure in debug builds. If Initialize() can fail, this can leave the GC lock in an unusable state with no signal to callers.

    void Init(CrstType eType, CrstFlags eFlags = CRST_DEFAULT)
    {
        (void)m_cs.Initialize();
    }

src/coreclr/inc/holder.h:1019

• The body of ReleaseHolderTraits::Free has inconsistent indentation (contract lines and ContractReleaseValidate() are not indented to match the rest of the function body). This makes the block harder to read and stands out from the surrounding style in holder.h.

    {
    STATIC_CONTRACT_NOTHROW;
    STATIC_CONTRACT_GC_TRIGGERS;
    STATIC_CONTRACT_MODE_PREEMPTIVE;

#ifdef ENABLE_CONTRACTS_IMPL
    ContractReleaseValidate();
#endif // ENABLE_CONTRACTS_IMPL

src/coreclr/vm/eedbginterface.h:136

• New virtual methods on these VM<->debugger interfaces should be appended at the end to preserve vtable slot ordering (per established CoreCLR convention). IsIPInModule is currently inserted in the middle of the interface, which increases ABI risk for components that may be built against a different header version.

    virtual BOOL IsManagedNativeCode(const BYTE *address) = 0;

#endif // #ifndef DACCESS_COMPILE

    virtual BOOL IsIPInModule(PTR_VOID pModuleBaseAddress, PCODE ip) = 0;

    virtual PCODE GetNativeCodeStartAddress(PCODE address) = 0;

    virtual MethodDesc *GetNativeCodeMethodDesc(const PCODE address) = 0;

• Files reviewed: 24/24 changed files
• Comments generated: 0 new

[jkotas]

Remove legacy fault contract annotations

Can this be in a separate PR? It makes it impossible to review the actual changes

[Copilot]

Copilot's findings

Suppressed comments (4)

src/coreclr/inc/holder.h:1018

• The ContractReleaseValidate(); call inside ReleaseHolderTraits::Free is indented less than the rest of the function body, which looks accidental and makes the new contract hook easy to miss.

    ContractReleaseValidate();

src/coreclr/gc/env/gcenv.sync.h:32

• CrstStatic::Init ignores the bool return from CLRCriticalSection::Initialize() (which wraps minipal_mutex_init). If initialization can fail (e.g., pthread attr/mutex init failures), this will silently leave an uninitialized lock, leading to undefined behavior on first use. Consider either fail-fast here or keep an init API that propagates failure to callers.

    void Init(CrstType eType, CrstFlags eFlags = CRST_DEFAULT)
    {
        (void)m_cs.Initialize();
    }

src/coreclr/inc/holder.h:1015

• ReleaseHolderTraits::Free's contract lines are no longer indented to match the surrounding code block, making the new hook harder to read and inconsistent with the rest of holder.h.

This issue also appears on line 1018 of the same file.

    STATIC_CONTRACT_NOTHROW;
    STATIC_CONTRACT_GC_TRIGGERS;
    STATIC_CONTRACT_MODE_PREEMPTIVE;

src/coreclr/vm/crst.cpp:36

• CrstBase::InitWorker is now annotated as NOTHROW / GC_NOTRIGGER, but the implementation calls minipal_mutex_init (which can fail on Unix and can raise on Windows via InitializeCriticalSection). Unless the body is hardened to never throw/fail, these contracts are likely inaccurate and can cause contract asserts or hide real failure behavior.

• Files reviewed: 201/202 changed files
• Comments generated: 0 new

[AaronRobinsonMSFT]

Remove legacy fault contract annotations

Can this be in a separate PR? It makes it impossible to review the actual changes

I'll give it a try. There is so much entanglement in these systems that it was/is difficult to make parts work without the others. Let me try to create at least a distinct commit and see how it goes.