You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
HiddenClassDefiner incorrectly assumes Unsafe.staticFieldBase(Field) returns a real Object #1672
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 a klassOop as though it were an Object.
HiddenClassDefiner defines TRUSTED_LOOKUP_BASE like this:
Which calls Unsafe.getObject((Object)TRUSTED_LOOKUP_BASE, (long)TRUSTED_LOOKUP_OFFSET).
This use of TRUSTED_LOOKUP_BASE ignores the admonitions of Unsafe.staticFieldBase(), which says the value is not guaranteed to be a real Object:
/**
* Reports the location of a given static field, in conjunction with {@link
* #staticFieldOffset}.
* <p>Fetch the base "Object", if any, with which static fields of the
* given class can be accessed via methods like {@link #getInt(Object,
* long)}. This value may be null. This value may refer to an object
* which is a "cookie", not guaranteed to be a real Object, and it should
* not be used in any way except as argument to the get and put routines in
* this class.
*/
@ForceInline
public Object staticFieldBase(Field f) {
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 by getObject is, in fact, an Object, which a klassOop isn't. Hence the attempt to throw a ClassCastException in a place where the VM shouldn't.
Hi @breun - I'm not sure I fully agree with that assessment because the code is taking the values returned by staticFieldBase and staticFieldOffset and passing them into unsafe.getObject which is allowed (and intended) according to the Unsafe javadoc:
* <li>The offset and object reference {@code o} (either null or
* non-null) were both obtained via {@link #staticFieldOffset}
* and {@link #staticFieldBase} (respectively) from the
* reflective {@link Field} representation of some Java field.
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 staticFieldBase because we know that field is a member of Lookup.class - according to the javadoc we can use Lookup.class directly, which should hopefully avoid the issue on Azul Prime:
@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 java.lang.NoSuchMethodError: com.google.common.collect.ImmutableMap$Builder.buildOrThrow()Lcom/google/common/collect/ImmutableMap;. Any ideas? I can build Maven 3.9.1 with Guice 5.1.0 without problems.
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.
Did this change ever get merged? I still see the warning with Maven 3.9.10, which uses the older 5.1.0, but I built a 3.9.11-SNAPSHOT using 6.0.0 of Guice, and the issues remain.
Reacted by theotherp, Travis Furrer, Dimage Sapelkin and dclements
Activity
mcculls commentedon Apr 5, 2023
Hi @breun - I'm not sure I fully agree with that assessment because the code is taking the values returned by
staticFieldBaseandstaticFieldOffsetand passing them intounsafe.getObjectwhich is allowed (and intended) according to theUnsafejavadoc: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
staticFieldBasebecause we know that field is a member ofLookup.class- according to the javadoc we can useLookup.classdirectly, which should hopefully avoid the issue on Azul Prime:I'll try to recreate the original issue to verify this fix.
mcculls commentedon Apr 7, 2023
@breun PR #1673 has a potential fix - could you build this locally and try it out with your Azul Prime setup?
breun commentedon Apr 11, 2023
@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?
mcculls commentedon Apr 11, 2023
breun commentedon Apr 12, 2023
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
java.lang.NoSuchMethodError: com.google.common.collect.ImmutableMap$Builder.buildOrThrow()Lcom/google/common/collect/ImmutableMap;. Any ideas? I can build Maven 3.9.1 with Guice 5.1.0 without problems.mcculls commentedon Apr 12, 2023
Use Lookup.class as the base when retrieving trusted lookup because w…
Use Lookup.class as the base when retrieving trusted lookup because w…
azul-jf commentedon Apr 26, 2023
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?
sameb commentedon Apr 26, 2023
@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.
Use Unsafe API in more correct way
Use Unsafe API in more correct way
Use Unsafe API in more correct way
Rework sun.misc.Unsafe usages so that we directly refer to sun.misc.U…
Rework sun.misc.Unsafe usages so that we directly refer to sun.misc.U…
Rework sun.misc.Unsafe usages so that we directly refer to sun.misc.U…
Rework sun.misc.Unsafe usages so that we directly refer to sun.misc.U…
talios commentedon Jul 5, 2025
Did this change ever get merged? I still see the warning with Maven 3.9.10, which uses the older 5.1.0, but I built a 3.9.11-SNAPSHOT using 6.0.0 of Guice, and the issues remain.