From 5193b61fc9be0133757ffc72af8a24b4a9ee00fb Mon Sep 17 00:00:00 2001 From: Stefan Marr Date: Sun, 11 Aug 2024 19:14:41 +0100 Subject: [PATCH 1/2] Move VMFrame creation mostly into the constructor Signed-off-by: Stefan Marr --- src/unitTests/WalkObjectsTest.cpp | 4 ++-- src/unitTests/WriteBarrierTest.cpp | 3 --- src/vm/IsValidObject.cpp | 6 +++--- src/vm/Universe.cpp | 6 +----- src/vmobjects/VMFrame.cpp | 25 +---------------------- src/vmobjects/VMFrame.h | 32 ++++++++++++++++-------------- 6 files changed, 24 insertions(+), 52 deletions(-) diff --git a/src/unitTests/WalkObjectsTest.cpp b/src/unitTests/WalkObjectsTest.cpp index 644e3cc3..cb11c196 100644 --- a/src/unitTests/WalkObjectsTest.cpp +++ b/src/unitTests/WalkObjectsTest.cpp @@ -154,8 +154,8 @@ void WalkObjectsTest::testWalkFrame() { Universe::NewMethod(methodSymbol, 0, 0, 0, 0, new LexicalScope(nullptr, {}, {}), inlinedLoops); - VMFrame* frame = Universe::NewFrame(nullptr, method); - frame->SetPreviousFrame(frame->CloneForMovingGC()); + VMFrame* prev = Universe::NewFrame(nullptr, method); + VMFrame* frame = Universe::NewFrame(prev, method); frame->SetContext(frame->CloneForMovingGC()); VMInteger* dummyArg = Universe::NewInteger(1111); frame->SetArgument(0, 0, dummyArg); diff --git a/src/unitTests/WriteBarrierTest.cpp b/src/unitTests/WriteBarrierTest.cpp index 22e3b677..1526f50c 100644 --- a/src/unitTests/WriteBarrierTest.cpp +++ b/src/unitTests/WriteBarrierTest.cpp @@ -106,9 +106,6 @@ void WriteBarrierTest::testWriteFrame() { VMFrame* frame = Interpreter::GetFrame()->CloneForMovingGC(); frame->SetContext(frame->CloneForMovingGC()); - frame->SetPreviousFrame(Interpreter::GetFrame()); - TEST_WB_CALLED("VMFrame failed to call writeBarrier on SetPreviousFrame", - frame, Interpreter::GetFrame()); frame->SetContext(frame->GetContext()->CloneForMovingGC()); TEST_WB_CALLED("VMFrame failed to call writeBarrier on SetContext", frame, frame->GetContext()); diff --git a/src/vm/IsValidObject.cpp b/src/vm/IsValidObject.cpp index 592160bc..a0583c4a 100644 --- a/src/vm/IsValidObject.cpp +++ b/src/vm/IsValidObject.cpp @@ -182,9 +182,6 @@ void obtain_vtables_of_known_classes(VMSymbol* someValidSymbol) { new (GetHeap(), 0) VMEvaluationPrimitive(1); vt_eval_primitive = get_vtable(ev); - VMFrame* frm = new (GetHeap(), 0) VMFrame(0, 0); - vt_frame = get_vtable(frm); - VMInteger* i = new (GetHeap(), 0) VMInteger(0); vt_integer = get_vtable(i); @@ -193,6 +190,9 @@ void obtain_vtables_of_known_classes(VMSymbol* someValidSymbol) { vt_method = get_vtable(mth); vt_object = get_vtable(load_ptr(nilObject)); + VMFrame* frm = new (GetHeap(), 0) VMFrame(0, mth, nullptr); + vt_frame = get_vtable(frm); + VMPrimitive* prm = new (GetHeap(), 0) VMPrimitive(someValidSymbol, FramePrim()); vt_primitive = get_vtable(prm); diff --git a/src/vm/Universe.cpp b/src/vm/Universe.cpp index 83ed4df3..0a16c583 100644 --- a/src/vm/Universe.cpp +++ b/src/vm/Universe.cpp @@ -720,11 +720,7 @@ VMFrame* Universe::NewFrame(VMFrame* previousFrame, VMMethod* method) { method->GetMaximumNumberOfStackElements(); size_t additionalBytes = length * sizeof(VMObject*); - result = new (GetHeap(), additionalBytes) - VMFrame(length, additionalBytes); - result->method = store_root(method); - result->previousFrame = store_root(previousFrame); - result->ResetStackPointer(); + result = new (GetHeap(), additionalBytes) VMFrame(additionalBytes, method, previousFrame); LOG_ALLOCATION("VMFrame", result->GetObjectSize()); return result; diff --git a/src/vmobjects/VMFrame.cpp b/src/vmobjects/VMFrame.cpp index 920b90b3..ff911c89 100644 --- a/src/vmobjects/VMFrame.cpp +++ b/src/vmobjects/VMFrame.cpp @@ -53,13 +53,11 @@ VMFrame* VMFrame::EmergencyFrameFrom(VMFrame* from, long extraLength) { size_t additionalBytes = length * sizeof(VMObject*); VMFrame* result = new (GetHeap(), additionalBytes) - VMFrame(length, additionalBytes); + VMFrame(additionalBytes, method, from->GetPreviousFrame()); result->clazz = nullptr; // result->SetClass(from->GetClass()); // set Frame members - result->SetPreviousFrame(from->GetPreviousFrame()); - result->SetMethod(method); result->SetContext(from->GetContext()); result->stack_ptr = (gc_oop_t*)SHIFTED_PTR(result, (size_t)from->stack_ptr - (size_t)from); @@ -124,27 +122,6 @@ VMFrame* VMFrame::CloneForMovingGC() const { const long VMFrame::VMFrameNumberOfFields = 0; -VMFrame::VMFrame(size_t, size_t additionalBytes) - : VMObject(0, additionalBytes + sizeof(VMFrame)), bytecodeIndex(0), - previousFrame(nullptr), context(nullptr), method(nullptr) { - arguments = (gc_oop_t*)&(stack_ptr) + 1; - locals = arguments; - stack_ptr = locals; - - // initilize all other fields - // --> until end of Frame - gc_oop_t* end = (gc_oop_t*)SHIFTED_PTR(this, totalObjectSize); - size_t i = 0; - while (arguments + i < end) { - arguments[i] = nilObject; - i++; - } -} - -void VMFrame::SetMethod(VMMethod* method) { - store_ptr(this->method, method); -} - VMFrame* VMFrame::GetContextLevel(long lvl) { VMFrame* current = this; while (lvl > 0) { diff --git a/src/vmobjects/VMFrame.h b/src/vmobjects/VMFrame.h index eaa3dc4f..0a0e92b0 100644 --- a/src/vmobjects/VMFrame.h +++ b/src/vmobjects/VMFrame.h @@ -42,10 +42,25 @@ class VMFrame : public VMObject { static VMFrame* EmergencyFrameFrom(VMFrame* from, long extraLength); - explicit VMFrame(size_t size, size_t additionalBytes); + explicit VMFrame(size_t additionalBytes, VMMethod* method, + VMFrame* previousFrame) + : VMObject(0, additionalBytes + sizeof(VMFrame)), bytecodeIndex(0), + previousFrame(store_root(previousFrame)), context(nullptr), + method(store_root(method)), arguments((gc_oop_t*)&(stack_ptr) + 1), + locals(arguments + method->GetNumberOfArguments()), + stack_ptr(locals + method->GetNumberOfLocals() - 1) { + // initilize all other fields. Don't need to initalize arguments, + // because they iwll be copied in still + // --> until end of Frame + gc_oop_t* end = (gc_oop_t*)SHIFTED_PTR(this, totalObjectSize); + size_t i = 0; + while (arguments + i < end) { + arguments[i] = nilObject; + i++; + } + } inline VMFrame* GetPreviousFrame() const; - inline void SetPreviousFrame(VMFrame*); inline void ClearPreviousFrame(); inline bool HasPreviousFrame() const; inline bool IsBootstrapFrame() const; @@ -55,7 +70,6 @@ class VMFrame : public VMObject { VMFrame* GetContextLevel(long); VMFrame* GetOuterContext(); inline VMMethod* GetMethod() const; - void SetMethod(VMMethod*); inline vm_oop_t Pop() { vm_oop_t result = load_ptr(*stack_ptr); @@ -83,14 +97,6 @@ class VMFrame : public VMObject { store_ptr(*stack_ptr, obj); } - void ResetStackPointer() { - // arguments are stored in front of local variables - VMMethod* meth = GetMethod(); - locals = arguments + meth->GetNumberOfArguments(); - // Set the stack pointer to its initial value thereby clearing the stack - stack_ptr = locals + meth->GetNumberOfLocals() - 1; - } - inline long GetBytecodeIndex() const; inline vm_oop_t GetStackElement(long index) const { @@ -192,10 +198,6 @@ VMFrame* VMFrame::GetPreviousFrame() const { return load_ptr(previousFrame); } -void VMFrame::SetPreviousFrame(VMFrame* frm) { - store_ptr(previousFrame, frm); -} - void VMFrame::ClearPreviousFrame() { previousFrame = nullptr; } From 4ce1f2187c79f92efda63095dc9b7bcf2c5b21be Mon Sep 17 00:00:00 2001 From: Stefan Marr Date: Sun, 11 Aug 2024 20:05:41 +0100 Subject: [PATCH 2/2] =?UTF-8?q?Don=E2=80=99t=20initialize=20arguments=20on?= =?UTF-8?q?=20frame=20creation,=20only=20the=20stack=20and=20locals?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Though, need to make sure I initialize the bootstrap frame arguments. Otherwise, we may run into uninitialized memory. Signed-off-by: Stefan Marr --- src/vm/Universe.cpp | 8 +++++++- src/vmobjects/VMFrame.h | 4 ++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/vm/Universe.cpp b/src/vm/Universe.cpp index 0a16c583..e88c350b 100644 --- a/src/vm/Universe.cpp +++ b/src/vm/Universe.cpp @@ -302,6 +302,11 @@ vm_oop_t Universe::interpretMethod(VMObject* receiver, VMInvokable* initialize, VMMethod* bootstrapMethod = createBootstrapMethod(load_ptr(systemClass), 2); VMFrame* bootstrapFrame = Interpreter::PushNewFrame(bootstrapMethod); + for (size_t argIdx = 0; argIdx < bootstrapMethod->GetNumberOfArguments(); + argIdx += 1) { + bootstrapFrame->SetArgument((long)argIdx, (long)0, load_ptr(nilObject)); + } + bootstrapFrame->Push(receiver); if (argumentsArray != nullptr) { @@ -720,7 +725,8 @@ VMFrame* Universe::NewFrame(VMFrame* previousFrame, VMMethod* method) { method->GetMaximumNumberOfStackElements(); size_t additionalBytes = length * sizeof(VMObject*); - result = new (GetHeap(), additionalBytes) VMFrame(additionalBytes, method, previousFrame); + result = new (GetHeap(), additionalBytes) + VMFrame(additionalBytes, method, previousFrame); LOG_ALLOCATION("VMFrame", result->GetObjectSize()); return result; diff --git a/src/vmobjects/VMFrame.h b/src/vmobjects/VMFrame.h index 0a0e92b0..a6e853d9 100644 --- a/src/vmobjects/VMFrame.h +++ b/src/vmobjects/VMFrame.h @@ -54,8 +54,8 @@ class VMFrame : public VMObject { // --> until end of Frame gc_oop_t* end = (gc_oop_t*)SHIFTED_PTR(this, totalObjectSize); size_t i = 0; - while (arguments + i < end) { - arguments[i] = nilObject; + while (locals + i < end) { + locals[i] = nilObject; i++; } }