Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Linker tracks wrong IL offset for return values #2778

Closed
sbomer opened this issue May 2, 2022 · 0 comments · Fixed by dotnet/runtime#105661
Closed

Linker tracks wrong IL offset for return values #2778

sbomer opened this issue May 2, 2022 · 0 comments · Fixed by dotnet/runtime#105661
Milestone

Comments

@sbomer
Copy link
Member

sbomer commented May 2, 2022

ScanAndProcessReturnValue uses the current origin to report warnings for method return values. The origin's IL offset is updated at certain spots in ReflectionMethodBodyScanner, but not for ret opcodes. Instead we just track the possible return values:

case Code.Ret: {
bool hasReturnValue = !methodBody.Method.ReturnsVoid ();
if (currentStack.Count != (hasReturnValue ? 1 : 0)) {
WarnAboutInvalidILInMethod (methodBody, operation.Offset);
}
if (hasReturnValue) {
StackSlot retValue = PopUnknown (currentStack, 1, methodBody, operation.Offset);
ReturnValue = MultiValueLattice.Meet (ReturnValue, retValue.Value);

And later report warnings about the return value using the current origin (which will have whatever IL offset we last updated it to):

public void ScanAndProcessReturnValue (MethodBody methodBody)
{
Scan (methodBody);
if (!methodBody.Method.ReturnsVoid ()) {
var method = methodBody.Method;
var methodReturnValue = _annotations.GetMethodReturnValue (method);
if (methodReturnValue.DynamicallyAccessedMemberTypes != 0) {
var diagnosticContext = new DiagnosticContext (_origin, ShouldEnableReflectionPatternReporting (_origin.Provider), _context);
RequireDynamicallyAccessedMembers (diagnosticContext, ReturnValue, methodReturnValue);
}
}
}

We probably should be tracking the IL offset for each ret instruction instead. I can't think of a case where this produces any different warning behavior, but it requires a workaround in how we handle trim analysis patterns in the linker.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

1 participant