Skip to content
This repository was archived by the owner on Jun 17, 2025. It is now read-only.

[WIP] Fix crashes when passing arrays via reflection - #191

Open
aoli-al wants to merge 1 commit into
gmu-swe:masterfrom
aoli-al:object
Open

[WIP] Fix crashes when passing arrays via reflection #191
aoli-al wants to merge 1 commit into
gmu-swe:masterfrom
aoli-al:object

Conversation

@aoli-al

@aoli-al aoli-al commented Feb 16, 2023

Copy link
Copy Markdown
Contributor

If a method signature has Object type and it receives an array via reflection, Phosphor cannot identify this and will lead to java.lang.ClassCastException.

Initially I was planning to simply expand this check for all methods

} else if (ensureObjsAreWrapped && args[i].getInternalName().equals("java/lang/Object")) {

It seems not working because it calls Phosphor inside JDK when JVM is initialized (and cause another crash). It seems that we need to maintain an allow list either:

  1. call this wrapper for specific methods
  2. do not call this wrapper for specific methods

It seems plan 2 is more robust but building that list exhaustively is hard. Please let me know what you think and I'm happy to bring a patch.

Ps. if you want to see a realistic example, the class is
https://github.com/openjdk/jdk/blob/a39cf2e3b242298fbf5fafdb8aa9b5d4562061ef/src/java.base/share/classes/sun/security/x509/BasicConstraintsExtension.java#L114

@jon-bell

Copy link
Copy Markdown
Collaborator

Yes, I've run into this one before, too :/

ensureObjsAreWrapped = true; //TODO should we always do this? just in some places? performance is what? If try everywhere, it crashes...

I haven't had a chance yet to run and step through your failing test case, but I think that for the case of reflection, this is not the right direction to be going. We should be intercepting reflective calls and ensuring that primitive arrays are represented by their corresponding wrappers.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants