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

Inlining doesn't respect JPMS access restrictions #11812

Open
szeiger opened this issue Nov 27, 2019 · 5 comments
Open

Inlining doesn't respect JPMS access restrictions #11812

szeiger opened this issue Nov 27, 2019 · 5 comments

Comments

@szeiger
Copy link

szeiger commented Nov 27, 2019

The Scala optimizer respects Java access modifiers so it doesn't inline any code that needs to call other code which is not accessible at the call site. This is not the case for JPMS restrictions. For example, running on OpenJDK 11 with -opt:l:inline -opt-inline-from:**:

Welcome to Scala 2.13.1 (OpenJDK 64-Bit Server VM, Java 11.0.5).
Type in expressions for evaluation. Or try :help.

scala> "".toDouble
java.lang.IllegalAccessError: class  (in unnamed module @0x69b88f85) cannot access class jdk.internal.math.FloatingDecimal (in module java.base) because module java.base does not export jdk.internal.math to unnamed module @0x69b88f85
  ... 36 elided

This happens because the String:toDouble implementation from the JDK gets inlined into user code even though it cannot access jdk.internal.math.FloatingDecimal from outside the JDK.

(It's probably a bad idea to inline from the JDK in the first place but the problem will become more pronounced as more and more libraries make use of Java Modules, too.)

@SethTisue
Copy link
Member

@lrytz want to milestone this?

@lrytz
Copy link
Member

lrytz commented Dec 3, 2019

We don't have the infrastructure in place to check module access restrictions, so this is on hold until then.

$> java -version
openjdk version "11.0.4" 2019-07-16
OpenJDK Runtime Environment AdoptOpenJDK (build 11.0.4+11)
OpenJDK 64-Bit Server VM AdoptOpenJDK (build 11.0.4+11, mixed mode)
$> scala
Welcome to Scala 2.13.1 (OpenJDK 64-Bit Server VM, Java 11.0.4).
Type in expressions for evaluation. Or try :help.

scala> jdk.internal.math.FloatingDecimal.parseDouble("1")
java.lang.IllegalAccessError: class  (in unnamed module @0x74fab04a) cannot access class jdk.internal.math.FloatingDecimal (in module java.base) because module java.base does not export jdk.internal.math to unnamed module @0x74fab04a
  ... 28 elided

@SethTisue SethTisue added this to the Backlog milestone Dec 4, 2019
@lrytz
Copy link
Member

lrytz commented Dec 9, 2019

(Related to scala/scala-dev#529)

@szeiger
Copy link
Author

szeiger commented Dec 9, 2019

Another angle when inlining from the JDK: There should probably be a default exclusion list for classes with intrinsics. You don't want to accidentally inline from an intrinsic method that could be replaced by HotSpot with a native machine instruction. The resulting code will still work but it will run much slower.

@lrytz
Copy link
Member

lrytz commented Dec 9, 2019

Good point; how can we find out what parts of the JDK the VMs intrinsify?

szeiger added a commit to szeiger/scala that referenced this issue Dec 12, 2019
szeiger added a commit to szeiger/scala that referenced this issue Feb 17, 2020
szeiger added a commit to szeiger/scala that referenced this issue Mar 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants