Refactoring including fixes: rework of non-static field offset

tests/should_do_unsafe_object_field_offset.rs

@@ -0,0 +1,10 @@
+mod utils;
+use utils::get_int;
+use vm::vm::VM;
+
+#[test]
+fn should_do_unsafe_object_field_offset() {
+    let last_frame_value =
+        VM::run("samples.jdkinternal.unsafe.objectfieldoffset.UnsafeObjectFieldOffset").unwrap();
+    assert_eq!(127, get_int(last_frame_value))
+}

tests/test_data/UnsafeObjectFieldOffset.java

@@ -0,0 +1,125 @@
+// javac --add-exports java.base/jdk.internal.misc=ALL-UNNAMED  -d . UnsafeObjectFieldOffset.java
+
+package samples.jdkinternal.unsafe.objectfieldoffset;
+
+import jdk.internal.misc.Unsafe;
+
+public class UnsafeObjectFieldOffset {
+
+    public static void main(String[] args) {
+        One one = new One(1, 2);
+        boolean oneFieldOneSet = one.compareAndSetFieldOne(1, 10);
+        boolean oneFieldThreeSet = one.compareAndSetFieldThree(2, 20);
+        int oneFieldOne = one.getFieldOne();
+        int oneFieldThree = one.getFieldThree();
+        int bit0 = oneFieldOneSet && oneFieldOne == 10 ? 1 : 0;
+        int bit1 = oneFieldThreeSet && oneFieldThree == 20 ? 1 : 0;
+
+        Two two = new Two(-1, -2, -3);
+        int twoFieldOne = two.getFieldOne(); // -1
+        int twoFieldTwo = two.getFieldTwo(); // -2
+        int twoFieldThree = two.getFieldThree(); // -3
+        int twoFieldThreeFromParent = two.getFieldThreeFromParent(); // -27
+        int bit2 = twoFieldOne == -1 && twoFieldTwo == -2 && twoFieldThree == -3 && twoFieldThreeFromParent == -27
+            ? 1
+            : 0;
+
+        boolean twoFieldOneSet = two.compareAndSetFieldOne(-1, -10);
+        boolean twoFieldTwoSet = two.compareAndSetFieldTwo(-2, -20);
+        boolean twoFieldThreeSet = two.compareAndSetFieldThree(-3, -30);
+        boolean twoFieldThreeFromParentSet = two.compareAndSetFieldThreeFromParent(-27, -270);
+        int twoFieldOneAfterSet = two.getFieldOne(); // -10
+        int twoFieldTwoAfterSet = two.getFieldTwo(); // -20
+        int twoFieldThreeAfterSet = two.getFieldThree(); // -30
+        int twoFieldThreeFromParentAfterSet = two.getFieldThreeFromParent(); // -270
+        int bit3 = twoFieldOneSet && twoFieldOneAfterSet == -10 ? 1 : 0;
+        int bit4 = twoFieldTwoSet && twoFieldTwoAfterSet == -20 ? 1 : 0;
+        int bit5 = twoFieldThreeSet && twoFieldThreeAfterSet == -30 ? 1 : 0;
+        int bit6 = twoFieldThreeFromParentSet && twoFieldThreeFromParentAfterSet == -270 ? 1 : 0;
+
+        int result = 0;
+        result = setBit(result, 0, bit0);
+        result = setBit(result, 1, bit1);
+        result = setBit(result, 2, bit2);
+        result = setBit(result, 3, bit3);
+        result = setBit(result, 4, bit4);
+        result = setBit(result, 5, bit5);
+        result = setBit(result, 6, bit6);
+    }
+
+    private static int setBit(int num, int position, int value) {
+        return value == 0 ? num & ~(1 << position) : num | (1 << position);
+    }
+}
+
+class One {
+    protected static final Unsafe U = Unsafe.getUnsafe();
+    private static final long FIELD_ONE_OFFSET = U.objectFieldOffset(One.class, "fieldOne");
+    protected static final long FIELD_THREE_OFFSET = U.objectFieldOffset(One.class, "fieldThree");
+
+    // Each field should have a particular offset which is the same for all instances of the class
+    // no matter if the class is parent of another class or not.
+    int placeholder1; // possible offset 0
+    int placeholder2; // possible offset 4
+    int fieldOne;     // possible offset 8
+    int fieldThree;   // possible offset 12
+    public One(int fieldOne, int fieldThree) {
+        this.fieldOne = fieldOne;
+        this.fieldThree = fieldThree;
+    }
+
+    public int getFieldOne() {
+        return fieldOne;
+    }
+
+    public int getFieldThree() {
+        return fieldThree;
+    }
+
+    protected boolean compareAndSetFieldOne(int expect, int update) {
+        return U.compareAndSetInt(this, FIELD_ONE_OFFSET, expect, update);
+    }
+
+    protected boolean compareAndSetFieldThree(int expect, int update) {
+        return U.compareAndSetInt(this, FIELD_THREE_OFFSET, expect, update);
+    }
+}
+
+class Two extends One {
+    private static final long FIELD_TWO_OFFSET = U.objectFieldOffset(Two.class, "fieldTwo");
+    private static final long FIELD_THREE_OFFSET = U.objectFieldOffset(Two.class, "fieldThree");
+
+    int placeholder10; // possible offset 16
+    int fieldTwo; // possible offset 20
+    int fieldThree; // possible offset 24, shadows One.fieldThree
+    public Two(int fieldOne, int fieldTwo, int fieldThree) {
+        super(fieldOne, fieldThree * 9);
+        this.fieldTwo = fieldTwo;
+        this.fieldThree = fieldThree;
+    }
+
+    public int getFieldTwo() {
+        return fieldTwo;
+    }
+
+    @Override
+    public int getFieldThree() {
+        return fieldThree;
+    }
+
+    public int getFieldThreeFromParent() {
+        return super.fieldThree;
+    }
+
+    protected boolean compareAndSetFieldTwo(int expect, int update) {
+        return U.compareAndSetInt(this, FIELD_TWO_OFFSET, expect, update);
+    }
+
+    protected boolean compareAndSetFieldThree(int expect, int update) {
+        return U.compareAndSetInt(this, FIELD_THREE_OFFSET, expect, update);
+    }
+
+    protected boolean compareAndSetFieldThreeFromParent(int expect, int update) {
+        return U.compareAndSetInt(this, One.FIELD_THREE_OFFSET, expect, update);
+    }
+}

vm/src/method_area/java_class.rs

@@ -1,17 +1,19 @@
+use crate::error::Error;
 use crate::execution_engine::executor::Executor;
 use crate::heap::java_instance::{ClassName, FieldNameType};
 use crate::method_area::cpool_helper::CPoolHelper;
 use crate::method_area::field::Field;
 use crate::method_area::java_method::JavaMethod;
 use crate::method_area::method_area::with_method_area;
-use indexmap::IndexMap;
+use indexmap::{IndexMap, IndexSet};
 use jdescriptor::TypeDescriptor;
 use once_cell::sync::OnceCell;
 use std::collections::{HashMap, HashSet};
 use std::sync::atomic::{AtomicBool, Ordering};
 use std::sync::Arc;
 
 const INTERFACE: u16 = 0x00000200;
+type FullyQualifiedFieldName = String; // format: com/example/models/Person.name
 
 #[derive(Debug)]
 pub(crate) struct JavaClass {
@@ -27,6 +29,7 @@ pub(crate) struct JavaClass {
     static_fields_initialized: AtomicBool,
 
     instance_fields_hierarchy: OnceCell<IndexMap<ClassName, IndexMap<FieldNameType, Field>>>,
+    fields_offset_mapping: OnceCell<IndexSet<FullyQualifiedFieldName>>,
 }
 
 #[derive(Debug)]
@@ -78,6 +81,7 @@ impl JavaClass {
             access_flags,
             static_fields_initialized: AtomicBool::new(false),
             instance_fields_hierarchy: OnceCell::new(),
+            fields_offset_mapping: OnceCell::new(),
         }
     }
 
@@ -138,46 +142,35 @@ impl JavaClass {
         self.access_flags
     }
 
-    pub fn get_field_offset(&self, field_name: &str) -> crate::error::Result<i64> {
-        let key = self
-            .non_static_field_descriptors
-            .descriptor_by_name
-            .iter()
-            .find_map(|(key, _)| {
-                let first = key.split(':').next().map(|n| n.to_string())?;
-                if first == field_name {
-                    Some(key)
-                } else {
-                    None
-                }
-            })
-            .ok_or_else(|| {
-                crate::error::Error::new_native(&format!("Field {field_name} not found"))
-            })?;
-
+    pub fn get_field_offset(&self, fully_qualified_field_name: &str) -> crate::error::Result<i64> {
         let offset = self
-            .non_static_field_descriptors
-            .descriptor_by_name
-            .get_index_of(key)
+            .fields_offset_mapping()
+            .get_index_of(fully_qualified_field_name)
             .ok_or_else(|| {
-                crate::error::Error::new_native(&format!(
-                    "Failed to get index by key {field_name}"
+                Error::new_execution(&format!(
+                    "Failed to get offset by name {fully_qualified_field_name}"
                 ))
             })?;
-
         Ok(offset as i64)
     }
 
-    pub fn get_field_name_by_offset(&self, offset: i64) -> crate::error::Result<String> {
-        let (field_name, _) = self
-            .non_static_field_descriptors
-            .descriptor_by_name
+    pub fn get_field_name_by_offset(&self, offset: i64) -> crate::error::Result<(String, String)> {
+        let result = self
+            .fields_offset_mapping()
             .get_index(offset as usize)
             .ok_or_else(|| {
-                crate::error::Error::new_native(&format!("Failed to get entry by index {offset}"))
+                Error::new_execution(&format!("Failed to get field name by offset {offset}"))
             })?;
 
-        Ok(field_name.clone())
+        let mut parts = result.split('.');
+        let class_name = parts.next().ok_or_else(|| {
+            Error::new_execution(&format!("Failed to get class name by offset {offset}"))
+        })?;
+        let field_name = parts.next().ok_or_else(|| {
+            Error::new_execution(&format!("Failed to get field name by offset {offset}"))
+        })?;
+
+        Ok((class_name.to_string(), field_name.to_string()))
     }
 
     fn get_method_internal(&self, full_signature: &str) -> Option<Arc<JavaMethod>> {
@@ -193,7 +186,7 @@ impl JavaClass {
 
     pub fn get_method(&self, full_signature: &str) -> crate::error::Result<Arc<JavaMethod>> {
         self.get_method_internal(full_signature).ok_or_else(|| {
-            crate::error::Error::new_native(&format!(
+            Error::new_native(&format!(
                 "Method {full_signature} not found in {}",
                 self.this_class_name
             ))
@@ -202,8 +195,8 @@ impl JavaClass {
 
     pub fn instance_fields_hierarchy(
         &self,
-    ) -> crate::error::Result<&IndexMap<ClassName, IndexMap<FieldNameType, Field>>> {
-        Ok(&self.instance_fields_hierarchy.get_or_init(|| {
+    ) -> &IndexMap<ClassName, IndexMap<FieldNameType, Field>> {
+        &self.instance_fields_hierarchy.get_or_init(|| {
             let mut instance_fields_hierarchy = IndexMap::new();
             with_method_area(|area| {
                 area.lookup_and_fill_instance_fields_hierarchy(
@@ -214,7 +207,22 @@ impl JavaClass {
             .expect("error getting instance fields hierarchy");
 
             instance_fields_hierarchy
-        }))
+        })
+    }
+
+    fn fields_offset_mapping(&self) -> &IndexSet<FullyQualifiedFieldName> {
+        &self.fields_offset_mapping.get_or_init(|| {
+            let mut fields_offset_mapping = IndexSet::new();
+            let hierarchy = self.instance_fields_hierarchy();
+
+            hierarchy.iter().for_each(|(class_name, fields)| {
+                fields.iter().for_each(|(field_name, _)| {
+                    fields_offset_mapping.insert(format!("{class_name}.{field_name}"));
+                });
+            });
+
+            fields_offset_mapping
+        })
     }
 }
 

vm/src/method_area/method_area.rs

@@ -376,7 +376,7 @@ impl MethodArea {
         let jc = with_method_area(|area| area.get(class_name))?;
         Ok(JavaInstance::new(
             class_name.to_string(),
-            jc.instance_fields_hierarchy()?.clone(),
+            jc.instance_fields_hierarchy().clone(),
         ))
     }
 

vm/src/system_native/unsafe_.rs

@@ -1,7 +1,10 @@
+use crate::error::Error;
 use crate::heap::heap::{with_heap_read_lock, with_heap_write_lock};
 use crate::helper::{i32toi64, i64_to_vec};
+use crate::method_area::java_class::JavaClass;
 use crate::method_area::method_area::with_method_area;
 use crate::system_native::string::get_utf8_string_by_ref;
+use std::sync::Arc;
 
 pub(crate) fn object_field_offset_1_wrp(args: &[i32]) -> crate::error::Result<Vec<i32>> {
     let _unsafe_ref = args[0];
@@ -16,12 +19,13 @@ pub(crate) fn object_field_offset_1_wrp(args: &[i32]) -> crate::error::Result<Ve
 }
 fn object_field_offset_1(class_ref: i32, string_ref: i32) -> crate::error::Result<i64> {
     let field_name = get_utf8_string_by_ref(string_ref)?;
-    let jc = with_method_area(|area| {
+    let (class_name, jc) = with_method_area(|area| {
         let class_name = area.get_from_reflection_table(class_ref)?;
-        area.get(class_name.as_str())
+        let jc = area.get(class_name.as_str())?;
+        Ok::<(String, Arc<JavaClass>), Error>((class_name, jc))
     })?;
 
-    let offset = jc.get_field_offset(&field_name)?;
+    let offset = jc.get_field_offset(&format!("{class_name}.{field_name}"))?;
 
     Ok(offset)
 }
@@ -58,7 +62,7 @@ fn compare_and_set_int(
         })
     } else {
         let jc = with_method_area(|area| area.get(class_name.as_str()))?;
-        let field_name = jc.get_field_name_by_offset(offset)?;
+        let (class_name, field_name) = jc.get_field_name_by_offset(offset)?;
         with_heap_write_lock(|heap| {
             let result = heap
                 .get_object_field_value(obj_ref, &class_name, &field_name)
@@ -91,7 +95,7 @@ fn get_reference_volatile(obj_ref: i32, offset: i64) -> crate::error::Result<i32
         with_heap_read_lock(|heap| heap.get_array_value(obj_ref, offset as i32).cloned())?
     } else {
         let jc = with_method_area(|area| area.get(class_name.as_str()))?;
-        let field_name = jc.get_field_name_by_offset(offset)?;
+        let (class_name, field_name) = jc.get_field_name_by_offset(offset)?;
         with_heap_read_lock(|heap| heap.get_object_field_value(obj_ref, &class_name, &field_name))?
     };
 
@@ -140,7 +144,7 @@ fn compare_and_set_long(
 
     let jc = with_method_area(|area| area.get(class_name.as_str()))?;
 
-    let field_name = jc.get_field_name_by_offset(offset)?;
+    let (class_name, field_name) = jc.get_field_name_by_offset(offset)?;
 
     let updated = with_heap_write_lock(|heap| {
         let bytes = heap