Skip to content

Fix opcode parsing, interface instanceof, and runtime bugs#159

Merged
dlunch merged 8 commits into
mainfrom
fix-found-bugs
Jun 12, 2026
Merged

Fix opcode parsing, interface instanceof, and runtime bugs#159
dlunch merged 8 commits into
mainfrom
fix-found-bugs

Conversation

@dlunch

@dlunch dlunch commented Jun 10, 2026

Copy link
Copy Markdown
Owner

Summary

Fixes 10 bugs found during a codebase-wide analysis, each with a failing test written first (TDD), plus a test-infrastructure fix.

classfile

  • invokeinterface/invokedynamic consumed only 2 of 4 operand bytes, misaligning every opcode (and branch target) after them. The existing Interface.class E2E test only passed by accident; the new parser test asserts exact opcode offsets.

jvm / jvm_rust

  • instanceof/checkcast always failed against interfaces: interface names were dropped during class loading and is_inherited_from only walked the superclass chain. ClassDefinition now exposes interface_names() and the check covers direct interfaces, loaded superinterfaces, and the superclass chain. Covered by a unit test and a new InterfaceCast E2E class.
  • putstatic stored raw stack ints into boolean/byte/char/short fields, panicking on later typed reads from Rust. It now narrows by field descriptor like putfield (shared to_field_type helper).
  • JavaLangString::to_rust_string panicked on unpaired surrogates (legal in Java strings); now uses from_utf16_lossy.

java_runtime

  • ByteArrayInputStream.mark() stored readlimit instead of the current position, corrupting reset().
  • InputStream.skip() always returned n and allocated an n-sized scratch array; now loops with a bounded buffer and returns the actual skipped count.
  • Integer.parseInt() panicked on invalid input; now throws NumberFormatException.
  • String.substring(begin, end) underflowed on invalid ranges; now throws StringIndexOutOfBoundsException.
  • ISO-8859-1 encoding silently truncated chars > 0xFF; now maps them to ? like the JDK.
  • Vector.firstElement() returned null on an empty vector; now throws NoSuchElementException.

New runtime classes: NumberFormatException, StringIndexOutOfBoundsException, NoSuchElementException.

test_utils

  • DummyFile deep-copied its state on clone, but Runtime::get_file hands out cloned handles — so file position never persisted across reads. It now shares position via Arc<Mutex> like the production File impls, and read honors seek.

