[io-layer] Correctly detect a PE32+ assembly as managed when starting process #3745

Merged
merged 1 commit into from Oct 11, 2016

Projects

None yet

3 participants

@akoeplinger
Member

We weren't checking whether a PE file is in PE32 or PE32+ format, causing us to use the PE32 offsets on the newer format as well.

This means that using Process.Start() on a managed assembly didn't always work when a PE32+ file was in play. Such a file is created when targeting x64 e.g. via the -platform:x64 csc.exe/mcs option.

The reason why we probably didn't notice until now is that in an assembly produced by mcs there happens to be some data at the PE32 CLR header offset even in a PE32+ file, causing the check to "work".

This doesn't apply to csc.exe/roslyn though and so we couldn't execute those assemblies via Process.Start() even though they're perfectly managed. Since the .NET Core project.json toolchain produces x64-targeted assemblies by default more people ran into the issue trying to run those on Mono: cake-build/cake#1247

Fixes https://bugzilla.xamarin.com/show_bug.cgi?id=44937

PE/COFF spec at https://msdn.microsoft.com/en-us/library/windows/desktop/ms680547(v=vs.85).aspx

@akoeplinger akoeplinger [io-layer] Correctly detect a PE32+ assembly as managed when starting…
… process

We weren't checking whether a PE file is in PE32 or PE32+ format,
causing us to use the PE32 offsets on the newer format as well.

This means that using Process.Start() on a managed assembly didn't
always work when a PE32+ file was in play. Such a file is created
when targeting x64 e.g. via the -platform:x64 csc.exe/mcs option.

The reason why we probably didn't notice until now is that in an
assembly produced by mcs there happens to be some data at the PE32
CLR header offset even in a PE32+ file, causing the check to "work".

This doesn't apply to csc.exe/roslyn though and so we couldn't
execute those assemblies via Process.Start() even though they're
perfectly managed. Since the .NET Core project.json toolchain
produces x64-targeted assemblies by default more people ran into
the issue trying to run those on Mono: cake-build/cake#1247

Fixes https://bugzilla.xamarin.com/show_bug.cgi?id=44937

PE/COFF spec at https://msdn.microsoft.com/en-us/library/windows/desktop/ms680547(v=vs.85).aspx
68bcbb4
@akoeplinger
Member

@ludovic-henry want to review? (since according to the bugzilla you looked into it briefly)

@akoeplinger akoeplinger merged commit f7b73f0 into mono:master Oct 11, 2016

3 of 10 checks passed

Linux AArch64 Build finished. 44592 tests run, 1030 skipped, 1 failed.
Details
Linux i386 Build finished. 44563 tests run, 1024 skipped, 1 failed.
Details
Linux x64 FullAOT Build finished. 16405 tests run, 231 skipped, 4 failed.
Details
OS X i386 Build finished. 43237 tests run, 915 skipped, 30 failed.
Details
OS X x64 Build finished. 43360 tests run, 915 skipped, 5 failed.
Details
Windows i386 Build finished. 26325 tests run, 608 skipped, 33 failed.
Details
Windows x64 Build finished. 29800 tests run, 561 skipped, 0 failed.
Details
Linux ARMv5 soft float Build finished. 44567 tests run, 1027 skipped, 0 failed.
Details
Linux ARMv7 hard float Build finished. 44567 tests run, 1027 skipped, 0 failed.
Details
Linux x64 Build finished. 44594 tests run, 1024 skipped, 0 failed.
Details
@akoeplinger akoeplinger deleted the akoeplinger:fix-bug44937 branch Oct 11, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment