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

[GR-53803] Only allow Unsafe.allocateInstance for types registered explicitly in the configuration. #8869

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

graalvmbot
Copy link
Collaborator

@graalvmbot graalvmbot commented May 1, 2024

This PR tightens the rule Unsafe.allocateInstance in the strict reflection configuration mode (when ThrowMissingRegistrationErrors is enabled): now only types explicitly registered for Unsafe allocation can be allocated that way, i.e., without running a constructor.

The reflection configuration files always had the unsafeAllocated property for types, but we were not enforcing that. Instead, it was possible to allocate all types that the static analysis identified as instantiated. Since there was not a programmatic way to register a type in a Feature, this PR adds registerAsUnsafeAllocated to Feature.BeforeAnalysisAccess.

The JNI method AllocObject does the same as Unsafe.allocateInstance, and is actually implemented exactly like that in SVM. While the preferred way to allocate new instances is to use the JNI function that allocates a new instance and invokes the constructor in the same function, it is also possible to use AllocObject first to allocate an uninitizalized instance, and then via a separate JNI function invoke a constructor on this already existing instance. Since JNI usage is rare, all types that have a constructor registered for JNI are automatically also registered for unsafe allocation. In addition, AllocObject is now tracked by the tracing agent so that such types have the unsafeAllocated property set.

Why tighten the rules? The static analysis can find out which instance fields are never null, and remove unnecessary null checks. But such an optimization is only safe when we know that types are never instantiated without running a constructor, in which case all fields always remain uninitialized. With this new enforcement, the few types explicitly registered for unsafe allocation can easily be excluded from the optimization.

@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label May 1, 2024
@vvlevchenko
Copy link

vvlevchenko commented May 2, 2024

Is it possible to introduce flag that preserve old behavior for allocation objects via Unsafe at least in debug mode? We use such kind allocation for expression evaluation in plugin for graalvm in intelij ide.

@graalvmbot graalvmbot force-pushed the cwi/GR-51974-unsafe-allocation branch from ffb2ce3 to 1b2a31d Compare May 2, 2024 22:00
@graalvmbot graalvmbot changed the title Only allow Unsafe allocation for types registered explicitly in the configuration [GR-53803] Only allow Unsafe.allocateInstance for types registered explicitly in the configuration. May 2, 2024
@graalvmbot graalvmbot changed the title [GR-53803] Only allow Unsafe.allocateInstance for types registered explicitly in the configuration. [GR-53803] Only allow Unsafe.allocateInstance for types registered explicitly in the configuration. May 2, 2024
@graalvmbot graalvmbot force-pushed the cwi/GR-51974-unsafe-allocation branch 2 times, most recently from b7256b4 to 9ca8bdb Compare May 2, 2024 23:30
@graalvmbot graalvmbot force-pushed the cwi/GR-51974-unsafe-allocation branch 2 times, most recently from c3ee3cd to 1d0e620 Compare May 10, 2024 23:51
@graalvmbot graalvmbot force-pushed the cwi/GR-51974-unsafe-allocation branch from 1d0e620 to 6a20275 Compare May 15, 2024 21:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OCA Verified All contributors have signed the Oracle Contributor Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants