Remove legacy FAULT contract logic and annotations - #132535
Conversation
Removes the fault contract machinery and all of its usages across src/coreclr: the INJECT_FAULT, FORBID_FAULT, STATIC_CONTRACT_FAULT, STATIC_CONTRACT_FORBID_FAULT, FAULT_FORBID and FAULT_NOT_FATAL annotations; the FaultViolation and FaultNotFatal contract-violation bits; the FaultForbidHolder helper; CheckFaultInjection; the ARE_FAULTS_FORBIDDEN assertions; and the FAULT_* mask/state constants. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 15f13a4f-dffc-47e3-b836-05a24f2c7149
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
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. |
|
Tagging subscribers to this area: @agocke |
There was a problem hiding this comment.
Pull request overview
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
This PR removes the legacy “FAULT” contract infrastructure across CoreCLR and updates related comments/macros accordingly.
Changes:
- Removed
FORBID_FAULT,STATIC_CONTRACT_*FAULT*,INJECT_FAULT(...), andFAULT_NOT_FATAL()usages from contracts throughout VM/utilcode/GC/debug code. - Simplified several
CONTRACT_VIOLATION(...)masks by droppingFaultViolation/FaultNotFatalflags. - Updated helper macros and comments (e.g., loader-use forbid scopes) to no longer reference fault-contract behavior.
Show a summary per file
| File | Description |
|---|---|
| src/coreclr/vm/zapsig.cpp | Removes FORBID_FAULT contract annotations. |
| src/coreclr/vm/virtualcallstub.h | Removes fault-related contract annotations and inject-fault hooks. |
| src/coreclr/vm/util.cpp | Removes fault-related static/dynamic contracts and inject-fault hooks. |
| src/coreclr/vm/typehash.cpp | Removes INJECT_FAULT contract hooks. |
| src/coreclr/vm/typehandle.inl | Removes FORBID_FAULT contract annotations. |
| src/coreclr/vm/typehandle.h | Removes STATIC_CONTRACT_FORBID_FAULT usage. |
| src/coreclr/vm/typehandle.cpp | Removes fault-related contracts and inject-fault hooks. |
| src/coreclr/vm/typeequivalencehash.cpp | Removes INJECT_FAULT contract hooks. |
| src/coreclr/vm/typedesc.cpp | Removes fault-related contracts and inject-fault hooks. |
| src/coreclr/vm/typectxt.cpp | Removes FORBID_FAULT contract annotations. |
| src/coreclr/vm/threadsuspend.cpp | Updates comment to remove INJECT_FAULT mention. |
| src/coreclr/vm/threadstatics.cpp | Removes INJECT_FAULT contract hook. |
| src/coreclr/vm/threads.h | Updates loader-forbid macro/comment to drop fault forbiddance. |
| src/coreclr/vm/threads.cpp | Removes INJECT_FAULT and FAULT_NOT_FATAL usage in touched hunks. |
| src/coreclr/vm/tailcallhelp.cpp | Removes INJECT_FAULT contract hook. |
| src/coreclr/vm/syncblk.h | Removes FORBID_FAULT contract annotation. |
| src/coreclr/vm/syncblk.cpp | Removes inject-fault hooks and fault-forbid scopes. |
| src/coreclr/vm/stubmgr.cpp | Removes fault-not-fatal usage and fault-related contracts. |
| src/coreclr/vm/stubgen.cpp | Removes INJECT_FAULT contract hooks. |
| src/coreclr/vm/stringliteralmap.cpp | Removes fault-related contracts and inject-fault hooks. |
| src/coreclr/vm/stdinterfaces.cpp | Removes INJECT_FAULT contract hooks. |
| src/coreclr/vm/stackingallocator.h | Removes INJECT_FAULT from contract block. |
| src/coreclr/vm/stackingallocator.cpp | Removes STATIC_CONTRACT_FAULT annotations. |
| src/coreclr/vm/siginfo.hpp | Removes conditional fault-contract clause. |
| src/coreclr/vm/siginfo.cpp | Removes fault-related contracts and inject-fault hooks. |
| src/coreclr/vm/sigformat.cpp | Updates comments and removes inject-fault hooks. |
| src/coreclr/vm/runtimecallablewrapper.cpp | Removes INJECT_FAULT contract hooks. |
| src/coreclr/vm/reflectclasswriter.cpp | Removes inject-fault and forbid-fault contracts. |
| src/coreclr/vm/readytoruninfo.cpp | Removes FORBID_FAULT contract annotation. |
| src/coreclr/vm/proftoeeinterfaceimpl.cpp | Removes FAULT_NOT_FATAL usage and related explanatory comment. |
| src/coreclr/vm/pendingload.cpp | Removes inject-fault and fault-not-fatal usage. |
| src/coreclr/vm/peimagelayout.inl | Removes FORBID_FAULT contract annotation. |
| src/coreclr/vm/peimagelayout.cpp | Removes INJECT_FAULT contract hook. |
| src/coreclr/vm/peimage.cpp | Removes fault-related contract/violation flags and inject-fault hooks. |
| src/coreclr/vm/peassembly.inl | Removes fault-related violation flags and fault contract clauses. |
| src/coreclr/vm/peassembly.cpp | Removes INJECT_FAULT contract hooks. |
| src/coreclr/vm/olevariant.cpp | Removes INJECT_FAULT contract hooks. |
| src/coreclr/vm/object.inl | Removes fault-related contracts. |
| src/coreclr/vm/object.cpp | Removes fault-related contracts and simplifies contract-violation masks. |
| src/coreclr/vm/nativeimage.cpp | Removes INJECT_FAULT contract hook. |
| src/coreclr/vm/multicorejitplayer.cpp | Removes INJECT_FAULT contract hooks. |
| src/coreclr/vm/multicorejit.cpp | Removes INJECT_FAULT contract hooks. |
| src/coreclr/vm/mlinfo.cpp | Removes INJECT_FAULT contract hooks. |
| src/coreclr/vm/methodtablebuilder.inl | Removes fault-related static contracts. |
| src/coreclr/vm/methodtablebuilder.cpp | Removes fault-related contracts and inject-fault hook. |
| src/coreclr/vm/methodtable.cpp | Removes fault-related contracts and inject-fault hooks. |
| src/coreclr/vm/methodimpl.cpp | Removes fault-related conditional contract clauses and inject-fault hook. |
| src/coreclr/vm/method.cpp | Removes fault-related contracts and inject-fault hooks. |
| src/coreclr/vm/memberload.cpp | Removes INJECT_FAULT contract hooks. |
| src/coreclr/vm/loaderallocator.cpp | Removes FORBID_FAULT and INJECT_FAULT contract hooks. |
| src/coreclr/vm/jitinterface.cpp | Removes FORBID_FAULT contract annotation. |
| src/coreclr/vm/invokeutil.cpp | Removes INJECT_FAULT contract hooks. |
| src/coreclr/vm/interoputil.inl | Removes FORBID_FAULT contract annotation. |
| src/coreclr/vm/interoputil.cpp | Simplifies CONTRACT_VIOLATION flags and removes INJECT_FAULT hooks. |
| src/coreclr/vm/interopconverter.cpp | Removes FAULT_NOT_FATAL usage. |
| src/coreclr/vm/instmethhash.cpp | Removes fault-related contracts and inject-fault hooks. |
| src/coreclr/vm/ilstubcache.cpp | Removes INJECT_FAULT contract hook. |
| src/coreclr/vm/i386/stublinkerx86.cpp | Removes fault-related contracts and inject-fault hook. |
| src/coreclr/vm/hash.cpp | Removes fault-related contracts and fault-not-fatal usage. |
| src/coreclr/vm/genmeth.cpp | Removes INJECT_FAULT contract hooks. |
| src/coreclr/vm/generics.cpp | Removes FORBID_FAULT contract annotations. |
| src/coreclr/vm/genericdict.cpp | Removes INJECT_FAULT contract hooks. |
| src/coreclr/vm/gctoclreventsink.cpp | Removes FAULT_NOT_FATAL usage. |
| src/coreclr/vm/gchelpers.cpp | Removes INJECT_FAULT contract hook. |
| src/coreclr/vm/frames.cpp | Removes commented fault-contract annotations. |
| src/coreclr/vm/fptrstubs.cpp | Removes INJECT_FAULT contract hook. |
| src/coreclr/vm/fieldmarshaler.cpp | Removes INJECT_FAULT contract hook. |
| src/coreclr/vm/field.h | Removes INJECT_FAULT contract hooks. |
| src/coreclr/vm/field.cpp | Removes fault-related contracts and simplifies violation flags. |
| src/coreclr/vm/exstate.cpp | Removes STATIC_CONTRACT_FORBID_FAULT. |
| src/coreclr/vm/excep.cpp | Removes fault-related contracts, fault-not-fatal usage, and simplifies violation flags. |
| src/coreclr/vm/eventtrace.cpp | Removes STATIC_CONTRACT_FAULT annotation. |
| src/coreclr/vm/encee.cpp | Removes fault-related contracts and fault-not-fatal usage. |
| src/coreclr/vm/eepolicy.cpp | Removes FAULT_NOT_FATAL usage and simplifies violation flags. |
| src/coreclr/vm/eehash.inl | Removes fault-related contracts and inject-fault hooks. |
| src/coreclr/vm/eehash.h | Removes INJECT_FAULT contract hooks. |
| src/coreclr/vm/eehash.cpp | Removes fault-related contracts and inject-fault hooks. |
| src/coreclr/vm/eedbginterfaceimpl.cpp | Removes fault-related contract-violation suppression and FAULT_NOT_FATAL usage. |
| src/coreclr/vm/eeconfig.cpp | Removes fault-related contracts and inject-fault hooks. |
| src/coreclr/vm/dynamicmethod.cpp | Removes INJECT_FAULT contract hooks. |
| src/coreclr/vm/dwreport.cpp | Removes fault-related contracts and FAULT_NOT_FATAL usage. |
| src/coreclr/vm/dllimportcallback.cpp | Removes INJECT_FAULT contract hooks. |
| src/coreclr/vm/dispparammarshaler.cpp | Removes INJECT_FAULT contract hooks. |
| src/coreclr/vm/dispatchinfo.cpp | Removes INJECT_FAULT contract hooks. |
| src/coreclr/vm/decodemd.cpp | Removes fault-related static contracts. |
| src/coreclr/vm/dacenumerablehash.inl | Removes FAULT_NOT_FATAL usage. |
| src/coreclr/vm/custommarshalerinfo.cpp | Removes INJECT_FAULT contract hooks. |
| src/coreclr/vm/contractimpl.h | Removes INJECT_FAULT contract hooks. |
| src/coreclr/vm/contractimpl.cpp | Removes INJECT_FAULT contract hooks. |
| src/coreclr/vm/comutilnative.cpp | Removes INJECT_FAULT contract hook. |
| src/coreclr/vm/commtmemberinfomap.cpp | Removes INJECT_FAULT contract hooks. |
| src/coreclr/vm/comdelegate.cpp | Removes INJECT_FAULT contract hooks. |
| src/coreclr/vm/comconnectionpoints.h | Removes INJECT_FAULT contract hook. |
| src/coreclr/vm/comconnectionpoints.cpp | Removes INJECT_FAULT contract hooks. |
| src/coreclr/vm/comcallablewrapper.cpp | Removes INJECT_FAULT contract hooks. |
| src/coreclr/vm/comcache.cpp | Removes INJECT_FAULT contract hooks. |
| src/coreclr/vm/codeversion.cpp | Removes FORBID_FAULT contract annotation. |
| src/coreclr/vm/codeman.cpp | Removes fault-related contract infra comment/violation and FORBID_FAULT use. |
| src/coreclr/vm/clrex.h | Removes fault-not-fatal comment block and usage from macro. |
| src/coreclr/vm/clrex.cpp | Removes fault-related contracts and FAULT_NOT_FATAL usage. |
| src/coreclr/vm/classhash.inl | Removes FORBID_FAULT. |
| src/coreclr/vm/classhash.cpp | Removes fault-related contracts and simplifies violation flags. |
| src/coreclr/vm/classcompat.cpp | Removes STATIC_CONTRACT_FORBID_FAULT. |
| src/coreclr/vm/class.cpp | Removes fault-related contracts and inject-fault hooks. |
| src/coreclr/vm/ceemain.cpp | Removes fault-related contract flags and FAULT_NOT_FATAL usage. |
| src/coreclr/vm/ceeload.inl | Removes INJECT_FAULT and updates wrapper comment. |
| src/coreclr/vm/ceeload.h | Updates wrapper comments to remove fault references. |
| src/coreclr/vm/ceeload.cpp | Removes fault-related contracts and inject-fault hooks. |
| src/coreclr/vm/castcache.cpp | Removes FAULT_NOT_FATAL usage. |
| src/coreclr/vm/callingconvention.h | Removes conditional fault-contract clauses. |
| src/coreclr/vm/callhelpers.cpp | Updates comment and removes inject-fault mention. |
| src/coreclr/vm/cachelinealloc.cpp | Removes INJECT_FAULT contract hooks. |
| src/coreclr/vm/binder.h | Removes fault-related contracts and inject-fault hooks. |
| src/coreclr/vm/binder.cpp | Removes fault-related contracts and inject-fault hooks. |
| src/coreclr/vm/baseassemblyspec.inl | Removes INJECT_FAULT contract hooks. |
| src/coreclr/vm/baseassemblyspec.cpp | Removes INJECT_FAULT contract hooks. |
| src/coreclr/vm/asynccontinuations.cpp | Removes INJECT_FAULT contract hook. |
| src/coreclr/vm/assemblyspec.cpp | Removes fault-related contracts and inject-fault hooks. |
| src/coreclr/vm/assembly.cpp | Removes fault-related contracts and inject-fault hooks; updates comments. |
| src/coreclr/vm/array.cpp | Removes INJECT_FAULT contract hook. |
| src/coreclr/vm/argdestination.h | Removes STATIC_CONTRACT_FORBID_FAULT. |
| src/coreclr/vm/appdomain.hpp | Removes STATIC_CONTRACT_FORBID_FAULT and STATIC_CONTRACT_FAULT. |
| src/coreclr/vm/amd64/profiler.cpp | Removes STATIC_CONTRACT_FORBID_FAULT. |
| src/coreclr/vm/amd64/excepamd64.cpp | Removes FAULT_NOT_FATAL usage and related comment. |
| src/coreclr/utilcode/winfix.cpp | Removes STATIC_CONTRACT_FORBID_FAULT. |
| src/coreclr/utilcode/stresslog.cpp | Removes fault-related contracts and FAULT_NOT_FATAL usage. |
| src/coreclr/utilcode/sstring.cpp | Removes FAULTS_UNLESS_BOTH_NORMALIZED macro usage. |
| src/coreclr/utilcode/sigparser.cpp | Removes FORBID_FAULT annotations. |
| src/coreclr/utilcode/rangelist.cpp | Removes inject-fault and forbid-fault contracts. |
| src/coreclr/utilcode/prettyprintsig.cpp | Removes INJECT_FAULT contract hooks. |
| src/coreclr/utilcode/pedecoder.cpp | Removes FORBID_FAULT. |
| src/coreclr/utilcode/namespaceutil.cpp | Removes fault-related static contracts. |
| src/coreclr/utilcode/memorypool.cpp | Removes INJECT_FAULT contract hooks. |
| src/coreclr/utilcode/loaderheap_shared.cpp | Removes fault-related contracts and FAULT_NOT_FATAL usage. |
| src/coreclr/utilcode/loaderheap.cpp | Removes fault injection infrastructure and fault-related assertions/violations. |
| src/coreclr/utilcode/interleavedloaderheap.cpp | Removes fault injection infrastructure and fault-related assertions. |
| src/coreclr/utilcode/format1.cpp | Removes STATIC_CONTRACT_FORBID_FAULT and related commented contract. |
| src/coreclr/utilcode/explicitcontrolloaderheap.cpp | Removes inject-fault and fault-related assertions. |
| src/coreclr/utilcode/executableallocator.cpp | Removes permanent fault-violation comment. |
| src/coreclr/utilcode/ex.cpp | Removes STATIC_CONTRACT_FORBID_FAULT. |
| src/coreclr/utilcode/debug.cpp | Removes fault-related contracts, fault-not-fatal usage, and simplifies violation flags. |
| src/coreclr/utilcode/clrhost.cpp | Removes STATIC_CONTRACT_FORBID_FAULT. |
| src/coreclr/utilcode/clrconfig.cpp | Removes fault-related contracts, inject-fault hooks, and fault-not-fatal usage. |
| src/coreclr/utilcode/check.cpp | Removes fault-violation specializations and fault-not-fatal allocations. |
| src/coreclr/utilcode/arraylist.cpp | Removes fault-related contracts and inject-fault hook. |
| src/coreclr/utilcode/allocmemtracker.cpp | Removes fault-related contracts and inject-fault hook. |
| src/coreclr/md/runtime/stgpoolreadonly.cpp | Removes inject-fault and fault-related static contracts. |
| src/coreclr/md/runtime/stgpooli.cpp | Removes STATIC_CONTRACT_FORBID_FAULT. |
| src/coreclr/md/runtime/stgpool.cpp | Removes inject-fault hooks and forbid-fault contracts. |
| src/coreclr/md/inc/stgpooli.h | Removes STATIC_CONTRACT_FORBID_FAULT. |
| src/coreclr/md/inc/stgpool.h | Removes STATIC_CONTRACT_FORBID_FAULT. |
| src/coreclr/inc/utilcode.h | Removes FORBID_FAULT contract annotation. |
| src/coreclr/inc/staticcontract.h | Removes STATIC_CONTRACT_FAULT / STATIC_CONTRACT_FORBID_FAULT macro definitions. |
| src/coreclr/inc/sstring.h | Removes fault-related helper macros. |
| src/coreclr/inc/ex.h | Removes FAULT_NOT_FATAL() from throw macros. |
| src/coreclr/inc/contract.inl | Removes fault-contract checking and fault injection stub. |
| src/coreclr/inc/ceegentokenmapper.h | Removes STATIC_CONTRACT_FORBID_FAULT. |
| src/coreclr/inc/caparser.h | Removes STATIC_CONTRACT_FORBID_FAULT. |
| src/coreclr/gc/objecthandle.cpp | Removes INJECT_FAULT contract hook. |
| src/coreclr/gc/handletablescan.cpp | Removes FORBID_FAULT. |
| src/coreclr/gc/handletablecache.cpp | Removes FAULT_NOT_FATAL usage. |
| src/coreclr/gc/handletable.cpp | Removes INJECT_FAULT contract hook. |
| src/coreclr/gc/env/gcenv.base.h | Removes GC-side FORBID_FAULT/INJECT_FAULT/FAULT_NOT_FATAL macro stubs. |
| src/coreclr/gc/background.cpp | Removes FAULT_NOT_FATAL usage. |
| src/coreclr/debug/ee/functioninfo.cpp | Removes FORBID_FAULT. |
| src/coreclr/debug/ee/debuggermessagebox.cpp | Removes INJECT_FAULT contract hooks. |
| src/coreclr/debug/ee/debugger.cpp | Removes FAULT_NOT_FATAL usage and comment. |
Review details
Suppressed comments (1)
src/coreclr/vm/ceeload.inl:1
- The updated comment has a formatting typo (
GC_NOTRIGGER//MODE_ANY). Please fix the separator to keep these wrapper-contract comments consistent and readable (e.g.,THROWS/GC_NOTRIGGER/MODE_ANY).
- Files reviewed: 171/171 changed files
- Comments generated: 1
- Review effort level: Lite
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 171 out of 171 changed files in this pull request and generated no new comments.
Suppressed comments (3)
src/coreclr/vm/ceeload.inl:1
- The contract comment has a typo (
GC_NOTRIGGER//MODE_ANY). Update it to a single separator to keep comments readable and consistent.
src/coreclr/utilcode/loaderheap.cpp:1 - This change removes the debug-only fault-injection hook (previously driven by
CLRConfig::INTERNAL_InjectFault) from the no-throw loader heap allocation paths. If any internal stress runs / reliability tests still depend on that knob to simulate OOM, this will reduce coverage and can mask regressions; consider either removing/retiring the config flag everywhere, or replacing it with an explicit, non-contract-based OOM injection mechanism local to the allocator.
src/coreclr/utilcode/check.cpp:59 - The deletion of the
SPECIALIZED_VIOLATION(0);specialization can cause compile/link failures if any code still instantiatesAutoCleanupContractViolationHolder<0>(or equivalent “no violations” holders). Please either (a) restore theSPECIALIZED_VIOLATION(0)specialization, or (b) verify and document (with a repo-wide search) that no remaining call sites instantiate a zero-mask holder.
SPECIALIZE_CONTRACT_VIOLATION_HOLDER(mask); \
SPECIALIZE_AUTO_CLEANUP_CONTRACT_VIOLATION_HOLDER(mask)
// Basic Specializations
SPECIALIZED_VIOLATION(AllViolation);
SPECIALIZED_VIOLATION(ThrowsViolation);
SPECIALIZED_VIOLATION(GCViolation);
SPECIALIZED_VIOLATION(ModeViolation);
SPECIALIZED_VIOLATION(LoadsTypeViolation);
SPECIALIZED_VIOLATION(TakesLockViolation);
|
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. |
There was a problem hiding this comment.
Review details
Suppressed comments (2)
src/coreclr/inc/contract.inl:48
- The runtime contract removal leaves the fault-state implementation in
src/coreclr/inc/contract.h:CONTRACT_BITMASK_FAULT_FORBIDandClrDebugState::{Is,Set,Reset}FaultForbidnow have no callers. Please remove that dead state and its initialization comment as part of this cleanup so the obsolete implementation is actually eliminated.
switch (testmask & THROWS_Mask)
src/coreclr/vm/ceeload.inl:327
- Remove the duplicate
/left by deletingINJECT_FAULT()from this contract summary.
- Files reviewed: 171/171 changed files
- Comments generated: 1
- Review effort level: Balanced
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
/ba-g Known failure. Fixed in #132549 |
Removes the obsolete FAULT contract implementation and remaining annotations.
Note
This PR description was generated by GitHub Copilot.