Fix instance field resolution and storage across inheritance (#207)

* Fix instance field resolution and storage across inheritance

* Require explicit class names in instance field APIs

Author: dlunch

jvm-bytecode/src/class_definition.rs

@@ -83,7 +83,11 @@ impl ClassDefinitionImpl {
             .map(|x| MethodImpl::from_method_proto(x, context.clone()))
             .collect::<Vec<_>>();
 
-        let fields = proto.fields.into_iter().map(FieldImpl::from_field_proto).collect::<Vec<_>>();
+        let fields = proto
+            .fields
+            .into_iter()
+            .map(|field| FieldImpl::from_field_proto(proto.name, field))
+            .collect::<Vec<_>>();
 
         let interfaces = proto.interfaces.into_iter().map(|x| x.to_string()).collect();
 
@@ -111,7 +115,7 @@ impl ClassDefinitionImpl {
                     _ => None,
                 });
 
-                let field = FieldImpl::from_field_info(field_info);
+                let field = FieldImpl::from_field_info(&class.this_class, field_info);
                 if let Some(x) = constant
                     && field.access_flags().contains(FieldAccessFlags::STATIC)
                 {

jvm-bytecode/src/field.rs

@@ -11,6 +11,7 @@ use jvm_types::FieldAccessFlags;
 
 #[derive(Debug, Eq, PartialEq, Ord, PartialOrd)]
 struct FieldInner {
+    declaring_class: String,
     name: String,
     descriptor: String,
     access_flags: FieldAccessFlags,
@@ -22,28 +23,23 @@ pub struct FieldImpl {
 }
 
 impl FieldImpl {
-    pub fn new(name: &str, descriptor: &str, access_flags: FieldAccessFlags) -> Self {
+    pub fn new(declaring_class: &str, name: &str, descriptor: &str, access_flags: FieldAccessFlags) -> Self {
         Self {
             inner: Arc::new(FieldInner {
+                declaring_class: declaring_class.to_string(),
                 name: name.to_string(),
                 descriptor: descriptor.to_string(),
                 access_flags,
             }),
         }
     }
 
-    pub fn from_field_proto(proto: JavaFieldProto) -> Self {
-        Self::new(&proto.name, &proto.descriptor, proto.access_flags)
+    pub fn from_field_proto(declaring_class: &str, proto: JavaFieldProto) -> Self {
+        Self::new(declaring_class, &proto.name, &proto.descriptor, proto.access_flags)
     }
 
-    pub fn from_field_info(field_info: FieldInfo) -> Self {
-        Self {
-            inner: Arc::new(FieldInner {
-                name: field_info.name.to_string(),
-                descriptor: field_info.descriptor.to_string(),
-                access_flags: field_info.access_flags,
-            }),
-        }
+    pub fn from_field_info(declaring_class: &str, field_info: FieldInfo) -> Self {
+        Self::new(declaring_class, &field_info.name, &field_info.descriptor, field_info.access_flags)
     }
 }
 

jvm-bytecode/src/interpreter.rs

@@ -445,7 +445,7 @@ impl Interpreter {
                         return Err(jvm.exception("java/lang/NullPointerException", "null").await);
                     }
 
-                    let value = jvm.get_field(&instance.unwrap(), &x.name, &x.descriptor).await?;
+                    let value = jvm.get_field(&instance.unwrap(), &x.class, &x.name, &x.descriptor).await?;
 
                     stack_frame.operand_stack.push(Self::to_stack_frame_type(value));
                 }
@@ -921,7 +921,7 @@ impl Interpreter {
 
                     let value = Self::to_field_type(&x.descriptor, value);
 
-                    jvm.put_field(instance.as_mut().unwrap(), &x.name, &x.descriptor, value).await?;
+                    jvm.put_field(instance.as_mut().unwrap(), &x.class, &x.name, &x.descriptor, value).await?;
                 }
                 Opcode::Putstatic(x) => {
                     let x = x.as_field_ref();

jvm/src/jvm.rs

@@ -261,15 +261,19 @@ impl Jvm {
         }
     }
 
-    pub async fn get_field<T>(&self, instance: &Box<dyn ClassInstance>, name: &str, descriptor: &str) -> Result<T>
+    pub async fn get_field<T>(&self, instance: &Box<dyn ClassInstance>, class_name: &str, name: &str, descriptor: &str) -> Result<T>
     where
         T: From<JavaValue>,
     {
-        tracing::trace!("Get field {}.{name}:{descriptor}", instance.class_definition().name());
+        tracing::trace!("Get field {class_name}.{name}:{descriptor}");
 
-        let field = self.find_field(&*instance.class_definition(), name, descriptor)?;
-
-        if let Some(field) = field {
+        let class = self.resolve_class(class_name).await?;
+        if let Some((_, field)) = self.resolve_field(&class, name, descriptor) {
+            if field.access_flags().contains(FieldAccessFlags::STATIC) {
+                return Err(self
+                    .exception("java/lang/IncompatibleClassChangeError", &format!("{class_name}.{name}:{descriptor}"))
+                    .await);
+            }
             let value = instance.get_field(&*field)?;
             if let JavaValue::Object(Some(instance)) = &value {
                 let thread_id = (self.inner.get_current_thread_id)();
@@ -285,30 +289,28 @@ impl Jvm {
             Ok(value.into())
         } else {
             Err(self
-                .exception(
-                    "java/lang/NoSuchFieldError",
-                    &format!("{}.{}:{}", instance.class_definition().name(), name, descriptor),
-                )
+                .exception("java/lang/NoSuchFieldError", &format!("{class_name}.{name}:{descriptor}"))
                 .await)
         }
     }
 
-    pub async fn put_field<T>(&self, instance: &mut Box<dyn ClassInstance>, name: &str, descriptor: &str, value: T) -> Result<()>
+    pub async fn put_field<T>(&self, instance: &mut Box<dyn ClassInstance>, class_name: &str, name: &str, descriptor: &str, value: T) -> Result<()>
     where
         T: Into<JavaValue> + Debug,
     {
-        tracing::trace!("Put field {}.{name}:{descriptor} = {value:?}", instance.class_definition().name());
+        tracing::trace!("Put field {class_name}.{name}:{descriptor} = {value:?}");
 
-        let field = self.find_field(&*instance.class_definition(), name, descriptor)?;
-
-        if let Some(field) = field {
+        let class = self.resolve_class(class_name).await?;
+        if let Some((_, field)) = self.resolve_field(&class, name, descriptor) {
+            if field.access_flags().contains(FieldAccessFlags::STATIC) {
+                return Err(self
+                    .exception("java/lang/IncompatibleClassChangeError", &format!("{class_name}.{name}:{descriptor}"))
+                    .await);
+            }
             instance.put_field(&*field, value.into())
         } else {
             Err(self
-                .exception(
-                    "java/lang/NoSuchFieldError",
-                    &format!("{}.{}:{}", instance.class_definition().name(), name, descriptor),
-                )
+                .exception("java/lang/NoSuchFieldError", &format!("{class_name}.{name}:{descriptor}"))
                 .await)
         }
     }
@@ -1099,7 +1101,7 @@ impl Jvm {
             .get_mut(&thread_id)
             .unwrap()
             .set_java_thread(java_thread.clone());
-        let interrupted: bool = self.get_field(&java_thread, "interrupted", "Z").await?;
+        let interrupted: bool = self.get_field(&java_thread, "java/lang/Thread", "interrupted", "Z").await?;
         self.inner.threads.write().get_mut(&thread_id).unwrap().interrupted |= interrupted;
 
         Ok(())

jvm/src/runtime/java_lang_class.rs

@@ -12,14 +12,14 @@ impl JavaLangClass {
         let mut name_bytes = jvm.instantiate_array("B", name.len()).await?;
         let bytes: Vec<i8> = cast_vec(name.as_bytes().to_vec());
         jvm.store_array(&mut name_bytes, 0, bytes).await?;
-        jvm.put_field(&mut java_class, "nameBytes", "[B", name_bytes).await?;
+        jvm.put_field(&mut java_class, "java/lang/Class", "nameBytes", "[B", name_bytes).await?;
 
         Ok(java_class)
     }
 
     #[allow(clippy::borrowed_box)]
     pub async fn name(jvm: &Jvm, this: &Box<dyn ClassInstance>) -> Result<String> {
-        let name_bytes: ClassInstanceRef<Array<i8>> = jvm.get_field(this, "nameBytes", "[B").await?;
+        let name_bytes: ClassInstanceRef<Array<i8>> = jvm.get_field(this, "java/lang/Class", "nameBytes", "[B").await?;
         let len = jvm.array_length(&name_bytes).await?;
         let name_bytes_vec: Vec<i8> = jvm.load_array(&name_bytes, 0, len).await?;
         match String::from_utf8(cast_vec(name_bytes_vec)) {
@@ -49,16 +49,16 @@ impl JavaLangClass {
         let mut name_bytes = jvm.instantiate_array("B", class_name.len()).await?;
         let bytes: Vec<i8> = cast_vec(class_name.into_owned().into_bytes());
         jvm.store_array(&mut name_bytes, 0, bytes).await?;
-        jvm.put_field(&mut java_class, "nameBytes", "[B", name_bytes).await?;
+        jvm.put_field(&mut java_class, "java/lang/Class", "nameBytes", "[B", name_bytes).await?;
 
-        jvm.put_field(&mut java_class, "classLoader", "Ljava/lang/ClassLoader;", class_loader)
+        jvm.put_field(&mut java_class, "java/lang/Class", "classLoader", "Ljava/lang/ClassLoader;", class_loader)
             .await?;
 
         Ok(java_class)
     }
 
     #[allow(clippy::borrowed_box)]
     pub async fn class_loader(jvm: &Jvm, this: &Box<dyn ClassInstance>) -> Result<Option<Box<dyn ClassInstance>>> {
-        jvm.get_field(this, "classLoader", "Ljava/lang/ClassLoader;").await
+        jvm.get_field(this, "java/lang/Class", "classLoader", "Ljava/lang/ClassLoader;").await
     }
 }

jvm/src/runtime/java_lang_string.rs

@@ -7,9 +7,9 @@ pub struct JavaLangString;
 impl JavaLangString {
     #[allow(clippy::borrowed_box)]
     pub async fn to_utf16(jvm: &Jvm, this: &Box<dyn ClassInstance>) -> Result<Vec<JavaChar>> {
-        let value = jvm.get_field(this, "value", "[C").await?;
-        let offset: i32 = jvm.get_field(this, "offset", "I").await?;
-        let count: i32 = jvm.get_field(this, "count", "I").await?;
+        let value = jvm.get_field(this, "java/lang/String", "value", "[C").await?;
+        let offset: i32 = jvm.get_field(this, "java/lang/String", "offset", "I").await?;
+        let count: i32 = jvm.get_field(this, "java/lang/String", "count", "I").await?;
 
         // access flags are not enforced, so bytecode can leave a negative here, which would widen into a huge usize
         if offset < 0 || count < 0 {

jvm/tests/test_garbage_collection.rs

@@ -230,10 +230,11 @@ async fn field_and_method_results_are_local_references() -> JvmResult<()> {
     assert_eq!(jvm.collect_garbage()?, 0);
 
     jvm.push_native_frame();
-    let _: ClassInstanceRef<Array<Object>> = jvm.get_field(&vector, "elementData", "[Ljava/lang/Object;").await?;
+    let _: ClassInstanceRef<Array<Object>> = jvm.get_field(&vector, "java/util/Vector", "elementData", "[Ljava/lang/Object;").await?;
     let mut mutable_vector = (*vector).clone();
     jvm.put_field(
         &mut mutable_vector,
+        "java/util/Vector",
         "elementData",
         "[Ljava/lang/Object;",
         ClassInstanceRef::<Array<Object>>::new(None),