Repository navigation
Fix CordbReJitILCode reading small-format EH sections and tiny headers - #135920
Merged
max-charlamb merged 2 commits intoOct 29, 2026
Merged
Conversation
In the small-format branch of CordbReJitILCode::Init, `ULONG32 m_cClauses` declared a local that shadowed the member. The member stayed 0, so ICorDebugILCode::GetEHClauses reported no clauses for any ReJIT IL body whose EH section uses the small format. The same branch divided DataSize - 4 by the 2-byte section header size, sizeof(IMAGE_COR_ILMETHOD_SECT_SMALL), instead of by the 12-byte clause size. It also started reading clauses 2 bytes into the section instead of 4 bytes in, after the Reserved field. It therefore read about six times too much target memory, from the wrong offset, and failed Init when that memory was not readable. Assign the member, divide by sizeof(IMAGE_COR_ILMETHOD_SECT_EH_CLAUSE_SMALL) and read the clauses from offsetof(IMAGE_COR_ILMETHOD_SECT_EH_SMALL, Clauses). The fat-format branch is unchanged.
A tiny method header has no section flags: its upper six bits are the code size. CordbReJitILCode::Init tested CorILMethod_MoreSects (0x08) against the tiny header byte, so a tiny body whose code size has bit 1 set (2, 3, 6, 7, ...) was treated as having an extra section. The debugger then read the byte at the next 4-byte-aligned address after the code and, if it looked like an EH section kind, parsed whatever followed as EH clauses; if it was not readable, Init failed. Only consult CorILMethod_MoreSects for a fat header, matching COR_ILMETHOD_TINY::GetSect in corhlpr.h, which returns no sections.
|
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. |
Contributor
|
Tagging subscribers to this area: @agocke |
Contributor
|
Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag |
This was referenced Oct 16, 2026
Closed
Member
|
/ba-g build monitor helix failing |
Contributor
Author
|
Thank you for the review and merge! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
CordbReJitILCode::Initmis-read ReJIT IL in two ways.ICorDebugILCode::GetEHClausesreturned no clauses when the EH section used the small format. It could also return clauses for a tiny-header body, which has no EH section. The issue has the details. The change touches onlyCordbReJitILCode::Initand is split into two commits.Fix CordbReJitILCode reading small EH sections. This commit changes only the small-format branch:
m_cClausesmember instead of declaring a local that shadows it;DataSize - 4bysizeof(IMAGE_COR_ILMETHOD_SECT_EH_CLAUSE_SMALL)(12) instead ofsizeof(IMAGE_COR_ILMETHOD_SECT_SMALL)(2);offsetof(IMAGE_COR_ILMETHOD_SECT_EH_SMALL, Clauses)(4), after theReservedfield, instead of from +2.The loop that converts the clauses is unchanged; it already maps each field the same way the fat branch does. A
DataSizebelow 4 is still rejected withCORDBG_E_TARGET_INCONSISTENT, because the unsigned subtraction wraps and the existing> 10000check catches it.Skip EH sections after a tiny ReJIT IL header.
CorILMethod_MoreSectswas tested against the tiny header byte, where that bit belongs to the code size. A tiny header has no sections (COR_ILMETHOD_TINY::GetSect), so the check now applies only to fat headers. This commit has to ship with the first. Before the first commit, the small branch never returned clauses, so a tiny body followed by bytes shaped like a small EH section showed nothing. With the first commit alone, such a body reports a bogus clause; I measured that below.I kept direct struct reads, the same way the fat branch reads. The
COR_ILMETHOD_SECT_EHhelpers incorhlpr.hwork on an in-process pointer, so using them here would mean copying the whole section out of the target first.Testing
I did not find an in-repo test harness for these ICorDebug ReJIT APIs, so this PR does not add a test. If you would like one, I am happy to add it in whatever form you prefer. I validated the change in two ways.
A live repro with a small app, a profiler that ReJITs one method, and an ICorDebug client. The issue has the details. Each
mscordbiwas paired with its own DAC and runtime. The "before" builds are ofmain; the "after" builds are of this branch. All 40 runs, across the shipped andmainruntimes, completed. For the first two rows, the counts were the same throughGetActiveReJitRequestILCodeand throughGetCodeEx(ILCODE_REJIT_IL)on a frame inside the method being ReJITted. The tiny-body rows were measured throughGetActiveReJitRequestILCodeonly, because no frame of the tiny method exists.mainbefore (all three)Whenever a run reports the expected clause, its fields match the IL: try offset 0 and length 54, handler offset 54 and length 5, class token 0x01000013. Correctly handled tiny bodies report no clauses.
A harness that runs the source itself. It compiles
InitandGetEHClausescopied verbatim frommodule.cpp, together with the realcorhdr.h, against a stub process that serves synthetic IL bodies. It has 18 cases, covering:DataSizethat does not hold a whole number of clauses, andDataSize0 and 3;mainI also put each of the four defects (the shadowing local, the wrong divisor, the wrong clause offset and the tiny-header check) back into the fixed code one at a time. The harness catches each one on its own.
The fixed
mscordbibuilds on linux-x64, osx-arm64 and windows-x64; those builds are the ones used in the live runs above.Resolves #135919
Note
This PR was prepared with AI assistance (Anthropic Claude and OpenAI Codex), under my direction. I reviewed it before posting.