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

Async partial method crashes emission of portable debug info #17934

Closed
rolfbjarne opened this Issue Mar 17, 2017 · 13 comments

Comments

Projects
None yet
7 participants
@rolfbjarne

rolfbjarne commented Mar 17, 2017

Version Used:
2ba0d8f (master) - built locally on macOS.

Steps to Reproduce:

Compile the following code snippet with /debug:portable:

csc test.cs /t:library /debug:portable
public partial class TestClass {
    partial void X ();
    async partial void X () {}
}

Expected Behavior:

No crash

Actual Behavior:

Unhandled Exception: System.Collections.Generic.KeyNotFoundException: The given key was not present in the dictionary.
   at System.ThrowHelper.ThrowKeyNotFoundException()
   at System.Collections.Generic.Dictionary`2.get_Item(TKey key)
   at Microsoft.Cci.FullMetadataWriter.DefinitionIndex`1.get_Item(T item)
   at Microsoft.Cci.FullMetadataWriter.GetMethodDefinitionHandle(IMethodDefinition def)
   at Microsoft.Cci.MetadataWriter.SerializeMethodDebugInfo(IMethodBody bodyOpt, Int32 methodRid, StandaloneSignatureHandle localSignatureHandleOpt, LocalVariableHandle& lastLocalVariableHandle, LocalConstantHandle& lastLocalConstantHandle)
   at Microsoft.Cci.MetadataWriter.SerializeMethodBodies(BlobBuilder ilBuilder, PdbWriter nativePdbWriterOpt, Blob& mvidStringFixup)
   at Microsoft.Cci.MetadataWriter.BuildMetadataAndIL(PdbWriter nativePdbWriterOpt, BlobBuilder ilBuilder, BlobBuilder mappedFieldDataBuilder, BlobBuilder managedResourceDataBuilder, Blob& mvidFixup, Blob& mvidStringFixup)
   at Microsoft.Cci.PeWriter.WritePeToStream(EmitContext context, CommonMessageProvider messageProvider, Func`1 getPeStream, Func`1 getPortablePdbStreamOpt, PdbWriter nativePdbWriterOpt, String pdbPathOpt, Boolean allowMissingMethodBodies, Boolean isDeterministic, CancellationToken cancellationToken)
   at Microsoft.CodeAnalysis.Compilation.SerializeToPeStream(CommonPEModuleBuilder moduleBeingBuilt, EmitStreamProvider peStreamProvider, EmitStreamProvider pdbStreamProvider, Func`1 testSymWriterFactory, DiagnosticBag diagnostics, Boolean metadataOnly, CancellationToken cancellationToken)
   at Microsoft.CodeAnalysis.CommonCompiler.RunCore(TextWriter consoleOutput, ErrorLogger errorLogger, CancellationToken cancellationToken)
   at Microsoft.CodeAnalysis.CommonCompiler.Run(TextWriter consoleOutput, CancellationToken cancellationToken)
   at Microsoft.CodeAnalysis.CSharp.CommandLine.Csc.<>c__DisplayClass1_0.<Run>b__0(TextWriter tw) in /Users/rolf/test/roslyn/src/Compilers/Shared/Csc.cs:line 26
   at Microsoft.CodeAnalysis.CommandLine.ConsoleUtil.RunWithUtf8Output[T](Boolean utf8Output, TextWriter textWriter, Func`2 func) in /Users/rolf/test/roslyn/src/Compilers/Core/CommandLine/ConsoleUtil.cs:line 54
   at Microsoft.CodeAnalysis.CSharp.CommandLine.Csc.Run(String[] args, BuildPaths buildPaths, TextWriter textWriter, IAnalyzerAssemblyLoader analyzerLoader) in /Users/rolf/test/roslyn/src/Compilers/Shared/Csc.cs:line 26
   at Microsoft.CodeAnalysis.CommandLine.CoreClrBuildClient.RunLocalCompilation(String[] arguments, BuildPaths buildPaths, TextWriter textWriter) in /Users/rolf/test/roslyn/src/Compilers/Shared/CoreClrBuildClient.cs:line 47
   at Microsoft.CodeAnalysis.CommandLine.BuildClient.RunCompilation(IEnumerable`1 originalArguments, BuildPaths buildPaths, TextWriter textWriter) in /Users/rolf/test/roslyn/src/Compilers/Shared/BuildClient.cs:line 85
   at Microsoft.CodeAnalysis.CommandLine.CoreClrBuildClient.Run(IEnumerable`1 arguments, RequestLanguage language, CompileFunc compileFunc) in /Users/rolf/test/roslyn/src/Compilers/Shared/CoreClrBuildClient.cs:line 42

@marek-safar

This comment has been minimized.

Show comment
Hide comment
@marek-safar

marek-safar Mar 17, 2017

Contributor

/cc @tmat

Contributor

marek-safar commented Mar 17, 2017

/cc @tmat

@marek-safar

This comment has been minimized.

Show comment
Hide comment
@marek-safar

marek-safar Mar 21, 2017

Contributor

