-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
HiddenClassDefiner incorrectly assumes Unsafe.staticFieldBase(Field) returns a real Object #1672
Comments
Hi @breun - I'm not sure I fully agree with that assessment because the code is taking the values returned by
But I agree that we're not doing that directly though because of the use of the reflective call, and it appears that Azul Prime is doing something different when reflection is involved. I believe we can simplify that call to avoid using the value from
I'll try to recreate the original issue to verify this fix. |
@mcculls I've tried building your branch locally and I believe I need to use Bazel, which I've installed, but I haven't figured out yet what command I need to run to build Guice. Can you tell me how to build Guice or point me to documentation for this? |
You can also build it with Maven - to do a quick build without tests or
javadoc use:
mvn package -DskipTests -Dmaven.javadoc.skip
I'd recommend using one of the releases before 3.9.x, for example 3.8.8, to
avoid hitting this issue (assuming you want to build it with Azul Prime)
…On Tue, 11 Apr 2023 at 14:53, Nils Breunese ***@***.***> wrote:
@mcculls <https://github.com/mcculls> I've tried building your branch
locally and I believe I need to use Bazel, which I've installed, but I
haven't figured out yet what command I need to run to build Guice. Can you
tell me how to build Guice or point me to documentation for this?
—
Reply to this email directly, view it on GitHub
<#1672 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABDTO6H3CAI5S7AR6ZD7U3XAVO5FANCNFSM6AAAAAAWST3NJE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
I was able to install Guice 5.1.1-SNAPSHOT from your branch, but the next step is building Maven 3.9.1 with this version of Guice, but I various tests fail when building Maven 3.9.1 with Guice 5.1.1-SNAPSHOT due to |
Guice master branch currently depends on Guava 31.0.1-jre whereas Maven
3.9.1 branch depends on Guava 30.1-jre
If you upgrade the Guava dependency version in Maven's top-level pom.xml to
31.0.1-jre then it should work fine:
diff --git a/pom.xml b/pom.xml
index 334c502bd..a3d6eb01a 100644
--- a/pom.xml
+++ b/pom.xml
@@ -139,3 +139,3 @@ under the License.
<guiceVersion>5.1.0</guiceVersion>
- <guavaVersion>30.1-jre</guavaVersion>
+ <guavaVersion>31.0.1-jre</guavaVersion>
<guavafailureaccessVersion>1.0.1</guavafailureaccessVersion>
|
Hi @mcculls we at Azul have a change for this issue which we prefer to the one you proposed. I believe we're waiting for Google to approve the paperwork we've submitted that's required to join the project. Is there someone who can help expedite the application? |
@azul-jf: Please open a pull request with your proposed change and we can discuss it there. The automated CLA checks will kick in when the PR is open. It looks like Azul already has a corp CLA agreement AFAICT, so so long as the author of the PR is a member of Azul's CLA, it should just work. |
* pass result of Unsafe.staticFieldBase() instead of Lookup.class to conform with specification for Unsafe.getObject() requirements. * avoid calling Unsafe.getObject() via reflection Fixes google#1672
* pass result of Unsafe.staticFieldBase() instead of Lookup.class to conform with specification for Unsafe.getObject() requirements. * avoid calling Unsafe.getObject() via reflection Fixes google#1672
* pass result of Unsafe.staticFieldBase() instead of Lookup.class to conform with specification for Unsafe.getObject() requirements. The specification requires the arguments for Unsafe.getObject() to be obtained from Unsafe.staticFieldBase() and Unsafe.staticFieldOffset(). * avoid calling Unsafe.getObject() via reflection, because passing result of Unsafe.staticFieldBase() via reflection might cause problems if it is not real java object. Fixes google#1672
…nsafe, and call getObject on it natively instead of through reflection. Calling getObject through reflection breaks Azul JVMs, because the staticFieldBase is a funny "not object", but when passed through reflection it tries to become an Object and fails. Retrieval of the unsafe is based on how other google open source projects do it, see for example [AbstractFuture](https://github.com/google/guava/blob/d4bd0c5ffa913104283e61aeb2c41bac641af042/guava/src/com/google/common/util/concurrent/AbstractFuture.java#L1340-L1365), [UnsignedBytes](https://github.com/google/guava/blob/6405852bbf453b14d097b8ec3bcae494334b357d/guava/src/com/google/common/primitives/UnsignedBytes.java#L341-L366), and [protobufs](https://github.com/protocolbuffers/protobuf/blob/520c601c99012101c816b6ccc89e8d6fc28fdbb8/java/core/src/main/java/com/google/protobuf/UnsafeUtil.java#L289-L315). Fixes #1719, and properly fixes #1672. (Note that this change also bumps the Github Action's bazel version from 4.2.2 to 6.1.2, because somewhere along the way from 4.2.2->6.1.2, Bazel fixed the `--javacopts="--release 8"` option to also allow `sun.misc.Unsafe`.) PiperOrigin-RevId: 529205332
…nsafe, and call getObject on it natively instead of through reflection. Calling getObject through reflection breaks Azul JVMs, because the staticFieldBase is a funny "not object", but when passed through reflection it tries to become an Object and fails. Retrieval of the unsafe is based on how other google open source projects do it, see for example [AbstractFuture](https://github.com/google/guava/blob/d4bd0c5ffa913104283e61aeb2c41bac641af042/guava/src/com/google/common/util/concurrent/AbstractFuture.java#L1340-L1365), [UnsignedBytes](https://github.com/google/guava/blob/6405852bbf453b14d097b8ec3bcae494334b357d/guava/src/com/google/common/primitives/UnsignedBytes.java#L341-L366), and [protobufs](https://github.com/protocolbuffers/protobuf/blob/520c601c99012101c816b6ccc89e8d6fc28fdbb8/java/core/src/main/java/com/google/protobuf/UnsafeUtil.java#L289-L315). Fixes #1719, and properly fixes #1672. (Note that this change also bumps the Github Action's bazel version from 4.2.2 to 6.1.2, because somewhere along the way from 4.2.2->6.1.2, Bazel fixed the `--javacopts="--release 8"` option to also allow `sun.misc.Unsafe`.) PiperOrigin-RevId: 529465570
…nsafe, and call getObject on it natively instead of through reflection. (Take 2, after exempting its usages internally.) Calling getObject through reflection breaks Azul JVMs, because the staticFieldBase is a funny "not object", but when passed through reflection it tries to become an Object and fails. Retrieval of the unsafe is based on how other google open source projects do it, see for example AbstractFuture, UnsignedBytes, and protobufs. Fixes #1719, and properly fixes #1672. (Note that this change also bumps the Github Action's bazel version from 4.2.2 to 6.1.2, because somewhere along the way from 4.2.2->6.1.2, Bazel fixed the --javacopts="--release 8" option to also allow sun.misc.Unsafe.) PiperOrigin-RevId: 529479291
…nsafe, and call getObject on it natively instead of through reflection. (Take 2, after exempting its usages internally.) Calling getObject through reflection breaks Azul JVMs, because the staticFieldBase is a funny "not object", but when passed through reflection it tries to become an Object and fails. Retrieval of the unsafe is based on how other google open source projects do it, see for example AbstractFuture, UnsignedBytes, and protobufs. Fixes #1719, and properly fixes #1672. (Note that this change also bumps the Github Action's bazel version from 4.2.2 to 6.1.2, because somewhere along the way from 4.2.2->6.1.2, Bazel fixed the --javacopts="--release 8" option to also allow sun.misc.Unsafe.) PiperOrigin-RevId: 529486761
The combination of Google Guice 5.1.0 and the Azul Prime JVM is causing a VM crash. I've reached out to Azul support about this and they believe that the root cause is that the class
com.google.inject.internal.aop.HiddenClassDefiner
treats aklassOop
as though it were anObject
.HiddenClassDefiner
definesTRUSTED_LOOKUP_BASE
like this:And its value is obtained like this:
So
TRUSTED_LOOKUP_BASE
, nominally a private static finalObject
, is obtained by callingUnsafe.staticFieldBase()
via reflection.Here's how it's used in Guice:
Which calls
Unsafe.getObject((Object)TRUSTED_LOOKUP_BASE, (long)TRUSTED_LOOKUP_OFFSET)
.This use of
TRUSTED_LOOKUP_BASE
ignores the admonitions ofUnsafe.staticFieldBase()
, which says the value is not guaranteed to be a realObject
:Now, a lot of reflection code is generated on the fly at runtime and inlined. I'm also told the Prime VM emits a
checkcast
bytecode, which does what it says and checks the reference to be returned bygetObject
is, in fact, anObject
, which aklassOop
isn't. Hence the attempt to throw aClassCastException
in a place where the VM shouldn't.Here is the hs_err file for the crash: hs_err_pid12.log.gz
The text was updated successfully, but these errors were encountered: