From dd4ca90513add24aab9df454f82d29a55191224e Mon Sep 17 00:00:00 2001 From: hankluo6 Date: Tue, 7 Sep 2021 14:03:26 +0800 Subject: [PATCH 1/2] Handle inheritance correctly If fields or methods are not found in the class, pitifulvm will try to find them from the parent class recursively. Note that ptitfulvm have not supported objects' fields in the parent class yet, so `getfield` and `putfield` still don't find fields from the parent class. Add a new test script: "Inherit.java" --- Makefile | 3 +- class-heap.c | 1 + classfile.c | 17 +++--- classfile.h | 4 +- jvm.c | 126 ++++++++++++++++++++++++++++++++------------- tests/Inherit.java | 54 +++++++++++++++++++ 6 files changed, 159 insertions(+), 46 deletions(-) create mode 100644 tests/Inherit.java diff --git a/Makefile b/Makefile index 2620798..155f130 100644 --- a/Makefile +++ b/Makefile @@ -48,7 +48,8 @@ TESTS = \ Constructor \ Field \ Static \ - Invokevirtual + Invokevirtual \ + Inherit check: $(addprefix tests/,$(TESTS:=-result.out)) diff --git a/class-heap.c b/class-heap.c index 8221d01..eda6840 100644 --- a/class-heap.c +++ b/class-heap.c @@ -95,6 +95,7 @@ void free_class_heap() free(constant->info); } free(class_heap.class_info[i]->clazz->constant_pool.constant_pool); + free(class_heap.class_info[i]->clazz->info); field_t *field = class_heap.class_info[i]->clazz->fields; for (u2 j = 0; j < class_heap.class_info[i]->clazz->fields_count; diff --git a/classfile.c b/classfile.c index fb4acff..da6242f 100644 --- a/classfile.c +++ b/classfile.c @@ -9,13 +9,13 @@ class_header_t get_class_header(FILE *class_file) }; } -class_info_t get_class_info(FILE *class_file) +class_info_t *get_class_info(FILE *class_file) { - class_info_t info = { - .access_flags = read_u2(class_file), - .this_class = read_u2(class_file), - .super_class = read_u2(class_file), - }; + class_info_t *info = malloc(sizeof(class_info_t)); + info->access_flags = read_u2(class_file); + info->this_class = read_u2(class_file); + info->super_class = read_u2(class_file); + u2 interfaces_count = read_u2(class_file); assert(!interfaces_count && "This VM does not support interfaces."); return info; @@ -303,12 +303,15 @@ class_file_t get_class(FILE *class_file) class_file_t clazz = {.constant_pool = get_constant_pool(class_file)}; /* Read information about the class that was compiled. */ - get_class_info(class_file); + clazz.info = get_class_info(class_file); /* Read the list of fields */ clazz.fields = get_fields(class_file, &clazz.constant_pool, &clazz); /* Read the list of static methods */ clazz.methods = get_methods(class_file, &clazz.constant_pool); + + clazz.initialized = false; + return clazz; } \ No newline at end of file diff --git a/classfile.h b/classfile.h index 31fc94f..792811f 100644 --- a/classfile.h +++ b/classfile.h @@ -56,9 +56,11 @@ typedef struct { typedef struct { constant_pool_t constant_pool; + class_info_t *info; method_t *methods; field_t *fields; u2 fields_count; + bool initialized; } class_file_t; typedef struct { @@ -67,7 +69,7 @@ typedef struct { } meta_class_t; class_header_t get_class_header(FILE *class_file); -class_info_t get_class_info(FILE *class_file); +class_info_t *get_class_info(FILE *class_file); method_t *get_methods(FILE *class_file, constant_pool_t *cp); void read_method_attributes(FILE *class_file, method_info *info, diff --git a/jvm.c b/jvm.c index 18573a6..610c182 100644 --- a/jvm.c +++ b/jvm.c @@ -263,12 +263,28 @@ stack_entry_t *execute(method_t *method, /* the method to be called */ char *method_name, *method_descriptor, *class_name; - class_name = find_method_info_from_index(index, clazz, &method_name, - &method_descriptor); + method_t *own_method = NULL; + class_file_t *target_class = NULL; + + /* recursively find method from child to parent */ + while (!own_method) { + if (!target_class) + class_name = find_method_info_from_index( + index, clazz, &method_name, &method_descriptor); + else + class_name = find_class_name_from_index( + target_class->info->super_class, target_class); + find_or_add_class_to_heap(class_name, prefix, &target_class); + assert(target_class && + "Failed to load class in i_invokestatic"); + own_method = + find_method(method_name, method_descriptor, target_class); + } - class_file_t *target_class; - if (find_or_add_class_to_heap(class_name, prefix, &target_class)) { - /* Call static initialization */ + /* call static initialization. Only the class that contains this + * method should do static initialization */ + if (!target_class->initialized) { + target_class->initialized = true; method_t *method = find_method("", "()V", target_class); if (method) { local_variable_t own_locals[method->code.max_locals]; @@ -279,10 +295,7 @@ stack_entry_t *execute(method_t *method, free(exec_res); } } - assert(target_class && "Failed to load class in invokestatic"); - method_t *own_method = - find_method(method_name, method_descriptor, target_class); uint16_t num_params = get_number_of_parameters(own_method); local_variable_t own_locals[own_method->code.max_locals]; for (int i = num_params - 1; i >= 0; i--) @@ -794,9 +807,11 @@ stack_entry_t *execute(method_t *method, uint16_t index = ((param1 << 8) | param2); char *field_name, *field_descriptor, *class_name; + field_t *field = NULL; + class_file_t *target_class = NULL; + class_name = find_field_info_from_index(index, clazz, &field_name, &field_descriptor); - /* skip java.lang.System in order to support java print * method */ if (!strcmp(class_name, "java/lang/System")) { @@ -804,9 +819,21 @@ stack_entry_t *execute(method_t *method, break; } - class_file_t *target_class; - if (find_or_add_class_to_heap(class_name, prefix, &target_class)) { - /* Call static initialization */ + while (!field) { + if (target_class) + class_name = find_class_name_from_index( + target_class->info->super_class, target_class); + + find_or_add_class_to_heap(class_name, prefix, &target_class); + assert(target_class && "Failed to load class in i_getstatic"); + + field = find_field(field_name, field_descriptor, target_class); + } + + /* call static initialization. Only the class that contains this + * field should do static initialization */ + if (!target_class->initialized) { + target_class->initialized = true; method_t *method = find_method("", "()V", target_class); if (method) { local_variable_t own_locals[method->code.max_locals]; @@ -818,9 +845,6 @@ stack_entry_t *execute(method_t *method, } } - field_t *field = - find_field(field_name, field_descriptor, target_class); - switch (field_descriptor[0]) { case 'B': /* signed byte */ @@ -866,12 +890,33 @@ stack_entry_t *execute(method_t *method, uint16_t index = ((param1 << 8) | param2); char *field_name, *field_descriptor, *class_name; + field_t *field = NULL; + class_file_t *target_class = NULL; class_name = find_field_info_from_index(index, clazz, &field_name, &field_descriptor); - class_file_t *target_class; - if (find_or_add_class_to_heap(class_name, prefix, &target_class)) { - /* Call static initialization */ + /* skip java.lang.System in order to support java print + * method */ + if (!strcmp(class_name, "java/lang/System")) { + pc += 3; + break; + } + + while (!field) { + if (target_class) + class_name = find_class_name_from_index( + target_class->info->super_class, target_class); + + find_or_add_class_to_heap(class_name, prefix, &target_class); + assert(target_class && "Failed to load class in i_putstatic"); + + field = find_field(field_name, field_descriptor, target_class); + } + + /* call static initialization. Only the class that contains this + * field should do static initialization */ + if (!target_class->initialized) { + target_class->initialized = true; method_t *method = find_method("", "()V", target_class); if (method) { local_variable_t own_locals[method->code.max_locals]; @@ -882,8 +927,6 @@ stack_entry_t *execute(method_t *method, free(exec_res); } } - field_t *field = - find_field(field_name, field_descriptor, target_class); switch (field_descriptor[0]) { case 'B': @@ -938,6 +981,9 @@ stack_entry_t *execute(method_t *method, /* the method to be called */ char *method_name, *method_descriptor, *class_name; + class_file_t *target_class = NULL; + method_t *method = NULL; + class_name = find_method_info_from_index(index, clazz, &method_name, &method_descriptor); @@ -964,9 +1010,22 @@ stack_entry_t *execute(method_t *method, } /* FIXME: consider method modifier */ - class_file_t *target_class; - if (find_or_add_class_to_heap(class_name, prefix, &target_class)) { - /* Call static initialization */ + /* recursively find method from child to parent */ + while (!method) { + if (target_class) + class_name = find_class_name_from_index( + target_class->info->super_class, target_class); + find_or_add_class_to_heap(class_name, prefix, &target_class); + assert(target_class && + "Failed to load class in i_invokevirtual"); + method = + find_method(method_name, method_descriptor, target_class); + } + + /* call static initialization. Only the class that contains this + * method should do static initialization */ + if (!target_class->initialized) { + target_class->initialized = true; method_t *method = find_method("", "()V", target_class); if (method) { local_variable_t own_locals[method->code.max_locals]; @@ -977,8 +1036,7 @@ stack_entry_t *execute(method_t *method, free(exec_res); } } - method_t *method = - find_method(method_name, method_descriptor, target_class); + uint16_t num_params = get_number_of_parameters(method); local_variable_t own_locals[method->code.max_locals]; memset(own_locals, 0, sizeof(own_locals)); @@ -1178,8 +1236,12 @@ stack_entry_t *execute(method_t *method, } class_file_t *target_class; - if (find_or_add_class_to_heap(class_name, prefix, &target_class)) { - /* Call static initialization */ + find_or_add_class_to_heap(class_name, prefix, &target_class); + assert(target_class && "Failed to load class in i_invokespecial"); + + /* call static initialization */ + if (!target_class->initialized) { + target_class->initialized = true; method_t *method = find_method("", "()V", target_class); if (method) { local_variable_t own_locals[method->code.max_locals]; @@ -1190,7 +1252,6 @@ stack_entry_t *execute(method_t *method, free(exec_res); } } - assert(target_class && "Failed to load class in i_invokespecial"); /* find constructor method from class */ method_t *constructor = @@ -1254,15 +1315,6 @@ int main(int argc, char *argv[]) prefix[match - argv[1] + 1] = '\0'; } - method_t *method = find_method("", "()V", clazz); - if (method) { - local_variable_t own_locals[method->code.max_locals]; - stack_entry_t *exec_res = execute(method, own_locals, clazz); - assert(exec_res->type == STACK_ENTRY_NONE && - " must not return a value"); - free(exec_res); - } - /* execute the main method if found */ method_t *main_method = find_method("main", "([Ljava/lang/String;)V", clazz); diff --git a/tests/Inherit.java b/tests/Inherit.java new file mode 100644 index 0000000..9874413 --- /dev/null +++ b/tests/Inherit.java @@ -0,0 +1,54 @@ +public class Inherit { + static int x = 1; + + public static void static_call() { + System.out.println(1); + } + + public void virtual_call() { + System.out.println(2); + } + + public static void main(String[] args) { + Inherit obj = new Inherit(); + InheritA objA = new InheritA(); + InheritB objB = new InheritB(); + + /* check shared static fields */ + System.out.println(obj.x); + System.out.println(objA.x); + System.out.println(objB.x); + obj.x = 2; + System.out.println(obj.x); + System.out.println(objA.x); + System.out.println(objB.x); + objA.x = 3; + System.out.println(obj.x); + System.out.println(objA.x); + System.out.println(objB.x); + + /* check static methods inheritance (compiler will replace objects with classes) */ + obj.static_call(); + objA.static_call(); + objB.static_call(); + + /* check virtual methods inheritance */ + obj.virtual_call(); + objA.virtual_call(); + objB.virtual_call(); + } +} + +class InheritA extends Inherit { + +} + +class InheritB extends InheritA { + /* check override */ + public void virtual_call() { + System.out.println(3); + } + public static void static_call() { + System.out.println(4); + } +} \ No newline at end of file From 71875f4095fe3bb650156ce03b032b6a273e69c8 Mon Sep 17 00:00:00 2001 From: hankluo6 Date: Sun, 7 Aug 2022 22:14:26 +0800 Subject: [PATCH 2/2] Correct the order of constructor and static initialization In `new` opcode, jvm needs to call constructor and initialization in order. The sequence is begin at base class static initialization, and then derived class static initialization. After all classes are initialized, base class constructor will be executed, then execute its child class constructor. Add a new test script: Initializer.java --- Makefile | 3 ++- jvm.c | 24 +++++++++++++++++++++--- tests/Initializer.java | 37 +++++++++++++++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 4 deletions(-) create mode 100644 tests/Initializer.java diff --git a/Makefile b/Makefile index 155f130..e847e89 100644 --- a/Makefile +++ b/Makefile @@ -49,7 +49,8 @@ TESTS = \ Field \ Static \ Invokevirtual \ - Inherit + Inherit \ + Initializer check: $(addprefix tests/,$(TESTS:=-result.out)) diff --git a/jvm.c b/jvm.c index 610c182..3dd587b 100644 --- a/jvm.c +++ b/jvm.c @@ -1196,8 +1196,26 @@ stack_entry_t *execute(method_t *method, char *class_name = find_class_name_from_index(index, clazz); class_file_t *target_class; - if (find_or_add_class_to_heap(class_name, prefix, &target_class)) { - /* Call static initialization */ + + /* FIXME: use linked list to prevent wasted space */ + class_file_t **stack = malloc(sizeof(class_file_t *) * 100); + size_t count = 0; + while (true) { + find_or_add_class_to_heap(class_name, prefix, &target_class); + assert(target_class && "Failed to load class in i_new"); + stack[count++] = target_class; + class_name = find_class_name_from_index( + target_class->info->super_class, target_class); + if (!strcmp(class_name, "java/lang/Object")) + break; + } + + /* call static initialization */ + while (count) { + target_class = stack[--count]; + if (target_class->initialized) + continue; + target_class->initialized = true; method_t *method = find_method("", "()V", target_class); if (method) { local_variable_t own_locals[method->code.max_locals]; @@ -1208,7 +1226,7 @@ stack_entry_t *execute(method_t *method, free(exec_res); } } - assert(target_class && "Failed to load class in new"); + free(stack); object_t *object = create_object(target_class); push_ref(op_stack, object); diff --git a/tests/Initializer.java b/tests/Initializer.java new file mode 100644 index 0000000..72e5974 --- /dev/null +++ b/tests/Initializer.java @@ -0,0 +1,37 @@ +public class Initializer { + static { + System.out.println(1); + } + Initializer () { + System.out.println(2); + } + public static void call() { + + } + public static void main(String[] args) { + /* only call `Initializer` init */ + InitializerB.call(); + /* call remaining init in order */ + InitializerB obj = new InitializerB(); + } +} + +class InitializerA extends Initializer { + static { + System.out.println(3); + } + InitializerA() { + super(); + System.out.println(4); + } +} + +class InitializerB extends InitializerA { + static { + System.out.println(5); + } + InitializerB() { + super(); + System.out.println(6); + } +} \ No newline at end of file