-
Notifications
You must be signed in to change notification settings - Fork 127
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
BCEL-341: Oak class file patch #58
base: master
Are you sure you want to change the base?
Conversation
Note: All test pass with mvn clean verify |
-1 for now:
|
I will look into the check style error, and ill add a unit test. But as far as it being a version specific brute force hack is well, because this only occurs when one condition is met. But if you really want me to give an oop approach on it I will do so. |
I think we should also have a class file or jar file that we can run through other existing tests. |
Since this is just support for oak class files, do I really need to write a test for it? Can't I just use the existing test cases? |
This is an oak class file that BCEL fails to parse without my patches, I fixed the checkstyle error. But I am still not sure how I feel about over complicating things with an oop system on weather to read a u1 or u2. But if that is what you want I will implement it once I get off work 👍 |
Actually I just thought of a more stable way to implement this, I will do so when I get the chance. |
Thank you for the class file. I think we'll need the source to include it in the project repo to make sure the licensing is OK for an Apache project. |
There is no license, it's practically a hello world program. |
I realized that my idea for making it "more stable" would require a massive overhaul on the library, which doesn't seem like a good idea. I can think of a different approach if you would like, though I personally don't think its necessary. |
Hi @Hippo |
Notes
|
@@ -303,5 +304,6 @@ private void readMethods() throws IOException, ClassFormatException { | |||
private void readVersion() throws IOException, ClassFormatException { | |||
minor = dataInputStream.readUnsignedShort(); | |||
major = dataInputStream.readUnsignedShort(); | |||
isOak = major < Const.MAJOR_1_1 || (major == Const.MAJOR_1_1 && minor < 3); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the check be minor <= 3
instead? See notes.
Are you saying that 45.2 is not compatible with 45.3?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OakFile.java.zip
There is no license, it's practically a hello world program.Hi @Hippo
I'll should be able to take a look this weekend to see if a cleaner approach can be devised. In the meantime, please provide the source .java and instructions on how you created the class file if it is out of the ordinary Java tooling we are all used to. Is there an Oak/Java 1.0 compiler you used we can download to create additional files?
No I don't have an oak compiler, but you can emulate one by transforming a class file and changing its version to a proper oak version and halfing the correct data types in the code attribute.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the check be
minor <= 3
instead? See notes.
Are you saying that 45.2 is not compatible with 45.3?
No it should be minor < 3
take a look at the jvm's class parser
https://github.com/AdoptOpenJDK/openjdk-jdk8u/blob/4b69552faa8de8f7de5bed1c95c12c025702b8ee/hotspot/src/share/vm/classfile/classFileParser.cpp#L2137
Also I check major < Const.MAJOR_1_1
just in-case the major version is less than 45, which shouldn't happen, but it's there just in-case.
Any updates? |
Patches a bug when parsing oak class files (versions <= 45.2)
The code attribute's maxStack, maxLocals, codeLength is half the size (u2 -> u1, u4 -> u2)