Refactoring: non-static fields hierarchy - use direct order from pare…

…nt to child
hextriclosan · Nov 29, 2024 · b546892 · b546892
1 parent 844f29f
commit b546892
Show file tree

Hide file tree

Showing 6 changed files with 36 additions and 22 deletions.
diff --git a/vm/src/execution_engine/executor.rs b/vm/src/execution_engine/executor.rs
@@ -45,7 +45,7 @@ impl Executor {
     pub fn invoke_default_constructor(class_name: &str) -> crate::error::Result<i32> {
         let instance_with_default_fields = with_method_area(|method_area| {
             method_area.create_instance_with_default_fields(class_name)
-        });
+        })?;
 
         let instance_ref =
             with_heap_write_lock(|heap| heap.create_instance(instance_with_default_fields));
@@ -61,7 +61,7 @@ impl Executor {
     ) -> crate::error::Result<i32> {
         let instance_with_default_fields = with_method_area(|method_area| {
             method_area.create_instance_with_default_fields(&class_name)
-        });
+        })?;
 
         let instance_ref =
             with_heap_write_lock(|heap| heap.create_instance(instance_with_default_fields));
@@ -77,7 +77,7 @@ impl Executor {
     pub fn create_primordial_thread(args: &[StackValueKind]) -> crate::error::Result<i32> {
         let instance_with_default_fields = with_method_area(|method_area| {
             method_area.create_instance_with_default_fields("java/lang/Thread")
-        });
+        })?;
 
         let thread_obj_ref =
             with_heap_write_lock(|heap| heap.create_instance(instance_with_default_fields));

diff --git a/vm/src/execution_engine/ops_reference_processor.rs b/vm/src/execution_engine/ops_reference_processor.rs
@@ -250,7 +250,7 @@ pub(crate) fn process(
                 })?;
             let instance_with_default_fields = with_method_area(|method_area| {
                 method_area.create_instance_with_default_fields(&class_to_invoke_new_for)
-            });
+            })?;
 
             let instanceref =
                 with_heap_write_lock(|heap| heap.create_instance(instance_with_default_fields));

diff --git a/vm/src/execution_engine/reflection_class_loader.rs b/vm/src/execution_engine/reflection_class_loader.rs
@@ -87,7 +87,7 @@ impl ReflectionClassLoader {
     ) -> crate::error::Result<i32> {
         let mut reflection_instance = with_method_area(|method_area| {
             method_area.create_instance_with_default_fields("java/lang/Class")
-        });
+        })?;
         reflection_instance.set_field_value(
             "java/lang/Class",
             "componentType:Ljava/lang/Class;",

diff --git a/vm/src/heap/java_instance.rs b/vm/src/heap/java_instance.rs
@@ -10,7 +10,7 @@ pub type FieldNameType = String;
 #[derive(Debug, Serialize, Clone)]
 pub(crate) struct JavaInstance {
     instance_name: String,
-    fields: IndexMap<ClassName, HashMap<FieldNameType, Field>>,
+    fields: IndexMap<ClassName, IndexMap<FieldNameType, Field>>,
 }
 
 #[derive(Debug, Clone, Serialize)]
@@ -85,7 +85,7 @@ pub(crate) enum HeapValue {
 impl<'a> JavaInstance {
     pub fn new(
         instance_name: String,
-        fields: IndexMap<ClassName, HashMap<FieldNameType, Field>>,
+        fields: IndexMap<ClassName, IndexMap<FieldNameType, Field>>,
     ) -> Self {
         Self {
             instance_name,
@@ -129,7 +129,8 @@ impl<'a> JavaInstance {
             Some(start_index) => self
                 .fields
                 .iter()
-                .skip(start_index)
+                .take(start_index + 1)
+                .rev()
                 .find_map(|(_, map)| map.get(field_name_type)),
             None => None,
         }
@@ -144,7 +145,8 @@ impl<'a> JavaInstance {
             Some(start_index) => self
                 .fields
                 .iter_mut()
-                .skip(start_index)
+                .take(start_index + 1)
+                .rev()
                 .find_map(|(_, map)| map.get_mut(field_name_type)),
             None => None,
         }

diff --git a/vm/src/method_area/java_class.rs b/vm/src/method_area/java_class.rs
@@ -117,7 +117,7 @@ impl JavaClass {
             .get(instance_field_name_type)
     }
 
-    pub fn default_value_instance_fields(&self) -> HashMap<FieldNameType, Field> {
+    pub fn default_value_instance_fields(&self) -> IndexMap<FieldNameType, Field> {
         self.non_static_field_descriptors
             .descriptor_by_name
             .iter()

diff --git a/vm/src/method_area/method_area.rs b/vm/src/method_area/method_area.rs
@@ -370,27 +370,39 @@ impl MethodArea {
         }
     }
 
-    pub fn create_instance_with_default_fields(&self, class_name: &str) -> JavaInstance {
+    pub fn create_instance_with_default_fields(
+        &self,
+        class_name: &str,
+    ) -> crate::error::Result<JavaInstance> {
         let mut instance_fields_hierarchy = IndexMap::new();
-        self.lookup_and_fill_instance_fields_hierarchy(class_name, &mut instance_fields_hierarchy);
-        JavaInstance::new(class_name.to_string(), instance_fields_hierarchy)
+        self.lookup_and_fill_instance_fields_hierarchy(
+            class_name,
+            &mut instance_fields_hierarchy,
+        )?;
+
+        Ok(JavaInstance::new(
+            class_name.to_string(),
+            instance_fields_hierarchy,
+        ))
     }
 
     fn lookup_and_fill_instance_fields_hierarchy(
         &self,
         class_name: &str,
-        instance_fields_hierarchy: &mut IndexMap<ClassName, HashMap<FieldNameType, Field>>,
-    ) -> Option<JavaMethod> {
-        let rc = self.get(class_name).ok()?;
+        instance_fields_hierarchy: &mut IndexMap<ClassName, IndexMap<FieldNameType, Field>>,
+    ) -> crate::error::Result<()> {
+        let rc = self.get(class_name)?;
+        if let Some(parent_class_name) = rc.parent().as_ref() {
+            self.lookup_and_fill_instance_fields_hierarchy(
+                parent_class_name,
+                instance_fields_hierarchy,
+            )?;
+        };
+
         let instance_fields = rc.default_value_instance_fields();
         instance_fields_hierarchy.insert(class_name.to_string(), instance_fields);
 
-        let parent_class_name = rc.parent().clone()?;
-
-        self.lookup_and_fill_instance_fields_hierarchy(
-            &parent_class_name,
-            instance_fields_hierarchy,
-        )
+        Ok(())
     }
 
     pub(crate) fn put_to_reflection_table(&self, reflection_ref: i32, java_class_name: &str) {