@Pilchie why is it Area-Interactive when it happens during any compilation?

Contributor

marek-safar commented Mar 21, 2017

@Pilchie why is it Area-Interactive when it happens during any compilation?

@Pilchie

This comment has been minimized.

Show comment
Hide comment
@Pilchie

Pilchie Mar 21, 2017

Member

Generally Area-Interactive owns the pdb emission stuff AFAIK.

Member

Pilchie commented Mar 21, 2017

Generally Area-Interactive owns the pdb emission stuff AFAIK.

@tmat tmat self-assigned this Mar 21, 2017

@tmat tmat added this to the 15.3 milestone Mar 21, 2017

@tmat tmat closed this in #18724 Apr 26, 2017

@jcouv

This comment has been minimized.

Show comment
Hide comment
@jcouv

jcouv May 12, 2017

Member

@marek-safar This issue was fixed by #18724, in dev15.3. Please escalate to Jared if this is not a sufficient servicing approach.

Member

jcouv commented May 12, 2017

@marek-safar This issue was fixed by #18724, in dev15.3. Please escalate to Jared if this is not a sufficient servicing approach.

@marek-safar

This comment has been minimized.

Show comment
Hide comment
@marek-safar

marek-safar May 13, 2017

Contributor

@jcouv @jaredpar I don't know what is in a scope for 15.3 for roslyn but we need this as a fix for dev15.2 and I guess it'd be odd to patch dev15.2 with 15.3 preview.

Contributor

marek-safar commented May 13, 2017

@jcouv @jaredpar I don't know what is in a scope for 15.3 for roslyn but we need this as a fix for dev15.2 and I guess it'd be odd to patch dev15.2 with 15.3 preview.

@jcouv

This comment has been minimized.

Show comment
Hide comment
@jcouv

jcouv May 13, 2017

Member

@marek-safar 15.3 includes C# 7.1 (async Main, default literal, inferred tuple names, ref assemblies, pattern-matching with generic types) and all the bug fixes since 15.0. Less than 5 compiler bugs met the bar for 15.1 or 15.2.

As for pulling this fix into 15.2, I'll let @jaredpar comment. I'm not sure where that release is at.

Member

jcouv commented May 13, 2017

@marek-safar 15.3 includes C# 7.1 (async Main, default literal, inferred tuple names, ref assemblies, pattern-matching with generic types) and all the bug fixes since 15.0. Less than 5 compiler bugs met the bar for 15.1 or 15.2.

As for pulling this fix into 15.2, I'll let @jaredpar comment. I'm not sure where that release is at.

@jaredpar

This comment has been minimized.

Show comment
Hide comment
@jaredpar

jaredpar May 13, 2017

Member

The 15.2 release is hard escrow at the moment. Getting this in for 15.2 seems very unlikely due to where we are in the release, the components involved and the size of the fix. If this is critical for 15.2 we need to begin escalating internally to the directors.

Member

jaredpar commented May 13, 2017

The 15.2 release is hard escrow at the moment. Getting this in for 15.2 seems very unlikely due to where we are in the release, the components involved and the size of the fix. If this is critical for 15.2 we need to begin escalating internally to the directors.

@migueldeicaza

This comment has been minimized.

Show comment
Hide comment
@migueldeicaza

migueldeicaza May 19, 2017

Member

Could we consider a hot fix as an alternative? Something we could give developers while 15.3 happens or while issues escalate?

We have a number of developers that are running into this and can only get their code to build by reverting back to 15.1:

https://forums.xamarin.com/discussion/93701/visual-studio-for-mac-csc-exe-exited-with-code-1-when-building-xamarin-ios-project

Member

migueldeicaza commented May 19, 2017

Could we consider a hot fix as an alternative? Something we could give developers while 15.3 happens or while issues escalate?

We have a number of developers that are running into this and can only get their code to build by reverting back to 15.1:

https://forums.xamarin.com/discussion/93701/visual-studio-for-mac-csc-exe-exited-with-code-1-when-building-xamarin-ios-project

@jaredpar

This comment has been minimized.

Show comment
Hide comment
@jaredpar

jaredpar May 19, 2017

Member

The fix is readily available. Any of our 2.3.0-beta2 packages from our myget feed will have this fix already. For example here is todays build.

I'm unsure how to distribute this patch to Xamarin Stuido though. Distributing to Visual Studio is fairly easy because one of our NuGet packages is designed specifically to patch csproj / vbproj files to use the compiler in the NuGet. I'm not sure how do to the same for Xamarin Studio.

Member

jaredpar commented May 19, 2017

The fix is readily available. Any of our 2.3.0-beta2 packages from our myget feed will have this fix already. For example here is todays build.

I'm unsure how to distribute this patch to Xamarin Stuido though. Distributing to Visual Studio is fairly easy because one of our NuGet packages is designed specifically to patch csproj / vbproj files to use the compiler in the NuGet. I'm not sure how do to the same for Xamarin Studio.

@marek-safar

This comment has been minimized.

Show comment
Hide comment
@marek-safar

marek-safar May 19, 2017

