Harden JVM runtime correctness (#180)

* Harden JVM runtime correctness

* Address classfile review findings

* Move class initialization tests to Java fixture

* Separate classfile validation from JVM verification

* Remove ClassFileError re-export

Author: dlunch

AGENTS.md

@@ -22,3 +22,8 @@
 - `classfile/` - Class file parser
 - `java_class_proto/` - Java class prototypes
 - `test_utils/` - Shared test utilities
+
+## Testing Boundaries
+- Keep `java_runtime/tests/classes` limited to Java standard library class and API behavior.
+- Test JVM and interpreter semantics, including class initialization, bytecode execution, and monitor behavior, with compiled Java fixtures under `test_data/src` and expected output under `test_data`, executed by `tests/test_class.rs`.
+- Do not place JVM core behavior tests in the `java_runtime` standard library test tree.

Cargo.lock

@@ -42,18 +42,20 @@ async-trait = { workspace = true }
 bytemuck = { workspace = true }
 
 anyhow = { workspace = true }
+tracing = { workspace = true }
 tracing-subscriber = { version = "^0.3", features = ["env-filter"] }
 
 jvm = { workspace = true }
 jvm_rust = { workspace = true }
+classfile = { workspace = true }
 java_class_proto = { workspace = true }
 java_runtime = { workspace = true }
 
 [dev-dependencies]
 test_utils = { workspace = true }
 
 [target.'cfg(not(target_arch = "wasm32"))'.dependencies]
-tokio = { workspace = true, features = ["rt-multi-thread"] }
+tokio = { workspace = true, features = ["rt-multi-thread", "time"] }
 
 [target.'cfg(target_arch = "wasm32")'.dependencies]
-tokio = { workspace = true, features = ["rt"] }
+tokio = { workspace = true, features = ["rt", "time"] }

Cargo.toml

@@ -19,20 +19,20 @@ pub struct CodeAttributeExceptionTable {
 
 impl CodeAttributeExceptionTable {
     pub fn parse<'a>(data: &'a [u8], constant_pool: &BTreeMap<u16, ConstantPoolItem>) -> IResult<&'a [u8], Self> {
-        map((be_u16, be_u16, be_u16, be_u16), |(start_pc, end_pc, handler_pc, catch_type)| {
+        map_res((be_u16, be_u16, be_u16, be_u16), |(start_pc, end_pc, handler_pc, catch_type)| {
             let catch_type = if catch_type != 0 {
-                let index = constant_pool.get(&catch_type).unwrap().class_name_index();
-                Some(constant_pool.get(&index).unwrap().utf8())
+                let index = constant_pool.get(&catch_type).and_then(ConstantPoolItem::class_name_index).ok_or(())?;
+                Some(constant_pool.get(&index).and_then(ConstantPoolItem::utf8).ok_or(())?)
             } else {
                 None
             };
 
-            Self {
+            Ok::<_, ()>(Self {
                 start_pc,
                 end_pc,
                 handler_pc,
                 catch_type,
-            }
+            })
         })
         .parse(data)
     }
@@ -52,7 +52,7 @@ impl AttributeInfoCode {
             (
                 be_u16,
                 be_u16,
-                map(flat_map(be_u32, take), |x: &[u8]| Self::parse_code(x, constant_pool)),
+                map_res(flat_map(be_u32, take), |x: &[u8]| Self::parse_code(x, constant_pool)),
                 length_count(be_u16, |x| CodeAttributeExceptionTable::parse(x, constant_pool)),
                 length_count(be_u16, |x| AttributeInfo::parse(x, constant_pool)),
             ),
@@ -67,22 +67,21 @@ impl AttributeInfoCode {
         .parse(data)
     }
 
-    fn parse_code(code: &[u8], constant_pool: &BTreeMap<u16, ConstantPoolItem>) -> BTreeMap<u32, Opcode> {
+    fn parse_code(code: &[u8], constant_pool: &BTreeMap<u16, ConstantPoolItem>) -> Result<BTreeMap<u32, Opcode>, ()> {
         let mut result = BTreeMap::new();
 
         let mut data = code;
-        loop {
+        while !data.is_empty() {
             let offset = unsafe { data.as_ptr().offset_from(code.as_ptr()) } as usize;
-            if let Ok((remaining, opcode)) = Opcode::parse(data, offset, constant_pool) {
-                result.insert(offset as _, opcode);
-
-                data = remaining;
-            } else {
-                break;
+            let (remaining, opcode) = Opcode::parse(data, offset, constant_pool).map_err(|_| ())?;
+            if remaining.len() >= data.len() {
+                return Err(());
             }
+            result.insert(offset as _, opcode);
+            data = remaining;
         }
 
-        result
+        Ok(result)
     }
 }
 
@@ -114,8 +113,8 @@ impl LocalVariableTableEntry {
             (
                 be_u16,
                 be_u16,
-                map(be_u16, |x| constant_pool.get(&x).unwrap().utf8()),
-                map(be_u16, |x| constant_pool.get(&x).unwrap().utf8()),
+                map_res(be_u16, |x| constant_pool.get(&x).and_then(ConstantPoolItem::utf8).ok_or(())),
+                map_res(be_u16, |x| constant_pool.get(&x).and_then(ConstantPoolItem::utf8).ok_or(())),
                 be_u16,
             ),
             |(start_pc, length, name, descriptor, index)| Self {
@@ -152,7 +151,10 @@ pub enum AttributeInfo {
 impl AttributeInfo {
     pub fn parse<'a>(data: &'a [u8], constant_pool: &BTreeMap<u16, ConstantPoolItem>) -> IResult<&'a [u8], Self> {
         map_res(
-            (map(be_u16, |x| constant_pool.get(&x).unwrap().utf8()), flat_map(be_u32, take)),
+            (
+                map_res(be_u16, |x| constant_pool.get(&x).and_then(ConstantPoolItem::utf8).ok_or(())),
+                flat_map(be_u32, take),
+            ),
             |(name, info): (_, &[u8])| {
                 Ok::<_, nom::Err<_>>(match name.as_str() {
                     "ConstantValue" => AttributeInfo::ConstantValue(Self::parse_constant_value(info, constant_pool)?.1),
@@ -180,11 +182,11 @@ impl AttributeInfo {
     }
 
     fn parse_source_file<'a>(data: &'a [u8], constant_pool: &BTreeMap<u16, ConstantPoolItem>) -> IResult<&'a [u8], Arc<String>> {
-        map(be_u16, |x| constant_pool.get(&x).unwrap().utf8()).parse(data)
+        map_res(be_u16, |x| constant_pool.get(&x).and_then(ConstantPoolItem::utf8).ok_or(())).parse(data)
     }
 
     fn parse_constant_value<'a>(data: &'a [u8], constant_pool: &BTreeMap<u16, ConstantPoolItem>) -> IResult<&'a [u8], ConstantPoolReference> {
-        map(be_u16, |x| ConstantPoolReference::from_constant_pool(constant_pool, x as _)).parse(data)
+        map_res(be_u16, |x| ConstantPoolReference::from_constant_pool(constant_pool, x).ok_or(())).parse(data)
     }
 
     fn parse_local_variable_table<'a>(

classfile/src/attribute.rs

@@ -2,27 +2,46 @@ use alloc::{collections::BTreeMap, string::String, sync::Arc, vec::Vec};
 
 use nom::{
     IResult, Parser,
+    error::{Error, ErrorKind},
     multi::length_count,
     number::complete::{be_u16, be_u32},
 };
 
 use java_constants::ClassAccessFlags;
 
-use crate::{attribute::AttributeInfo, constant_pool::ConstantPoolItem, field::FieldInfo, interface::parse_interface, method::MethodInfo};
+use crate::{
+    ClassFileError, attribute::AttributeInfo, constant_pool::ConstantPoolItem, field::FieldInfo, interface::parse_interface, method::MethodInfo,
+    validation::validate_class,
+};
 
 fn parse_this_class<'a>(data: &'a [u8], constant_pool: &BTreeMap<u16, ConstantPoolItem>) -> IResult<&'a [u8], Arc<String>> {
     let (data, this_class) = be_u16(data)?;
-    let class_name_index = constant_pool.get(&this_class).unwrap().class_name_index();
+    let class_name_index = constant_pool
+        .get(&this_class)
+        .and_then(ConstantPoolItem::class_name_index)
+        .ok_or_else(|| nom::Err::Error(Error::new(data, ErrorKind::Verify)))?;
+    let class_name = constant_pool
+        .get(&class_name_index)
+        .and_then(ConstantPoolItem::utf8)
+        .ok_or_else(|| nom::Err::Error(Error::new(data, ErrorKind::Verify)))?;
 
-    Ok((data, constant_pool.get(&class_name_index).unwrap().utf8()))
+    Ok((data, class_name))
 }
 
 fn parse_super_class<'a>(data: &'a [u8], constant_pool: &BTreeMap<u16, ConstantPoolItem>) -> IResult<&'a [u8], Option<Arc<String>>> {
     let (data, super_class) = be_u16(data)?;
 
     let super_class = if super_class != 0 {
-        let class_name_index = constant_pool.get(&super_class).unwrap().class_name_index();
-        Some(constant_pool.get(&class_name_index).unwrap().utf8())
+        let class_name_index = constant_pool
+            .get(&super_class)
+            .and_then(ConstantPoolItem::class_name_index)
+            .ok_or_else(|| nom::Err::Error(Error::new(data, ErrorKind::Verify)))?;
+        Some(
+            constant_pool
+                .get(&class_name_index)
+                .and_then(ConstantPoolItem::utf8)
+                .ok_or_else(|| nom::Err::Error(Error::new(data, ErrorKind::Verify)))?,
+        )
     } else {
         None
     };
@@ -80,12 +99,23 @@ impl ClassInfo {
         ))
     }
 
-    pub fn parse(file: &[u8]) -> Option<Self> {
-        let (remaining, result) = Self::parse_info(file).ok()?;
+    pub fn parse(file: &[u8]) -> Result<Self, ClassFileError> {
+        let (remaining, result) = Self::parse_info(file).map_err(|_| ClassFileError::InvalidFormat)?;
         if !remaining.is_empty() {
-            return None;
+            return Err(ClassFileError::InvalidFormat);
+        }
+        if result.major_version < 45 {
+            return Err(ClassFileError::InvalidFormat);
+        }
+        if result.major_version > 70 {
+            return Err(ClassFileError::UnsupportedVersion(result.major_version));
         }
+        validate_class(&result)?;
+
+        Ok(result)
+    }
 
-        Some(result)
+    pub fn validate(&self) -> Result<(), ClassFileError> {
+        validate_class(self)
     }
 }