Test plan

  • Each fix has a test that failed before the fix and passes after.
  • cargo test --workspace: all 25 targets pass.
  • cargo clippy --workspace --all-targets: no new warnings (one pre-existing approx_constant error at test_string.rs:309 from Implement WIPI String API #155 remains).

dlunch added 3 commits June 10, 2026 09:24
- invokeinterface/invokedynamic now consume all four operand bytes
- ClassDefinition exposes interface names; is_inherited_from checks interfaces
- putstatic narrows int stack values to the field type like putfield
- JavaLangString::to_rust_string no longer panics on unpaired surrogates
- ByteArrayInputStream.mark saves position instead of readlimit
- InputStream.skip returns actual skipped bytes with bounded buffer
- Integer.parseInt throws NumberFormatException on invalid input
- String.substring validates range, ISO-8859-1 encoding maps unmappable chars to '?'
- Vector.firstElement throws NoSuchElementException on empty vector
Runtime::get_file hands out cloned handles, so reads through fresh
handles never advanced the file position. Share pos like the
production File impls and make read honor it instead of consuming
the buffer.
Copilot AI review requested due to automatic review settings June 10, 2026 08:15

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3cd2eaf407

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread java_runtime/src/classes/java/lang/string.rs Outdated
@codecov

codecov Bot commented Jun 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.37380% with 27 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.53%. Comparing base (f189376) to head (174373e).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...e/src/classes/java/lang/number_format_exception.rs 64.70% 6 Missing ⚠️
.../java/lang/string_index_out_of_bounds_exception.rs 66.66% 6 Missing ⚠️
java_runtime/src/classes/java/lang/string.rs 77.77% 4 Missing ⚠️
...src/classes/java/util/no_such_element_exception.rs 73.33% 4 Missing ⚠️
classfile/tests/test.rs 92.30% 1 Missing ⚠️
java_runtime/src/classes/java/io/input_stream.rs 90.90% 1 Missing ⚠️
...va_runtime/tests/classes/java/lang/test_integer.rs 91.66% 1 Missing ⚠️
...ava_runtime/tests/classes/java/lang/test_string.rs 97.82% 1 Missing ⚠️
...ava_runtime/tests/classes/java/util/test_vector.rs 96.00% 1 Missing ⚠️
jvm_rust/src/class_definition.rs 90.90% 1 Missing ⚠️
... and 1 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #159      +/-   ##
==========================================
+ Coverage   79.46%   80.53%   +1.07%     
==========================================
  Files         178      183       +5     
  Lines        7741     8018     +277     
==========================================
+ Hits         6151     6457     +306     
+ Misses       1590     1561      -29     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes multiple JVM/classfile parsing and Java runtime correctness bugs uncovered via codebase-wide analysis, and adds regression tests to prevent reintroductions.

Changes:

  • Fix classfile opcode parsing alignment for invokeinterface / invokedynamic and add targeted parser tests.
  • Fix JVM runtime type checks for instanceof / checkcast with interfaces and correct putstatic narrowing for small integral field types.
  • Fix several java_runtime behavioral bugs (exceptions, substring, charset encoding, skip, mark/reset, Vector.firstElement) and improve test utilities’ file-handle semantics.

Reviewed changes

Copilot reviewed 34 out of 40 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tests/test_putstatic.rs Adds regression test ensuring putstatic narrows stack ints to the declared field type.
tests/test_helper/mod.rs Removes a crate-level dead_code allow in test helper module.
test_utils/src/lib.rs Fixes DummyFile cloning semantics by sharing position across clones and honoring seek.
test_data/unit/StaticFlag.java Adds unit-test fixture class for static small-type fields.
test_data/InterfaceCast.txt Adds expected output for new interface cast E2E test.
test_data/InterfaceCast.java Adds E2E program covering instanceof/interface cast behavior.
jvm/tests/test_string.rs Adds regression test for unpaired surrogate handling in string conversion.
jvm/tests/test_is_instance.rs Adds regression test for is_instance across interface + superclass chains.
jvm/src/runtime/java_lang_string.rs Makes Java→Rust UTF-16 conversion lossy to avoid panics on invalid UTF-16.
jvm/src/jvm.rs Extends inheritance checks to include direct interfaces and loaded superinterfaces.
jvm/src/class_definition.rs Extends ClassDefinition API with interface_names() (default empty).
jvm_rust/src/interpreter.rs Shares putfield/putstatic narrowing logic via to_field_type helper.
jvm_rust/src/class_definition.rs Preserves interface names when building class definitions from protos/classfiles.
java_runtime/tests/classes/java/util/test_vector.rs Adds tests for Vector.firstElement() empty behavior and indexOf(null).
java_runtime/tests/classes/java/lang/test_string.rs Adjusts floating test constant and adds tests for invalid substring + ISO-8859-1 unmappable chars.
java_runtime/tests/classes/java/lang/test_integer.rs Adds test asserting parseInt throws NumberFormatException on invalid input.
java_runtime/tests/classes/java/io/test_file_input_stream.rs Adds tests ensuring sequential reads advance position and skip returns actual skipped count.
java_runtime/tests/classes/java/io/test_data_input_stream.rs Adds test covering InputStream.skip() behavior via DataInputStream.
java_runtime/tests/classes/java/io/test_byte_array_input_stream.rs Adds test for ByteArrayInputStream.mark/reset correctness.
java_runtime/tests/classes/java/io/mod.rs Registers new ByteArrayInputStream test module.
java_runtime/src/loader.rs Registers newly added runtime exception classes in the runtime loader.
java_runtime/src/classes/java/util/vector.rs Changes Vector.firstElement() empty behavior to throw NoSuchElementException.
java_runtime/src/classes/java/util/no_such_element_exception.rs Implements java/util/NoSuchElementException runtime class.
java_runtime/src/classes/java/util.rs Wires NoSuchElementException into the java.util module exports.
java_runtime/src/classes/java/lang/string.rs Adds substring range validation + exception, and fixes ISO-8859-1 encoding for >0xFF chars.
java_runtime/src/classes/java/lang/string_index_out_of_bounds_exception.rs Implements java/lang/StringIndexOutOfBoundsException runtime class.
java_runtime/src/classes/java/lang/number_format_exception.rs Implements java/lang/NumberFormatException runtime class.
java_runtime/src/classes/java/lang/integer.rs Makes Integer.parseInt throw NumberFormatException instead of panicking.
java_runtime/src/classes/java/lang.rs Wires new java.lang exception classes into the module exports.
java_runtime/src/classes/java/io/input_stream.rs Fixes InputStream.skip() to bound allocations and return actual skipped bytes.
java_runtime/src/classes/java/io/byte_array_input_stream.rs Fixes ByteArrayInputStream.mark() to store current position (not readlimit).
classfile/tests/test.rs Adds opcode-offset regression test for invokeinterface parsing alignment.
classfile/src/opcode.rs Fixes parsing to consume all operand bytes for invokedynamic/invokeinterface; adds unit tests.
Cargo.toml Adds workspace test_utils as a dev-dependency for integration tests.
Cargo.lock Updates lockfile for added dev-dependency.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread java_runtime/src/classes/java/lang/string.rs Outdated
Comment thread test_utils/src/lib.rs Outdated
dlunch added 4 commits June 10, 2026 17:19
- substring uses UTF-16 code unit indices like Java String.length()
- US-ASCII encoding replaces non-ASCII chars instead of acting as latin-1
- clamp DummyFile position in u64 before casting to usize
Resolving interfaces alongside the superclass guarantees the whole
interface hierarchy is registered, so is_inherited_from can look up
transitive superinterfaces unconditionally.
@dlunch dlunch enabled auto-merge (squash) June 12, 2026 10:32
@dlunch dlunch disabled auto-merge June 12, 2026 10:36
@dlunch dlunch merged commit ebd9c03 into main Jun 12, 2026
10 checks passed
@dlunch dlunch deleted the fix-found-bugs branch June 12, 2026 10:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants