Skip to content

Conversation

wwh1004
Copy link
Contributor

@wwh1004 wwh1004 commented Dec 30, 2021

Fixes #238

Test code:

using System;
using System.IO;
using dnlib.DotNet;
using dnlib.DotNet.Emit;
using dnlib.DotNet.Writer;

class Program {
	static void Main() {
		Test1();
		Console.ReadKey(true);
	}

	static void Test1() {
		using var module = ModuleDefMD.Load(typeof(Program).Module);
		var type = module.FindNormal("Program");
		var method = type.FindMethod("Test1");

		module.Resources.Add(null);
		// null resource

		type.CustomAttributes.Add(null);
		// null custom attribute

		method.ParamDefs.Add(new ParamDefUser { Constant = new ConstantUser() });
		// null constant

		var body = method.Body;
		for (int i = 0; i < ushort.MaxValue + 10; i++)
			body.Variables.Add(new Local(module.CorLibTypes.Int32));
		// too many locals

		body.Instructions[0] = new Instruction(OpCodes.Ldstr, 123);
		// mismatching operand type

		body.Instructions[1] = new Instruction(OpCodes.Pop);
		body.Instructions[2] = new Instruction(OpCodes.Pop);
		// evaluation stack overflow

		Test2();
		type.Remove(type.FindMethod("Test2"));
		// removed method but still referenced

		using var output = new MemoryStream();
		var writerOptions = new ModuleWriterOptions(module) { Logger = new Logger() };
		module.Write(output, writerOptions);
	}

	static void Test2() {
	}

	class Logger : ILogger {
		int count = 0;

		public bool IgnoresEvent(LoggerEvent loggerEvent) {
			return loggerEvent != LoggerEvent.Error && loggerEvent != LoggerEvent.Warning;
		}

		public void Log(object sender, LoggerEvent loggerEvent, string format, params object[] args) {
			Console.WriteLine($"{++count}. {string.Format(format, args)}");
			Console.WriteLine();
		}
	}
}

Before:

1. Constant is null

2. Custom attribute is null

3. Resource is null

4. Error calculating max stack value. If the method's obfuscated, set CilBody.KeepOldMaxStack or MetadataOptions.Flags (KeepOldMaxStack, global option) to ignore this error. Otherwise fix your generated CIL code so it conforms to the ECMA standard. At method System.Void Program::Test1() (06000002).

5. Too many locals, max number of locals is 65535 (0xFFFF)

6. Invalid instruction operand

7. Method System.Void Test2() (06000003) is not defined in this module (dnlib-test.dll). A method was removed that is still referenced by this module.

After:

1. Constant is null. Error occurred after metadata event MemberDefRidsAllocated during writing method 'System.Void Program::Test1()' (0x06000002).

2. Custom attribute is null. Error occurred after metadata event MostTablesSorted during writing type 'Program' (0x02000002).

3. Resource is null. Error occurred after metadata event BeginAddResources.

4. Error calculating max stack value. If the method's obfuscated, set CilBody.KeepOldMaxStack or MetadataOptions.Flags (KeepOldMaxStack, global option) to ignore this error. Otherwise fix your generated CIL code so it conforms to the ECMA standard. Error occurred after metadata event BeginWriteMethodBodies during writing method 'System.Void Program::Test1()' (0x06000002).

5. Too many locals, max number of locals is 65535 (0xFFFF). Error occurred after metadata event BeginWriteMethodBodies during writing method 'System.Void Program::Test1()' (0x06000002).

6. Invalid instruction operand. Error occurred after metadata event BeginWriteMethodBodies during writing method 'System.Void Program::Test1()' (0x06000002).

7. Method 'System.Void Test2()' (0x06000003) is not defined in this module 'dnlib-test.dll'. A method was removed that is still referenced by this module. Error occurred after metadata event BeginWriteMethodBodies during writing method 'System.Void Program::Test1()' (0x06000002).

If allows break changes in new major version, is it possible to merge IWriterError2 into IWriterError and delete Extensions.Error2? I think code can be more clear in that case.
https://github.com/wwh1004/dnlib/blob/a9bb8f9e7514504e22b9d98d2ac625d5b4d975bb/src/DotNet/Writer/IWriterError.cs#L39

@wtfsck
Copy link
Contributor

wtfsck commented Dec 30, 2021

I'll check this later when I have more time. I just had a very quick check.

  • I think this fixes Show more information. #238 , and if so add Fixes #238 to your first post.
  • I think the responsibility of adding the . is in the other code, not the callers of Error(). There will be less changes and we won't forget to add a period in the future if we add another error.

Copy link
Contributor

@wtfsck wtfsck left a comment

Choose a reason for hiding this comment

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

Looks good, just a few problems, including the extra ".".

@wtfsck
Copy link
Contributor

wtfsck commented Jan 31, 2022

See also my comments here: #447 (comment)

@CreateAndInject
Copy link
Contributor

CreateAndInject commented Mar 5, 2022

@wtfsck Could you complete this PR yourself or bump version without this PR? We need other updates. This PR is sended more than 2 months, maybe @wwh1004 has stopped working on it.

@wtfsck
Copy link
Contributor

wtfsck commented Mar 18, 2022

@wwh1004 are you still working on this PR?

@wwh1004
Copy link
Contributor Author

wwh1004 commented Mar 18, 2022

@wwh1004 are you still working on this PR?
I will update it these days.

@wwh1004 wwh1004 force-pushed the better-module-writer-log branch from 9bf3c74 to 4262c8e Compare March 23, 2022 07:00
@wwh1004 wwh1004 force-pushed the better-module-writer-log branch from 4262c8e to f2007ae Compare March 23, 2022 07:23
@wwh1004
Copy link
Contributor Author

wwh1004 commented Mar 23, 2022

Updated

@wwh1004
Copy link
Contributor Author

wwh1004 commented Mar 24, 2022

Updated

@wtfsck wtfsck merged commit dafe486 into 0xd4d:master Mar 24, 2022
@wtfsck
Copy link
Contributor

wtfsck commented Mar 24, 2022

Thanks!

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.

Show more information.
3 participants