Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

更易读的测试用例;更友好的命名;兼容性更强的方法匹配;增加关键代码注释 #5

Merged
merged 15 commits into from
Dec 26, 2018

Conversation

linsmod
Copy link
Contributor

@linsmod linsmod commented Dec 21, 2018

No description provided.

@xiangyuecn
Copy link
Contributor

#4 😇

@bigbaldy1128
Copy link
Owner

非常感谢你的改进,我工作一直很忙,这个项目很久不弄了,能有大家的帮助让这个项目更加完善,真的是太好了,非常感谢!

@bigbaldy1128 bigbaldy1128 merged commit b5f9f58 into bigbaldy1128:master Dec 26, 2018
Copy link
Contributor

@xiangyuecn xiangyuecn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

改动过于巨大,难以兼容以前的老代码 😂 我尝试合并了一下我本地老版本的代码,放弃全部合并了,合并了大部分不影响老接口的方法,好用多了👍

public string TargetTypeFullName { get; internal set; }
public override string ToString()
{
return $"{TargetTypeFullName}.{MethodName}".ToString();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

C# 6.0 的$语法,用String.Format吧,低版本vs就这一处导致没法编译,好尴尬

var monitorAttribute = dest.GetCustomAttribute(typeof(MonitorAttribute)) as MonitorAttribute;
var methodName = dest.Name;
var paramTypes = dest.GetParameters().Select(t => t.ParameterType).ToArray();
if (monitorAttribute.Type != null)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

如果只提供了字符串形式的类型,后面代码将无法执行,detour.TargetType==null,105行将报错。确实是改动比较大,这个功能被改废了哦。

刚才尝试着我本地合并一下代码,发现不能运行了
[RelocatedMethodAttribute("ConsoleApplication1", "A")]我都是用的这种形式的,.Net内部类很难拿到Type,用字符串就简单多了。

type = asm.GetTypes().FirstOrDefault(t => t.Name == detour.TargetType.Name && t.Namespace == detour.TargetType.Namespace && t.Module.FullyQualifiedName == detour.TargetType.Module.FullyQualifiedName);
if (type == null)
{
var types = asm.GetTypes().Where(t => t.Name.StartsWith("Computer"));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

活捉手写的Computer,此处应该是一个什么类型的名字吗?

*(uint*)(newInstrPtr + 1) = (uint)customImplMethodPtr - (uint)rawMethodPtr - 5;
}

//因测试项目的特殊性,确保测试项目代码不会重入
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

测试调试《测试项目》,发现这部分代码并不会生效,多次运行还是照样提示内存已损坏,关掉执行引擎保持运行选项才不会出错。不知道是不是我测试方法错了,如果这个代码确实不会生效,那就移除掉吧

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

我指的是IsDetourInstalled函数

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

我想了好几遍,对于重入的可能性几乎没有,ClrMethodHook,Install只会运行一次(忽略错误的在超高并发代码中调用),后续InstallInternal调用是AssemblyLoad事件引发的,如果一个程序集被加载,就算是同一个dll,这个程序集是全新的(猜的,有时间我用CSharpCodeProvider动态编译验证一下)。

其实想说的是,如果此代码确实没用,Monitor.cs中engine.Patch那段代码可以少好多,哈哈哈

[AttributeUsage(AttributeTargets.Method)]
public class OriginalAttribute:Attribute
public class ShadowMethodAttribute : Attribute
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hook方法要写一遍类型+方法名称,原始函数还要写一遍,累的慌。其实最后发现是ShadowMethodAttribute .TargetMethodNameRelocatedMethodAttribute.TargetMethodName相同就能完成配对,其他属性是多余的。

甚至还可以进一步优化,修饰的方法的名称(我看例子里面都是这样的)==原始方法的名称==RelocatedMethodAttribute.TargetMethodName,(其实跟老版本MonitorAttribute修饰的Hook方法名称==原始方法的名称一样,减轻使用者敲代码负担)

var shadowMethod = detour.ShadowMethod;

//IsStatic必须一致,否则报内存访问错误
if (rawMethod.IsStatic != shadowMethod.IsStatic)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

此处要加上shadowMethod!=null,有可能没有提供原函数的ShadowMethod

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

此处检测代码好像并没有什么作用,我把public static string GetCpu()改成public string GetCpu()能通过测试,也许是你以前没有关掉执行引擎保持运行选项,反复测试代码导致错误。

如果虚拟原方法可以定义成非静态,不会出错的话,这段代码应该移除掉

}
if (type != null)
{
if (methodName == ".ctor")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

如果是hook构造函数,方法名直接用类名可能更好些。if(methodName == type.Name)...,并且下面这个methodName引用直接使用字符串.ctor

{
src = monitorAttribute.Type.GetMethod(methodName, paramTypes);
Type type = null;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

此处和下面一大段获取Type对象的代码感觉是多余的,因为detour.TargetType已经是目标类型的对象了(包括泛型),无需再通过自身来找自身。直接赋值Type type=detour.TargetType;

反而需要处理的是仅仅提供类型名字符串这种(类型名字符串更加不需要去处理泛型,因为如果这个类是泛型类,使用者给的类名字上会有类似^123

if(type==null){
   //通过类名字查找到类型对象
  ....
}

@linsmod
Copy link
Contributor Author

linsmod commented Dec 27, 2018

辛苦了!是我没写好测试样例。内部类都可以名称获取的。构造函数有实例和静态,.ctor是实例的。其他没有用到的是测试剩下的,都删了也好😁

@linsmod
Copy link
Contributor Author

linsmod commented Dec 27, 2018

指明方法名出于有Unicode的考虑,特殊字符编译不过的。

@xiangyuecn
Copy link
Contributor

xiangyuecn commented Dec 27, 2018

这两个还是要改一下:

  1. 这个$:

    return $"{TargetTypeFullName}.{MethodName}".ToString();

    优化成String.Format,兼容低版本C#

  2. 这段:

    Type type = null;
    if (detour.TargetType.IsGenericType)
    {
    type = asm.GetTypes().FirstOrDefault(t => t.BaseType == detour.TargetType.DeclaringType && detour.TargetType.GenericTypeArguments.SequenceEqual(t.GenericTypeArguments));
    if (type == null)
    {
    type = asm.GetTypes().FirstOrDefault(t => t.Name == detour.TargetType.Name && t.Namespace == detour.TargetType.Namespace && t.Module.FullyQualifiedName == detour.TargetType.Module.FullyQualifiedName);
    if (type == null)
    {
    var types = asm.GetTypes().Where(t => t.Name.StartsWith("Computer"));
    if (types.Any())
    {
    var m = type.GenericTypeArguments.SequenceEqual(detour.TargetType.GenericTypeArguments);
    }
    }
    else
    {
    type = type.MakeGenericType(detour.TargetType.GenericTypeArguments);
    }
    }
    }
    else
    {

    可以优化成:

Type type = detour.TargetType;
if (type==null)
{

移除其实是多余的代码,自然的修复了detour.TargetType==null时的异常。注:[RelocatedMethod("System.IO.File", "ReadAllText")]时为null,[RelocatedMethod(typeof(System.IO.File), "ReadAllText")]没有这种问题。


另外感觉上举的例子应该一半以上使用[RelocatedMethod("System.IO.File", "ReadAllText")]这种形式,使用Type对象我觉得还是算次要的,因为要用到这种类似反射的功能,基本上很大概率会和私有类型、方法打交道,用类型全名应景些,也有利于测试发现问题。


另外需要提供至少一个不带ShadowMethod方法的测试用例,也好发现问题,比如这处shadowMethod==null的引用:

if (rawMethod.IsStatic != shadowMethod.IsStatic)

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

Successfully merging this pull request may close these issues.

None yet

3 participants