MonoMod icon indicating copy to clipboard operation
MonoMod copied to clipboard

Detouring methods that return Nullable<bool> crashes or results in a NRE

Open pardeike opened this issue 1 year ago • 1 comments

Description

This is a follow up from this bug report on Harmony: https://github.com/pardeike/Harmony/issues/640

I cooked it down to a minimal case without Harmony or dynamic methods:

Example

// tested on Win 64bit, .NET 4.7.2
// but Harmony tests fail on different combinations

using MonoMod.Core;
using System;
using System.Reflection;

class Program
{
    static void Main()
    {
        var original = typeof(SomeClass).GetMethod(nameof(SomeClass.Test), BindingFlags.Public | BindingFlags.Instance) ?? throw new NullReferenceException();
        var replacement = typeof(SomeClass).GetMethod(nameof(SomeClass.Replacement), BindingFlags.Public | BindingFlags.Static) ?? throw new NullReferenceException();
        Console.WriteLine($"Methods {original} {replacement}");
        DetourFactory.Current.CreateDetour(original, replacement);

        var instance = new SomeClass();
        instance.Test();

        Console.WriteLine("Done");
    }
}

public class SomeClass
{
    public static bool? Replacement(SomeClass instance)
    {
        _ = instance;
        Console.WriteLine("Replacement");
        return true;
    }

    public bool? Test()
    {
        Console.WriteLine("Test2");
        return false;
    }

    // comment out this to avoid the error
    public string s1 = "test";
}

Observations:

  • fields in the source method class do matter somehow (without the field, the bug does not appear)
  • making the replacement non-static does not exhibit the bug
  • changing the bool? to int? won't trigger the bug

pardeike avatar Dec 29 '24 20:12 pardeike

For reference, these are the matrix tests that failed before I removed the test that tests the above case. You can see that it works on many platforms but not on all:

https://github.com/pardeike/Harmony/actions/runs/12431102708/job/34707941441

pardeike avatar Dec 29 '24 20:12 pardeike