From fd4727e70121c3bacaa5c5538a4f46feb634e7d7 Mon Sep 17 00:00:00 2001 From: Christoph Oelckers Date: Sat, 4 Mar 2017 10:28:51 +0100 Subject: [PATCH] - did a bit of cleanup. - disabled the runtime check in OP_CALL because as implemented it doesn't clean up properly and is not fully implemented. --- src/CMakeLists.txt | 1 + src/scripting/backend/codegen.cpp | 2 + src/scripting/backend/codegen.h | 176 +------------------------ src/scripting/backend/scopebarrier.cpp | 152 +++++++++++++++++++++ src/scripting/backend/scopebarrier.h | 52 ++++++++ src/scripting/vm/vmexec.h | 2 + 6 files changed, 210 insertions(+), 175 deletions(-) create mode 100644 src/scripting/backend/scopebarrier.cpp create mode 100644 src/scripting/backend/scopebarrier.h diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 4ff73e32c..8913ef6ea 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -1168,6 +1168,7 @@ set (PCH_SOURCES scripting/thingdef_data.cpp scripting/thingdef_properties.cpp scripting/backend/codegen.cpp + scripting/backend/scopebarrier.cpp scripting/backend/dynarrays.cpp scripting/backend/vmbuilder.cpp scripting/backend/vmdisasm.cpp diff --git a/src/scripting/backend/codegen.cpp b/src/scripting/backend/codegen.cpp index 1a212f5af..eb92fe974 100644 --- a/src/scripting/backend/codegen.cpp +++ b/src/scripting/backend/codegen.cpp @@ -8725,6 +8725,7 @@ ExpEmit FxVMFunctionCall::Emit(VMFunctionBuilder *build) ExpEmit selfemit; if (Function->Variants[0].Flags & VARF_Method) { +#if 0 // [ZZ] if (Function->Variants[0].Implementation && Function->Variants[0].Implementation->BarrierSide == FScopeBarrier::Side_Virtual) { @@ -8734,6 +8735,7 @@ ExpEmit FxVMFunctionCall::Emit(VMFunctionBuilder *build) build->Emit(OP_PARAM, 0, REGT_POINTER | REGT_KONST, build->GetConstantAddress(CallingFunction, ATAG_OBJECT)); count += 2; } +#endif assert(Self != nullptr); selfemit = Self->Emit(build); assert((selfemit.RegType == REGT_POINTER) || (selfemit.Fixed && selfemit.Target)); diff --git a/src/scripting/backend/codegen.h b/src/scripting/backend/codegen.h index 07425bd90..000688341 100644 --- a/src/scripting/backend/codegen.h +++ b/src/scripting/backend/codegen.h @@ -45,6 +45,7 @@ #include "s_sound.h" #include "actor.h" #include "vmbuilder.h" +#include "scopebarrier.h" #define CHECKRESOLVED() if (isresolved) return this; isresolved=true; @@ -70,181 +71,6 @@ class FxCompoundStatement; class FxLocalVariableDeclaration; typedef TDeletingArray FArgumentList; -// -// [ZZ] this really should be in codegen.h, but vmexec needs to access it -struct FScopeBarrier -{ - bool callable; - bool readable; - bool writable; - - // this is the error message - FString callerror; - FString readerror; - FString writeerror; - - // this is used to make the error message. - enum Side - { - Side_PlainData = 0, - Side_UI = 1, - Side_Play = 2, - Side_Virtual = 3, // do NOT change the value - Side_Clear = 4 - }; - int sidefrom; - int sidelast; - - // Note: the same object can't be both UI and Play. This is checked explicitly in the field construction and will cause esoteric errors here if found. - static int SideFromFlags(int flags) - { - if (flags & VARF_UI) - return Side_UI; - if (flags & VARF_Play) - return Side_Play; - if (flags & VARF_VirtualScope) - return Side_Virtual; - if (flags & VARF_ClearScope) - return Side_Clear; - return Side_PlainData; - } - - // same as above, but from object flags - static int SideFromObjectFlags(int flags) - { - if (flags & OF_UI) - return Side_UI; - if (flags & OF_Play) - return Side_Play; - return Side_PlainData; - } - - // - static int FlagsFromSide(int side) - { - switch (side) - { - case Side_Play: - return VARF_Play; - case Side_UI: - return VARF_UI; - case Side_Virtual: - return VARF_VirtualScope; - case Side_Clear: - return VARF_ClearScope; - default: - return 0; - } - } - - // used for errors - static const char* StringFromSide(int side) - { - switch (side) - { - case Side_PlainData: - return "data"; - case Side_UI: - return "ui"; - case Side_Play: - return "play"; - case Side_Virtual: - return "virtualscope"; // should not happen! - case Side_Clear: - return "clearscope"; // should not happen! - default: - return "unknown"; - } - } - - // this modifies VARF_ flags and sets the side properly. - static int ChangeSideInFlags(int flags, int side) - { - flags &= ~(VARF_UI | VARF_Play | VARF_VirtualScope | VARF_ClearScope); - flags |= FlagsFromSide(side); - return flags; - } - - FScopeBarrier() - { - sidefrom = -1; - sidelast = -1; - callable = true; - readable = true; - writable = true; - } - - FScopeBarrier(int flags1, int flags2, const char* name) - { - sidefrom = -1; - sidelast = -1; - callable = true; - readable = true; - writable = true; - - AddFlags(flags1, flags2, name); - } - - // AddFlags modifies ALLOWED actions by flags1->flags2. - // This is used for comparing a.b.c.d access - if non-allowed field is seen anywhere in the chain, anything after it is non-allowed. - // This struct is used so that the logic is in a single place. - void AddFlags(int flags1, int flags2, const char* name) - { - // note: if it's already non-readable, don't even try advancing - if (!readable) - return; - - // we aren't interested in any other flags - // - update: including VARF_VirtualScope. inside the function itself, we treat it as if it's PlainData. - flags1 &= VARF_UI | VARF_Play; - flags2 &= VARF_UI | VARF_Play | VARF_ReadOnly; - - if (sidefrom < 0) sidefrom = SideFromFlags(flags1); - if (sidelast < 0) sidelast = sidefrom; - - // flags1 = what's trying to access - // flags2 = what's being accessed - - int sideto = SideFromFlags(flags2); - - // plain data inherits whatever scope modifiers that context or field container has. - // i.e. play String bla; is play, and all non-specified methods/fields inside it are play as well. - if (sideto != Side_PlainData) - sidelast = sideto; - else sideto = sidelast; - - if ((sideto == Side_UI) && (sidefrom != Side_UI)) // only ui -> ui is readable - { - readable = false; - if (name) readerror.Format("Can't read %s field %s from %s context", StringFromSide(sideto), name, StringFromSide(sidefrom)); - } - - if (!readable) - { - writable = false; - callable = false; - if (name) - { - writeerror.Format("Can't write %s field %s from %s context (not readable)", StringFromSide(sideto), name, StringFromSide(sidefrom)); - callerror.Format("Can't call %s function %s from %s context (not readable)", StringFromSide(sideto), name, StringFromSide(sidefrom)); - } - return; - } - - if (writable && (sidefrom != sideto)) // only matching types are writable (plain data implicitly takes context type by default, unless overridden) - { - writable = false; - if (name) writeerror.Format("Can't write %s field %s from %s context", StringFromSide(sideto), name, StringFromSide(sidefrom)); - } - - if (callable && (sidefrom != sideto) && !(flags2 & VARF_ReadOnly)) // readonly on methods is used for plain data stuff that can be called from ui/play context. - { - callable = false; - if (name) callerror.Format("Can't call %s function %s from %s context", StringFromSide(sideto), name, StringFromSide(sidefrom)); - } - } -}; - struct FCompileContext { FxExpression *ControlStmt = nullptr; diff --git a/src/scripting/backend/scopebarrier.cpp b/src/scripting/backend/scopebarrier.cpp new file mode 100644 index 000000000..4d9d6d2fc --- /dev/null +++ b/src/scripting/backend/scopebarrier.cpp @@ -0,0 +1,152 @@ +#include "scopebarrier.h" +#include "dobject.h" + + +// Note: the same object can't be both UI and Play. This is checked explicitly in the field construction and will cause esoteric errors here if found. +int FScopeBarrier::SideFromFlags(int flags) +{ + if (flags & VARF_UI) + return Side_UI; + if (flags & VARF_Play) + return Side_Play; + if (flags & VARF_VirtualScope) + return Side_Virtual; + if (flags & VARF_ClearScope) + return Side_Clear; + return Side_PlainData; +} + +// same as above, but from object flags +int FScopeBarrier::SideFromObjectFlags(int flags) +{ + if (flags & OF_UI) + return Side_UI; + if (flags & OF_Play) + return Side_Play; + return Side_PlainData; +} + +// +int FScopeBarrier::FlagsFromSide(int side) +{ + switch (side) + { + case Side_Play: + return VARF_Play; + case Side_UI: + return VARF_UI; + case Side_Virtual: + return VARF_VirtualScope; + case Side_Clear: + return VARF_ClearScope; + default: + return 0; + } +} + +// used for errors +const char* FScopeBarrier::StringFromSide(int side) +{ + switch (side) + { + case Side_PlainData: + return "data"; + case Side_UI: + return "ui"; + case Side_Play: + return "play"; + case Side_Virtual: + return "virtualscope"; // should not happen! + case Side_Clear: + return "clearscope"; // should not happen! + default: + return "unknown"; + } +} + +// this modifies VARF_ flags and sets the side properly. +int FScopeBarrier::ChangeSideInFlags(int flags, int side) +{ + flags &= ~(VARF_UI | VARF_Play | VARF_VirtualScope | VARF_ClearScope); + flags |= FlagsFromSide(side); + return flags; +} + +FScopeBarrier::FScopeBarrier() +{ + sidefrom = -1; + sidelast = -1; + callable = true; + readable = true; + writable = true; +} + +FScopeBarrier::FScopeBarrier(int flags1, int flags2, const char* name) +{ + sidefrom = -1; + sidelast = -1; + callable = true; + readable = true; + writable = true; + + AddFlags(flags1, flags2, name); +} + +// AddFlags modifies ALLOWED actions by flags1->flags2. +// This is used for comparing a.b.c.d access - if non-allowed field is seen anywhere in the chain, anything after it is non-allowed. +// This struct is used so that the logic is in a single place. +void FScopeBarrier::AddFlags(int flags1, int flags2, const char* name) +{ + // note: if it's already non-readable, don't even try advancing + if (!readable) + return; + + // we aren't interested in any other flags + // - update: including VARF_VirtualScope. inside the function itself, we treat it as if it's PlainData. + flags1 &= VARF_UI | VARF_Play; + flags2 &= VARF_UI | VARF_Play | VARF_ReadOnly; + + if (sidefrom < 0) sidefrom = SideFromFlags(flags1); + if (sidelast < 0) sidelast = sidefrom; + + // flags1 = what's trying to access + // flags2 = what's being accessed + + int sideto = SideFromFlags(flags2); + + // plain data inherits whatever scope modifiers that context or field container has. + // i.e. play String bla; is play, and all non-specified methods/fields inside it are play as well. + if (sideto != Side_PlainData) + sidelast = sideto; + else sideto = sidelast; + + if ((sideto == Side_UI) && (sidefrom != Side_UI)) // only ui -> ui is readable + { + readable = false; + if (name) readerror.Format("Can't read %s field %s from %s context", StringFromSide(sideto), name, StringFromSide(sidefrom)); + } + + if (!readable) + { + writable = false; + callable = false; + if (name) + { + writeerror.Format("Can't write %s field %s from %s context (not readable)", StringFromSide(sideto), name, StringFromSide(sidefrom)); + callerror.Format("Can't call %s function %s from %s context (not readable)", StringFromSide(sideto), name, StringFromSide(sidefrom)); + } + return; + } + + if (writable && (sidefrom != sideto)) // only matching types are writable (plain data implicitly takes context type by default, unless overridden) + { + writable = false; + if (name) writeerror.Format("Can't write %s field %s from %s context", StringFromSide(sideto), name, StringFromSide(sidefrom)); + } + + if (callable && (sidefrom != sideto) && !(flags2 & VARF_ReadOnly)) // readonly on methods is used for plain data stuff that can be called from ui/play context. + { + callable = false; + if (name) callerror.Format("Can't call %s function %s from %s context", StringFromSide(sideto), name, StringFromSide(sidefrom)); + } +} diff --git a/src/scripting/backend/scopebarrier.h b/src/scripting/backend/scopebarrier.h new file mode 100644 index 000000000..ed4f25a10 --- /dev/null +++ b/src/scripting/backend/scopebarrier.h @@ -0,0 +1,52 @@ +#pragma once + +#include "zstring.h" + +// +// [ZZ] this really should be in codegen.h, but vmexec needs to access it +struct FScopeBarrier +{ + bool callable; + bool readable; + bool writable; + + // this is the error message + FString callerror; + FString readerror; + FString writeerror; + + // this is used to make the error message. + enum Side + { + Side_PlainData = 0, + Side_UI = 1, + Side_Play = 2, + Side_Virtual = 3, // do NOT change the value + Side_Clear = 4 + }; + int sidefrom; + int sidelast; + + // Note: the same object can't be both UI and Play. This is checked explicitly in the field construction and will cause esoteric errors here if found. + static int SideFromFlags(int flags); + + // same as above, but from object flags + static int SideFromObjectFlags(int flags); + + // + static int FlagsFromSide(int side); + + // used for errors + static const char* StringFromSide(int side); + + // this modifies VARF_ flags and sets the side properly. + static int ChangeSideInFlags(int flags, int side); + FScopeBarrier(); + FScopeBarrier(int flags1, int flags2, const char* name); + + // AddFlags modifies ALLOWED actions by flags1->flags2. + // This is used for comparing a.b.c.d access - if non-allowed field is seen anywhere in the chain, anything after it is non-allowed. + // This struct is used so that the logic is in a single place. + void AddFlags(int flags1, int flags2, const char* name); +}; + diff --git a/src/scripting/vm/vmexec.h b/src/scripting/vm/vmexec.h index b3d1b2426..c11b351c0 100644 --- a/src/scripting/vm/vmexec.h +++ b/src/scripting/vm/vmexec.h @@ -664,6 +664,7 @@ begin: VMReturn returns[MAX_RETURNS]; int numret; +#if 0 // [ZZ] hax! b = B; if (call->BarrierSide == 3) // :( - this is Side_Virtual. Side_Virtual should receive special arguments. @@ -675,6 +676,7 @@ begin: FScopeBarrier_ValidateCall(calledfunc, callingfunc, selftype); b -= 2; } +#endif FillReturns(reg, f, returns, pc+1, C); if (call->Native)