Resolve field offsets through byref bases in ResolveFieldOffsets - #613
Open
shadesbelow wants to merge 1 commit into
Open
Resolve field offsets through byref bases in ResolveFieldOffsets#613shadesbelow wants to merge 1 commit into
shadesbelow wants to merge 1 commit into
Conversation
ResolveFieldOffsets only succeeds when the memory operand's base local has a normal instance type with a populated .Fields list. When the base is a ByRefTypeAnalysisContext (a T& local - e.g. a `ref MyStruct data` parameter), .Fields is always empty, because the byref wrapper is a ReferencedTypeAnalysisContext with no Definition. The offset therefore never resolves, and every field read through that ref parameter falls through to the "Unmanaged memory load" diagnostic - even though Ldfld accepts a managed pointer operand directly, which is the same precedent IlGenerator's addend==0 byref dereference case already relies on. Resolve field offsets against the referent (.ElementType) when the base is a byref, rather than against the byref wrapper itself. Measured on a shipped Unity 6 IL2CPP title (x86-64, metadata v31): 20,807 -> 20,336 "Unmanaged memory load" sites, i.e. 471 eliminated. Spot-checked one newly-resolved site against Il2CppInspector's dumped field layout for the same struct: byref base + 0x1C resolved to the field the dump lists at 0x1C.
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.
ResolveFieldOffsetsonly succeeds when the memory operand's base local has a normal instance type with a populated.Fieldslist. When the base is aByRefTypeAnalysisContext(aT&local — e.g. aref MyStruct dataparameter),.Fieldsis always empty, because the byref wrapper is aReferencedTypeAnalysisContextwith noDefinition.The offset therefore never resolves, and every field read through that
refparameter falls through to theUnmanaged memory loaddiagnostic — even thoughLdfldaccepts a managed pointer operand directly, which is the same precedentIlGenerator'saddend == 0byref dereference case already relies on.This resolves field offsets against the referent (
.ElementType) when the base is a byref, rather than against the byref wrapper itself.Measured
On a shipped Unity 6 IL2CPP title (x86-64, metadata v31), with only this diff toggled:
Unmanaged memory loadbeforeVerified rather than just gate-passed: one newly-resolved site (a byref base +
0x1C) was checked against Il2CppInspector's dumped field layout for the same struct, which lists a field at0x1C. Exact match. A single heavyrefconsumer in that title drops from 394 to 200 diagnostic sites.The remainder of the 20,336 is a different problem (generic value-type layout and
Il2CppClassruntime-metadata reads) and is deliberately left as diagnostics rather than guessed.