All 3 compilers PARTIAL: missing static/<clinit> checks; L2 also breaks LONG/DOUBLE returns#571
Open
opencode-agent[bot] wants to merge 1 commit into
Open
All 3 compilers PARTIAL: missing static/<clinit> checks; L2 also breaks LONG/DOUBLE returns#571opencode-agent[bot] wants to merge 1 commit into
opencode-agent[bot] wants to merge 1 commit into
Conversation
…ks LONG/DOUBLE returns Co-authored-by: LSantha <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Spec compliance report
Details
l1a Implementation
core/src/core/org/jnode/vm/x86/compiler/l1a/X86BytecodeVisitor.java:2786, plusX86CompilerHelper.java:276,X86StackFrame.java:221VmStaticMethodwithout checking if resolved method is actually static. Spec §invokestatic Linking Exceptions: "if the resolved method is an instance method, the invokestatic instruction throws an IncompatibleClassChangeError". Current code would throw ClassCastException instead.method.isInitializer().vstack.push()flushes vstack,dropParameters(method, false)pops nargs arguments using category-aware pops,helper.invokeJavaMethod()callspushReturnValue()which correctly handles VOID (no push), category-1 (EAX), and category-2 (EAX:EDX for LONG/DOUBLE).X86StackFrame.emitTrailer()→helper.writeClassInitialize(method)at line 221.emitSynchronizationCode()which pushes declaring class and calls monitorEnter.l1b Implementation
core/src/core/org/jnode/vm/x86/compiler/l1b/X86BytecodeVisitor.java:3462, plus shared helper/stackframeVmStaticMethodwithout IncompatibleClassChangeError check.l2 Implementation
core/src/core/org/jnode/vm/compiler/ir/IRGenerator.java:1140(IR generation)core/src/core/org/jnode/vm/x86/compiler/l2/GenericX86CodeGenerator.java:5640(StaticCallAssignQuad),:5666(StaticCallQuad)core/src/core/org/jnode/vm/x86/compiler/l2/X86StackFrame.java:221(callee init)VmStaticMethodwithout checking for instance method → should throw IncompatibleClassChangeError.generateCodeFor(StaticCallAssignQuad)only moves EAX to destination. For LONG/DOUBLE returns (category 2), spec requires both EAX and EDX (32-bit) or RAX (64-bit) to be preserved. Compare withVarReturnQuadhandler (lines 376-383) which correctly handles LONG/DOUBLE by moving both registers.visit_invokestatic(correctly inherits no-op from BytecodeVisitorSupport), so invokestatic is considered supported.stackOffset -= stackChangewherestackChange = getCategory(jvmType)), creates appropriate quad, and adjusts stackOffset for return type (line 1165:stackOffset += typeSizeInfo.getStackSlots(returnType)).X86StackFrame.emitTrailer()line 221.writeParameters()(line 5728) correctly handles LONG/DOUBLE by pushing two stack slots (lines 5742-5744).JVM Spec References (Java SE 6)
invokestatic indexbyte1 indexbyte2(opcode 0xb8)..., [arg1, [arg2 ...]] → ...(void) or..., result(non-void)Summary of Spec Deviations
All three compilers lack the required linking-time check that the resolved method is static (not instance) and not . L2 additionally has incomplete codegen for LONG/DOUBLE return values from static calls.
Closes #408
opencode session | github run