Skip to content

Bug 217681: compiler should not crash on public+private metadata#11547

Merged
jcouv merged 3 commits intodotnet:masterfrom
jcouv:bug-217681
May 25, 2016
Merged

Bug 217681: compiler should not crash on public+private metadata#11547
jcouv merged 3 commits intodotnet:masterfrom
jcouv:bug-217681

Conversation

@jcouv
Copy link
Copy Markdown
Member

@jcouv jcouv commented May 25, 2016

This fixes this Watson-reported crash for update 3.

The crash occurs when accessibility flags on a method are set to 7 (which is all the flags, which means both public and private).

This problem existed on the VB side for a long time. On the C# side, before this change, those switch statements handled the default case with a debug assert and defaulting to "private".

On the C# side:

  • native compiler treated those flags as "private" and reports errors:
consumer.cs(5,15): error CS1061: 'C' does not contain a definition for 'M' and no extension method 'M' accepting a first argument of type 'C' could be found (are you missing a using directive or an assembly reference?)
consumer.cs(6,40): error CS1061: 'C' does not contain a definition for 'F' and no extension method 'F' accepting a first argument of type 'C' could be found (are you missing a using directive or an assembly reference?)
consumer.cs(7,13): error CS0426: The type name 'C2' does not exist in the type 'C'
  • compiler 1.2 compiler failed silently (no executable produced)
  • future branch crashes
  • after fix, errors are reported:
consumer.cs(5,15): error CS0122: 'C.M()' is inaccessible due to its protection level
consumer.cs(6,40): error CS0122: 'C.F' is inaccessible due to its protection level
consumer.cs(7,13): error CS0426: The type name 'C2' does not exist in the type 'C'

On the VB side:

  • native compiler successfully compiles the method and field case (those only yield runtime error System.TypeLoadException: Invalid Field Access Flags) and reported an accessibility error on the nested type.
  • compiler 1.2 generated errors:
error BC30390: 'C.Private Overloads Sub M()' is not accessible in this context because it is 'Private'.
error BC30389: 'C.F' is not accessible in this context because it is 'Private'.
error BC30389: 'C.C2' is not accessible in this context because it is 'Protected Friend'.
error BC30390: 'C2.Private Overloads Sub M2()' is not accessible in this context because it is 'Private'.
  • future branch crashes
  • after fix, the same errors as 1.2 are generated again. I will notify the compat council of this breaking with the native compiler.

@jcouv jcouv added Area-Compilers Tenet-Reliability Customer telemetry indicates that the product is failing in a crash/hang/dataloss manner. labels May 25, 2016
@jcouv jcouv self-assigned this May 25, 2016
@jcouv
Copy link
Copy Markdown
Member Author

jcouv commented May 25, 2016

CC @dotnet/roslyn-compiler for review.

@jcouv
Copy link
Copy Markdown
Member Author

jcouv commented May 25, 2016

@MattGertz Can you approve for update 3?
The compat council approved the small compat break.

@jcouv
Copy link
Copy Markdown
Member Author

jcouv commented May 25, 2016

@dotnet/roslyn-compiler Please review for update 3.

@MattGertz
Copy link
Copy Markdown
Contributor

Approved pending CR signoffs.

{
new C().M();
System.Console.WriteLine(new C().F);
new C.C2().M2();
Copy link
Copy Markdown
Contributor

@AlekseyTs AlekseyTs May 25, 2016

Choose a reason for hiding this comment

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

I assume () after C2 are not intentional. Never mind.

@AlekseyTs
Copy link
Copy Markdown
Contributor

:shipit:

@AlekseyTs
Copy link
Copy Markdown
Contributor

LGTM

@jaredpar
Copy link
Copy Markdown
Member

👍

@jcouv jcouv merged commit b35416a into dotnet:master May 25, 2016
@jcouv jcouv added the Tenet-Compatibility Violation of forwards/backwards compatibility in a design-time piece. label Jun 21, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Approved to merge Area-Compilers cla-already-signed Tenet-Compatibility Violation of forwards/backwards compatibility in a design-time piece. Tenet-Reliability Customer telemetry indicates that the product is failing in a crash/hang/dataloss manner.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants