Skip to content

Commit

Permalink
[Mono.Cecil] Enforce empty body when a MethodDefinition's rva value is 0
Browse files Browse the repository at this point in the history
The prior behavior assumed a MethodDefinition's RVA value of 0 meant an effectively empty body with a single RET instruction. This change removes the RET insertion and leaves the method completely empty. This is important for mono's UnsafeAccessor detection around methods specified as extern as it uses a quicker method of an empty vs non-empty body to bypass loading / checking custom attributes.

Fixes dotnet/runtime#102045
  • Loading branch information
steveisok committed Aug 7, 2024
2 parents 994a0a5 + aa357d9 commit 3ed40d4
Showing 1 changed file with 17 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ internal sealed class MethodDefinition : MethodReference, IMemberDefinition,
PInvokeInfo m_pinvoke;
readonly ParameterDefinition m_this;

bool m_enforceNoBody;

public MethodAttributes Attributes {
get { return m_attributes; }
set { m_attributes = value; }
Expand Down Expand Up @@ -524,7 +526,8 @@ public bool HasBody {
(m_implAttrs & MethodImplAttributes.InternalCall) == 0 &&
(m_implAttrs & MethodImplAttributes.Native) == 0 &&
(m_implAttrs & MethodImplAttributes.Unmanaged) == 0 &&
(m_implAttrs & MethodImplAttributes.Runtime) == 0;
(m_implAttrs & MethodImplAttributes.Runtime) == 0 &&
(!m_enforceNoBody);
}
}

Expand Down Expand Up @@ -563,7 +566,19 @@ public MethodDefinition (string name, MethodAttributes attrs, TypeReference retu

internal void LoadBody ()
{
if (m_body == null && this.HasBody) {
// if m_rva is zero, it is assumed that there is no method body and it will not even
// try to read the actual code size value. Therefore, if it is zero, make sure to enforce
// there is no body. This is important for extern methods as there is no equivalent IL to detect.
//
// The default in cecil if you do have a method body is to have at least a RET instruction. If we were to let extern methods
// fall through, then that would interfere with mono's UnsafeAccessor detection as it tries to detect via body vs no body because
// it is much cheaper.
//
// See https://github.com/dotnet/runtime/issues/102045
if (m_rva == RVA.Zero) {
m_enforceNoBody = true;
}
else if (m_body == null && this.HasBody) {
m_body = new MethodBody (this);

ModuleDefinition module = DeclaringType != null ? DeclaringType.Module : null;
Expand Down

0 comments on commit 3ed40d4

Please sign in to comment.