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

Don't generate a Select for a TermRef with NoPrefix #16754

Merged
merged 1 commit into from
Jan 24, 2023

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Jan 23, 2023

In TreeTypeMap, don't generate a Select node if the new type of the tree is a TermRef with NoPrefix. Generate an Ident node instead.

Fixes #16740

In TreeTypeMap, don't generate a Select node if the new type of
the tree is a TermRef with NoPrefix. Generate an Ident node instead.

Fixes scala#16740
@odersky
Copy link
Contributor Author

odersky commented Jan 24, 2023

How I fixed it:

It took quite a long search to find the culprit. I first compiled with -verbose on and saw that the program ran out of memory in erasure. I then turned all tracing on in erasure and found that it happened while trying to generate a cast. Adding some instrumentation revealed that it was inserting a cast to type T, but then deciding that the type of the cast still did not match T so it inserted another task, ad infinitum. Then I looked at the T, which was very strange: a TypeRef with a method as symbol. That clearly should not happen. I instrumented the creation site for TypeRefs and found that the TypeRef was first created in the cast logic, as the type of a tree that was created earlier. Compiling with -Yshow-tree-ids and -Ydebug-tree-with-id revealed that the tree got created in Inliner as part of logic to deal with opaque types.

Aside: I could have avoided all of this lengthy hunt if I had compiled with -Ycheck:all; that would immediately blow up after inliner. But I did not think of that, so had to do it the hard way.

Anyway, I found out that the bad tree was created in a TreeTypeMap mapOpaques called from Inliner. What happened was that we had a Select node that got a new type, which was a TermRef to a local variable with NoPrefix as the prefix. In that case the Select makes no sense anymore since there is nothing to select from! But TreeTypeMap did not handle this case. It did handle the reverse case, though, where an Ident node gets a new type that needs a Select. So it was just a matter of adding the missing case.

This illustrates some vulnerabilities that we knew before but could not fix yet. There's a danger to having two different representation of trees and types that sometimes describe the same information. And there's also a danger to having Ident and Select nodes that are very close to each other. Problems happen when there are subtle mismatches in these representations.

@odersky odersky merged commit 43d0ec4 into scala:main Jan 24, 2023
@odersky odersky deleted the fix-16740 branch January 24, 2023 12:53
@Kordyjan Kordyjan added this to the 3.3.1 milestone Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Out of memory on opaque type usage in enclosed class
4 participants