IKVM.Reflection ILGenerator/PDB Rewrite

[wasabii]

Rewrote ILGenerator by taking the .NET Core RuntimeILGenerator code and a bit of code from MethodBuilder and retrofitting it to support ISymbolWriter and the other stuff it needed for saving. The end result is a hodge podge of code from Core and some Framework retrofits that were removed when AssemblyBuilder.Save was removed in Core.

But the end result is an ILGenerator instance that borrows the logic from .NET Core. Including it's more modern ECMA allowances, like backwards branches, etc. It generates IL exactly like .NET Core System.Reflection does since it's no longer our code.

PDB support is implemented through MetadataPdbSymbolWriter, which holds the symbol information until it's requested to write it into a MetadataBuilder. ModuleWriter now takes various options for a PdbStream and/or output file name and checksum hash. and uses PortablePdbBuilder to output the portable PDB metadata.

Removed MDB support and non-Portable PDB support. I'm sure Roslyn wishes they could do this too.

So, this PR should make it possible to generate and use IKVM PDBs on non-Windows.

ikvmc debug options are now more inline with csc.exe: -debug, -debug:full, :portable, :embedded. -debug:portable is the default and enabled on both IKVM.NET.Sdk and IkvmReference when DebugType is set.

[wasabii]

The ikvm.lang.DllExport feature was removed. This was insane. Generating native stubs? No need for this. Removed so we didn't have to fix up the native stub generation stuff.... because yeah.

[wasabii]

I think probably the best path forward for IKVM.Reflection.Emit at this point is to simply copy the .NET Core implementation of System.Reflection.Emit and patch up the missing peices (Saving, Symbols, etc). But, because our ModuleBuilder is pretty tied to our Module, and our AssemblyBuilder is pretty tied to our Assembly, etc, this isn't an easy task without also taking System.Reflection itself. Which might be an option. It might not be too hard to retrofit SRM into the reading side of things. This PR doesn't do that.

[wasabii]

Awesome. Thanks so much for taking a look.

[AaronRobinsonMSFT]

@wasabii This is a known issue and a fix has been made for it. Still working through when this will ship though.

[wasabii]

@AaronRobinsonMSFT

No workaround though? Can you explain the issue for me, or get me towards where it's described?

Is it planned to be fixed in all runtime versions?

[AaronRobinsonMSFT]

No workaround though? Can you explain the issue for me, or get me towards where it's described?

@wasabii The first indication was at dotnet/fsharp#16399.

Is it planned to be fixed in all runtime versions?

Not at present since using DebuggableAttribute.IgnoreSymbolStoreSequencePoints should address the issue. Once we confirm the assemblies all have that bit set, we will make that kind of decision.

/cc @hoyosjs

[hoyosjs]

IKVM.Java is the first module I see that lacks the attribute and triggers the issue. There may be more - this is just the first one I know triggers the issue.

[wasabii]

Okay. So.... maybe I'm not understanding. SequencePoints then play two pivotale roles, right? One is to provide the debugger with safe spots that can be referred to: but this isn't needed, as the information can also be derived from NOPs that have been inserted. But, the other goal of sequence points, and the one I'm most concerned with, is line/column offsets in the active Document.

Am I correct then that placing this attribute on an assembly causes the Sequence Points to be ignored for the purposes of the primary issue? But, continues to allow them to be used by the debugger to determine where in the source file you are?

[hoyosjs]

Yes, that flag is concerned with the JIT's use of that information vs the implicit sequence points. The PDB still gets used when doing Exception.ToString()

[wasabii]

@AaronRobinsonMSFT

Any idea why the addition of this attribute might result in prolific OutOfMemoryExceptions?

Specifically only on Windows. Linux and OSX seem to work fine.

[AaronRobinsonMSFT]

@AaronRobinsonMSFT

Any idea why the addition of this attribute might result in prolific OutOfMemoryExceptions?

Specifically only on Windows. Linux and OSX seem to work fine.

No. That is unexpected. If you can reproduce this please share or attach WinDBG and collect some of the stacks that are causing the OOMs.

[wasabii]

I haven't been able to locally. Yet. Everything that is failing in our CI seems to work fine locally. Going to try to run one of these failing tests in a loop and see if it hits harder.

[wasabii]

@AaronRobinsonMSFT The OOM errors have gone away after I disabled Code Coverage tracking during tests. So, if there's an issue, it relates to that. I am completely unfamiliar with how the code coverage stuff operates at this point. Gotta dig into that.

[wasabii]

@AaronRobinsonMSFT

Okay. So, the crash is gone with code coverage out. But, I'm still seeing a bunch of internal BadImageFormatExceptions happening during the collection of stack traces.

Exception thrown: 'System.BadImageFormatException' in System.Reflection.Metadata.dll

I can get a debugger into most of this.

Inside of System.Diagnostics.StackFrameHelper:InitializeSourceInfo.

This block of code:

                for (int index = 0; index < iFrameCount; index++)
                {
                    // If there was some reason not to try get the symbols from the portable PDB reader like the module was
                    // ENC or the source/line info was already retrieved, the method token is 0.
                    if (rgiMethodToken![index] != 0)
                    {
                        s_getSourceLineInfo!(rgAssembly![index], rgAssemblyPath![index]!, rgLoadedPeAddress![index], rgiLoadedPeSize![index], rgiIsFileLayout![index],
                            rgInMemoryPdbAddress![index], rgiInMemoryPdbSize![index], rgiMethodToken![index],
                            rgiILOffset![index], out rgFilename![index], out rgiLineNumber![index], out rgiColumnNumber![index]);
                    }
                }

It's calling into s_getSourceLineInfo, which itself is calling System.Reflection.Throw.ThrowOutOfBounds. in System.Reflection.Metadata.

I can't debug into that easily right now. But, what I think is happening is it's failing to load the MethodDebugInfo associated with the MethodDef. Looking into these arrays, it fails on, say, index 9 (just an example). So, in rgiMethodToken, I see index 9 has the value of 100798173. Assuming that's supposed to be a MethodDefHandle...... that value is way too high. So, it turns it into a MethodDebugInformationHandle and tries to read that and it's way beyond the end of the table so it fails.

There are "only" 252925 rows in the MethodDef table. So, a value of 798173 is way outside that. I'm pretty sure these aren't like compressed ints or anything, but just tokens. So, 100798173 should be row id 798173....

So yeah, way off the end of the table.

I can't easily trace this back into the origin of these tokens. So I'm not sure where it's getting these numbers.... but it seems like it is getting the wrong ones.

[wasabii]

Okay. I made a mistake. That number is correct. What I'm actually seeing in this attempt though is 0x06000000. Which is a Nil token.

But I think I understand it now. That's a dynamic method.

Okay. I think there is something I'd consider a bug in here. When PortablePDBs are enabled for an assembly, and there are dynamic methods attached to it, the token makes it through into here as a nil token with a methoddef kind. Thus it doesn't get filtered out by the if (handle.Kind == HandleKind.MethodDefinition) line. And the lookup attempts to go through to the MetadataReader attached to the PDB. Which then throws an OutOfBounds exception. Which I believe itself needs a stacktrace.

[wasabii]

dotnet/runtime#98506