Contributor

@jaredpar as I explained on several occasions this nuget is Windows only and won't easily work with VS4M, we also support command line scenarios which this does not address.

We could replace Roslyn 2.0 RTM with this version of Roslyn on our Alpha channel for everyone if you have confidence it has good quality but I could not find any info what this version includes and what could potentially break.

Contributor

marek-safar commented May 19, 2017

@jaredpar as I explained on several occasions this nuget is Windows only and won't easily work with VS4M, we also support command line scenarios which this does not address.

We could replace Roslyn 2.0 RTM with this version of Roslyn on our Alpha channel for everyone if you have confidence it has good quality but I could not find any info what this version includes and what could potentially break.

@jaredpar

This comment has been minimized.

Show comment
Hide comment
@jaredpar

jaredpar May 19, 2017

Member

@jaredpar as I explained on several occasions this nuget is Windows only and won't easily work with VS4M, we also support command line scenarios which this does not address.

Understood. There are other NuGets on that feed. As we've also said on several occasions, if there is a NuSpec we could use to generate a VS4M image we'd be more than happy to do so. But we don't have the expertise to create it.

We could replace Roslyn 2.0 RTM with this version of Roslyn on our Alpha channel

You just stated this won't work. So I don't understand how this helps.

Member

jaredpar commented May 19, 2017

@jaredpar as I explained on several occasions this nuget is Windows only and won't easily work with VS4M, we also support command line scenarios which this does not address.

Understood. There are other NuGets on that feed. As we've also said on several occasions, if there is a NuSpec we could use to generate a VS4M image we'd be more than happy to do so. But we don't have the expertise to create it.

We could replace Roslyn 2.0 RTM with this version of Roslyn on our Alpha channel

You just stated this won't work. So I don't understand how this helps.

@marek-safar

This comment has been minimized.

Show comment
Hide comment
@marek-safar

marek-safar May 19, 2017

Contributor

I perhaps misunderstood your point with the nuget where I though you suggested we tell the customers that the workaround is to add this nuget dependency to their project to fix the issue, which is the way VS does it if I remember correctly.

Using our alpha channel is more disturbing as it affect anyone automatically on that channel hence I asked about the quality of that nuget and if you are confident about it so we can offer it to much wider audience automatically.

Contributor

marek-safar commented May 19, 2017

I perhaps misunderstood your point with the nuget where I though you suggested we tell the customers that the workaround is to add this nuget dependency to their project to fix the issue, which is the way VS does it if I remember correctly.

Using our alpha channel is more disturbing as it affect anyone automatically on that channel hence I asked about the quality of that nuget and if you are confident about it so we can offer it to much wider audience automatically.

@jaredpar

This comment has been minimized.

Show comment
Hide comment
@jaredpar

jaredpar May 19, 2017

Member

I perhaps misunderstood your point with the nuget where I though you suggested we tell the customers that the workaround is to add this nuget dependency to their project to fix the issue, which is the way VS does it if I remember correctly.

Sorry for the misunderstanding here. That is the package that I give out the most. As you noted it's tailor made to patch into VS and unblock customers. Hence I just default to sending out a link to that package.

Meant the link to be a general example of how to grab packages out of our nightly feed. All of our NuGets are available there and we produce them nightly. Hence anything you depend on will be available there.

Using our alpha channel is more disturbing as it affect anyone automatically on that channel hence I asked about the quality of that nuget and if you are confident about it so we can offer it to much wider audience automatically.

The quality of our nightly feed in the master branch is very high. Subscribing to that feed and picking packages that have the version glob of 2.3.0-beta2* will keep you on master.

We strive to keep the master branch ship ready for compiler at all times. In addition to building Roslyn on every check in, nightly we build all of Visual Studio with the latest NuGet package + run their validation tests. This helps us ensure we keep ourselves clean here.

In particular now the quality is higher because we're in lock down for C# 7.1. 😄

Member

jaredpar commented May 19, 2017

I perhaps misunderstood your point with the nuget where I though you suggested we tell the customers that the workaround is to add this nuget dependency to their project to fix the issue, which is the way VS does it if I remember correctly.

Sorry for the misunderstanding here. That is the package that I give out the most. As you noted it's tailor made to patch into VS and unblock customers. Hence I just default to sending out a link to that package.

Meant the link to be a general example of how to grab packages out of our nightly feed. All of our NuGets are available there and we produce them nightly. Hence anything you depend on will be available there.

Using our alpha channel is more disturbing as it affect anyone automatically on that channel hence I asked about the quality of that nuget and if you are confident about it so we can offer it to much wider audience automatically.

The quality of our nightly feed in the master branch is very high. Subscribing to that feed and picking packages that have the version glob of 2.3.0-beta2* will keep you on master.

We strive to keep the master branch ship ready for compiler at all times. In addition to building Roslyn on every check in, nightly we build all of Visual Studio with the latest NuGet package + run their validation tests. This helps us ensure we keep ourselves clean here.

In particular now the quality is higher because we're in lock down for C# 7.1. 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment