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

ILLinker corrupts exception handler ranges for method #1507

Closed
BrzVlad opened this issue Sep 25, 2020 · 6 comments · Fixed by #1512
Closed

ILLinker corrupts exception handler ranges for method #1507

BrzVlad opened this issue Sep 25, 2020 · 6 comments · Fixed by #1512

Comments

@BrzVlad
Copy link
Member

BrzVlad commented Sep 25, 2020

Steps to reproduce:

  1. Install VS 16.8 Preview 3 and .net 5.0 rc1
  2. Build https://github.com/modulardev2/ThreadException (Client wasm project)
  3. Go to binaries folder, disassemble System.Net.Http.dll, look for method <GetStringAsyncCore>d__41 MoveNext (). The EH clause ranges for the method are correct
  4. Publish the same project (folder publish). Go to binaries folder, disassemble System.Net.Http.dll, look for same method as before and notice the clause ranges have been corrupted.

As part for publishing, _RunILLink is ran. This goes over the assemblies from the sdk, which are correct, and outputs them in the obj\Release\net5.0\linked\ folder. At this moment the method is corrupted. The linker seems to run as follows : https://gist.github.com/BrzVlad/e0f358c9d096c5acef745a1c49aae8d1

While I haven't tried building the project from linux with dotnet command, the behavior should be the same.

@BrzVlad
Copy link
Member Author

BrzVlad commented Sep 25, 2020

cc @marek-safar

Causes release blocking issue dotnet/runtime#42628

@BrzVlad BrzVlad changed the title ILLinker corrupts exception ranges for method ILLinker corrupts exception handler ranges for method Sep 25, 2020
@marek-safar marek-safar added this to the .NET 5.0 milestone Sep 25, 2020
@eerhardt
Copy link
Member

I have narrowed down a repro that produces the issue even outside of brower-wasm:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net5.0</TargetFramework>
    <PublishTrimmed>true</PublishTrimmed>
    <TrimMode>link</TrimMode>
    <EventSourceSupport>false</EventSourceSupport>
  </PropertyGroup>

</Project>
using System;
using System.Threading.Tasks;
using System.Net.Http;

namespace LinkerEHBug
{
    class Program
    {
        static async Task Main(string[] args)
        {
            var http = new HttpClient();
            var value = await http.GetStringAsync("http://www.msn.com");
            Console.WriteLine(value);
        }
    }
}

dotnet.exe publish -c Release -r win-x64 the above application, and run it. I get:

❯ .\bin\Release\net5.0\win-x64\publish\LinkerEHBug.exe
Unhandled exception. System.InvalidProgramException: Common Language Runtime detected an invalid program.
   at System.Net.Http.HttpClient.GetStringAsyncCore(HttpRequestMessage request, CancellationToken cancellationToken)
   at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[TStateMachine](TStateMachine& stateMachine)
   at System.Net.Http.HttpClient.GetStringAsyncCore(HttpRequestMessage request, CancellationToken cancellationToken)
   at System.Net.Http.HttpClient.GetStringAsync(Uri requestUri, CancellationToken cancellationToken)
   at System.Net.Http.HttpClient.GetStringAsync(Uri requestUri)
   at System.Net.Http.HttpClient.GetStringAsync(String requestUri)
   at LinkerEHBug.Program.Main(String[] args) in F:\DotNetTest\LinkerEHBug\Program.cs:line 12
   at LinkerEHBug.Program.<Main>(String[] args)

I found that removing <EventSourceSupport>false</EventSourceSupport> allows the program to work properly. This property is set automatically for Blazor WASM apps, which is why it was repro'ing in Blazor by default.

cc @vitek-karas @MichalStrehovsky

@marek-safar
Copy link
Contributor

The bug will be most likely in the substitution implementation for exception blocks with filters. This does not look like a regression on linker side but more like a bug exposed by changes in libraries

@lewing
Copy link
Member

lewing commented Sep 25, 2020

cc @kg

@eerhardt
Copy link
Member

This does not look like a regression on linker side but more like a bug exposed by changes in libraries

I think this change to HttpClient.GetStringAsyncCore in the libraries is exposing this linker bug - dotnet/runtime@6b1b038#diff-256c040533e94c4e518e0453c5a7d4a0R176

An extra try-catch-finally was added to that method:

image

With EventSourceSupport=false, it looks like that new finally may be empty since HttpTelemetry.Log.IsEnable() will return false.

@marek-safar
Copy link
Contributor

Simple repro

using System;

class X
{
	public static void Main ()
	{
		Test ();
	}

	static bool P => true;

	static void Test ()
	{
		try {
			if (!P)
				M ();

			M ();
		} catch when (Log ()) {
		}
	}

	static bool Log ()
	{
		return false;
	}

	static void M ()
	{
	}
}

marek-safar added a commit to marek-safar/linker that referenced this issue Sep 26, 2020
marek-safar added a commit to marek-safar/linker that referenced this issue Sep 26, 2020
marek-safar added a commit to marek-safar/linker that referenced this issue Sep 26, 2020
marek-safar added a commit to marek-safar/linker that referenced this issue Sep 26, 2020
agocke pushed a commit to dotnet/runtime that referenced this issue Nov 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants