close
Skip to content

Fix InvalidCastException when exception filter contains a switch expression#48260

Merged
RikkiGibson merged 6 commits into
dotnet:masterfrom
Youssef1313:patch-5
Oct 29, 2020
Merged

Fix InvalidCastException when exception filter contains a switch expression#48260
RikkiGibson merged 6 commits into
dotnet:masterfrom
Youssef1313:patch-5

Conversation

@Youssef1313
Copy link
Copy Markdown
Member

@Youssef1313 Youssef1313 commented Oct 2, 2020 •
edited
Loading

Hopefully fixes #48259

This is a very quick fix by looking into the exception and stacktrace in SharpLab:

System.InvalidCastException: Unable to cast object of type 'Microsoft.CodeAnalysis.CSharp.BoundStatementList' to type 'Microsoft.CodeAnalysis.CSharp.BoundBlock'.

   at Microsoft.CodeAnalysis.CSharp.Symbols.MethodToClassRewriter.VisitCatchBlock(BoundCatchBlock node) in /_/src/Compilers/CSharp/Portable/Lowering/MethodToClassRewriter.cs:line 130

I found a similar cast in:

roslyn/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_TryStatement.cs

Line 91 in cf55f3a

BoundBlock? rewrittenFilterPrologue = (BoundBlock?)this.Visit(node.ExceptionFilterPrologueOpt);

Can you please review if this have a similar bug?

@Youssef1313 Youssef1313 requested a review from a team as a code owner October 2, 2020 20:57
@jcouv jcouv added Area-Compilers Community The pull request was submitted by a contributor who is not a Microsoft employee. labels Oct 2, 2020
AlekseyTs
AlekseyTs approved these changes Oct 5, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@AlekseyTs AlekseyTs left a comment

Choose a reason for hiding this comment

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

LGTM (iteration 3)

@RikkiGibson
Copy link
Copy Markdown
Member

RikkiGibson commented Oct 5, 2020

@333fred @jaredpar can you confirm if this is something we want to take in 16.8?

@RikkiGibson RikkiGibson self-assigned this Oct 5, 2020
@RikkiGibson
Copy link
Copy Markdown
Member

RikkiGibson commented Oct 5, 2020

I found a similar cast in:

roslyn/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_TryStatement.cs

Line 91 in cf55f3a

BoundBlock? rewrittenFilterPrologue = (BoundBlock?)this.Visit(node.ExceptionFilterPrologueOpt);

Can you please review if this have a similar bug?

Currently I don't think it's possible to cause the compiler to crash due to this, because the exception filter prologue is only introduced by subsequent lowering passes (specifically SpillSequenceSpiller which calls BoundCatchBlock.Update().) Thus it is always null when we cast it in LocalRewriter_TryStatement.cs.

However, I think we should just do the same fix here.

@runfoapp runfoapp Bot mentioned this pull request Oct 28, 2020
Closed
@RikkiGibson RikkiGibson merged commit 02bff25 into dotnet:master Oct 29, 2020
@ghost ghost added this to the Next milestone Oct 29, 2020
@RikkiGibson
Copy link
Copy Markdown
Member

RikkiGibson commented Oct 29, 2020

Thanks @Youssef1313!

333fred added a commit to 333fred/roslyn that referenced this pull request Oct 30, 2020
@333fred
Merge remote-tracking branch 'upstream/master' into document-symbol-s…
664332f 
…ervice

* upstream/master: (131 commits)
  Fix SyntaxGenerator.InsertMembers for records (dotnet#48862)
  Register 64-bit Server GC brokered services
  Reduce length of the paths we make for temporary generated files
  Fix InvalidCastException when exception filter contains a switch expression (dotnet#48260)
  Use frozen partial semantics for commit.
  Renames
  Flip
  Simplify
  Simplify
  make things work in OOP
  add tests
  Remove
  Support add-using in Razor
  Show Analyzers that only contain source generators
  Remove unnecessary use of CODE_STYLE versioning.
  Update to roslyn-analyzers 3.3.2-beta1.20528.2
  Don't crash when adding a parameter to the end of an extension method (dotnet#49007)
  Use TextDiff service
  Continue to map push diagnostics for razor.
  Procdump in integration tests (dotnet#48981)
  ...
@Youssef1313 Youssef1313 deleted the patch-5 branch October 30, 2020 10:09
@Youssef1313 Youssef1313 mentioned this pull request Nov 4, 2020
Closed
@allisonchou allisonchou modified the milestones: Next, 16.9.P2 Nov 24, 2020
@mikadumont mikadumont mentioned this pull request Dec 10, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@RikkiGibson RikkiGibson RikkiGibson approved these changes

@AlekseyTs AlekseyTs AlekseyTs approved these changes

Assignees

@RikkiGibson RikkiGibson

Labels

Area-Compilers Community The pull request was submitted by a contributor who is not a Microsoft employee.

Projects

None yet

Milestone

16.9.P2

Development

Successfully merging this pull request may close these issues.

Switch expression in exception filter of async method crash compiler

5 participants

@Youssef1313 @RikkiGibson @AlekseyTs @jcouv @allisonchou