Add support for arbitrary IL patching

[gregros]

Add support for arbitrary IL patching

[GregRos]

I'm glad to see this framework is being used! I haven't worked on it for ages and I haven't seen many people use it. Could you tell me what project you're working on?

Your Proposal

I still remember my code though. Unfortunately, what you're proposing would require A LOT of work and has a few fundamental issues. Unless you know IL and the quirks of my framework very well, it would also be very hard to work with.

1. You'd need to reference Mono.Cecil from your patch assembly, which is a bad idea. It might end up forcing the target assembly to reference it.
2. I would need to execute the patch assembly. Technically I already do this, but I never try to run code in patching types, as they're rife of references to things like the source assembly and other dependencies. Also, it would require more work with managing AppDomains, which is a pain in the ass. Patch assemblies might have to use different framework versions than the patching engine which is a MASSIVE pain to deal with when you try to execute them.
3. For users, editing IL is extremely hard. There are a lot of gotchas and it's easy to make mistakes that will cause runtime errors and you can end up writing invalid or unverifiable code which may sometimes refuse to execute.
   It would be practically impossible to debug.
4. Mono.Cecil is hard to work with as well. Or at least it was when I was working on this project.

Better than editting IL, it might be possible to decompile the method to a syntax tree and have you edit that. It's a lot more user-friendly than editing IL and can even throw errors during the patching process, instead of when the application is executing.

(On the other hand, sometimes this would fail because languages other than C# can use IL features not present in C# and so cannot generate valid syntax trees, and even C# can compile to IL that decompiles into invalid C# because of code generation).

It would still be a lot of work though.

On the other hand, if you're asking for a feature like this, maybe you know more about this than I do. In that case, you can try to work on it yourself or give me a detailed explanation of how to do it and I'll try to find time to work on it.

About overloads

From the readme:

About Overloading

When you put an attribute on a code element, the framework will usually use that element's name (or an alternative name you supply) and, in the case of methods and properties, their parameters, to find what to modify.

To modify one of several overloaded methods, you just need to duplicate that method's parameter types exactly.

Note that return types of existing methods cannot be modified.

[awgil]

The project is nothing major - I'm just playing through PoE, making some tweaks and bugfixes along the way, and thought to package and publish them as mods. For local changes, I've just patched the assembly manually - however, I'd like to publish them as several small independent mods, so just distributing a patched assembly is not possible. So I've investigated a bit and found this awesome framework, which seems to be a community standard.

Re. patching IL directly - this is really the first time I'm working with .NET, so I've approached it in the same way I tweaked native x86 code before. Probably that's why I immediately looked for such a feature :) I agree that performing arbitrary edits on IL is hard - however, what I had in mind were relatively simple modifications (e.g. nopping/deleting a block, changing some condition for a branch, inserting a call to a custom function written in proper C# at a specific point and maybe adding a branch depending on return value) - these are relatively harmless and quite useful (at least, this is the common way of patching native code - however, maybe I'm missing something due to my inexperience with .NET).

I already did try to implement this myself before posting here, and indeed I've found this to be harder than I originally thought - that's why I've opened an issue instead of making a pull request :) Maybe it'll be possible to restrict this to several more specific editing operations, which can be represented as a set of annotations (e.g. [DeleteBlock(...)], [InsertCallAt(...)], etc. applied together with [DuplicatesBody]) and get most of the benefit without such a big hassle. I'll think more about it and experiment...

Re. overloading - yeah, I've seen that, what I meant is that with syntax proposed in my original post we would need some other way of selecting proper overload, since patching function's signature would be fixed.

Some more questions (sorry if they are noob ones :) ):

• when you're saying that referencing Cecil from patch assembly might cause target assembly to reference it as well, does it mean that Patchwork can implicitly add references to target? Why would it do that? Couldn't it just error out if patch code references something not already referenced by the target, unless there is an explicit instruction to add a reference?
• why is it important for patch assembly to use the same framework version as a target? Can't Patchwork simply error out if patch assembly uses features not available in framework used by target? Are differences between framework versions too large for that to be feasible?
• is the main reason for using AppDomain rather than loading patch assembly directly being possible framework version differences?

[GregRos]

Ah, cool. Thanks for telling me about what you're doing 😄

If you've tweaked native code before, you probably know what you're doing. It's just that you need to be aware of the precise gotchas of doing it with IL. But you can probably find those things out. Just don't touch references to types.

What's stopping you from manually tweaking the resulting assembly after Patchwork is done with it? I imagine it would be easier with purpose-made tools than with an automatic framework.

---

I already did try to implement this myself before posting here, and indeed I've found this to be harder than I originally thought - that's why I've opened an issue instead of making a pull request :) Maybe it'll be possible to restrict this to several more specific editing operations, which can be represented as a set of annotations (e.g. [DeleteBlock(...)], [InsertCallAt(...)], etc. applied together with [DuplicatesBody]) and get most of the benefit without such a big hassle. I'll think more about it and experiment.

To be honest, I imagine it would be a pretty huge effort. It would be completely different from how the framework works right now. The attributes you propose would be A LOT easier to implement, but they also sound pretty hard to work with and of limited use. Note that there is a limit to what you can put inside an attribute.

why is it important for patch assembly to use the same framework version as a target? Can't Patchwork simply error out if patch assembly uses features not available in framework used by target? Are differences between framework versions too large for that to be feasible?

It's not that the differences are large in a perceivable way. It's just that they are everywhere. Standard types like System.Int32 might need to be mapped from one version to another. Some commands that the compiler will allow for some versions of the framework will fail at runtime for another version. Some metadata the compiler emits for some versions will be invalid in later versions.

is the main reason for using AppDomain rather than loading patch assembly directly being possible framework version differences?

It's a big reason, but not the only one.

The central reason is that when you load an assembly, you can never unload it and when you execute code inside it, it will start loading its own dependencies on the fly, which you also can't get rid of.

When this happens and as a response to what is hard to predict. If you only handle types that don't reference anything other than framework libraries, you're probably safe. But once you start using types that reference types in the patched assembly, you'll get the patched assembly as a dependency (so you can't patch it) and it will start loading its own dependencies (so now you need to run in the same environment as it runs in, which isn't always possible).

If you load things inside a separate AppDomain, you can always destroy it after you're done with it, thus no longer depending on the assemblies it depended on.

[awgil]

Ok, I'll close the issue, since it seems to be hard to implement in a generic way, and there are workarounds available.