From f810b981672a082ef3543fd2df0b0d70bde484f3 Mon Sep 17 00:00:00 2001 From: Christoph Oelckers Date: Mon, 24 Oct 2016 17:18:20 +0200 Subject: [PATCH] - implement flag variables with the VM's sbit and lbit instructions. - synthesize PField entries from the flag list for AActor. This intentionally excludes the bounce flags for now. - allow deprecated flags that do not call the deprecated flag handler. - disallow constructs like (a = b) = c by not allowing an address request on an assignment operation. - restrict modify/assign on boolean variables to the bit operators. Everything else needs to promote the result to an integer to make sense so it should be disallowed. --- src/dobjtype.cpp | 25 +++++ src/dobjtype.h | 5 +- src/scripting/codegeneration/codegen.cpp | 135 ++++++++++++++++------- src/scripting/codegeneration/codegen.h | 9 +- src/scripting/decorate/thingdef_exp.cpp | 2 +- src/scripting/thingdef.h | 1 + src/scripting/thingdef_data.cpp | 46 +++++--- src/scripting/zscript/zcc_compile.cpp | 7 +- 8 files changed, 166 insertions(+), 64 deletions(-) diff --git a/src/dobjtype.cpp b/src/dobjtype.cpp index 9191c60c00..3bce521c8a 100644 --- a/src/dobjtype.cpp +++ b/src/dobjtype.cpp @@ -2479,6 +2479,31 @@ PField::PField() { } +PField::PField(FName name, PType *type, DWORD flags, size_t offset, int bitvalue) + : PSymbol(name), Offset(unsigned(offset)), Type(type), Flags(flags) +{ + BitValue = bitvalue; + if (bitvalue > -1) + { + if (type->IsA(RUNTIME_CLASS(PInt)) && unsigned(bitvalue) < 8 * type->Size) + { + // map to the single bytes in the actual variable. The internal bit instructions operate on 8 bit values. +#ifndef __BIG_ENDIAN__ + Offset += BitValue / 8; +#else + Offset += type->Size - 1 - BitValue / 8; +#endif + BitValue &= 7; + Type = TypeBool; + } + else + { + // Just abort. Bit fields should only be defined internally. + I_FatalError("Trying to create an invalid bit field element: %s", name.GetChars()); + } + } +} + /* PPrototype *************************************************************/ IMPLEMENT_CLASS(PPrototype) diff --git a/src/dobjtype.h b/src/dobjtype.h index f3e043bd92..0c26f3a358 100644 --- a/src/dobjtype.h +++ b/src/dobjtype.h @@ -572,13 +572,12 @@ class PField : public PSymbol DECLARE_CLASS(PField, PSymbol); HAS_OBJECT_POINTERS public: - PField(FName name, PType *type) : PSymbol(name), Offset(0), Type(type), Flags(0) {} - PField(FName name, PType *type, DWORD flags) : PSymbol(name), Offset(0), Type(type), Flags(flags) {} - PField(FName name, PType *type, DWORD flags, size_t offset) : PSymbol(name), Offset(unsigned(offset)), Type(type), Flags(flags) {} + PField(FName name, PType *type, DWORD flags = 0, size_t offset = 0, int bitvalue = -1); unsigned int Offset; PType *Type; DWORD Flags; + int BitValue; protected: PField(); }; diff --git a/src/scripting/codegeneration/codegen.cpp b/src/scripting/codegeneration/codegen.cpp index bcea7e9fa5..c8a682189a 100644 --- a/src/scripting/codegeneration/codegen.cpp +++ b/src/scripting/codegeneration/codegen.cpp @@ -1768,8 +1768,8 @@ ExpEmit FxPostIncrDecr::Emit(VMFunctionBuilder *build) // //========================================================================== -FxAssign::FxAssign(FxExpression *base, FxExpression *right) -: FxExpression(EFX_Assign, base->ScriptPosition), Base(base), Right(right) +FxAssign::FxAssign(FxExpression *base, FxExpression *right, bool ismodify) +: FxExpression(EFX_Assign, base->ScriptPosition), Base(base), Right(right), IsModifyAssign(ismodify), IsBitWrite(-1) { AddressRequested = false; AddressWritable = false; @@ -1781,12 +1781,14 @@ FxAssign::~FxAssign() SAFE_DELETE(Right); } +/* I don't think we should allow constructs like (a = b) = c;... bool FxAssign::RequestAddress(bool *writable) { AddressRequested = true; if (writable != nullptr) *writable = AddressWritable; return true; } +*/ FxExpression *FxAssign::Resolve(FCompileContext &ctx) { @@ -1797,6 +1799,15 @@ FxExpression *FxAssign::Resolve(FCompileContext &ctx) SAFE_RESOLVE(Right, ctx); + if (IsModifyAssign && Base->ValueType == TypeBool && Right->ValueType != TypeBool) + { + // If the modify operation resulted in a type promotion from bool to int, this must be blocked. + // (this means, for bool, only &=, ^= and |= are allowed, although DECORATE is more lax.) + ScriptPosition.Message(MSG_ERROR, "Invalid modify/assign operation with a boolean operand"); + delete this; + return nullptr; + } + if (Base->IsNumeric() && Right->IsNumeric()) { if (Right->ValueType != ValueType) @@ -1865,6 +1876,16 @@ FxExpression *FxAssign::Resolve(FCompileContext &ctx) return nullptr; } + // Special case: Assignment to a bitfield. + if (Base->ExprType == EFX_StructMember || Base->ExprType == EFX_ClassMember) + { + auto f = static_cast(Base)->membervar; + if (f->BitValue != -1 && !(f->Flags & VARF_ReadOnly)) + { + IsBitWrite = f->BitValue; + return this; + } + } return this; } @@ -1902,7 +1923,15 @@ ExpEmit FxAssign::Emit(VMFunctionBuilder *build) result = temp; } - build->Emit(ValueType->GetStoreOp(), pointer.RegNum, result.RegNum, build->GetConstantInt(0)); + if (IsBitWrite == -1) + { + build->Emit(ValueType->GetStoreOp(), pointer.RegNum, result.RegNum, build->GetConstantInt(0)); + } + else + { + build->Emit(OP_SBIT, pointer.RegNum, result.RegNum, 1 << IsBitWrite); + } + } if (AddressRequested) @@ -1941,12 +1970,19 @@ FxExpression *FxAssignSelf::Resolve(FCompileContext &ctx) ExpEmit FxAssignSelf::Emit(VMFunctionBuilder *build) { - assert(ValueType = Assignment->ValueType); + assert(ValueType == Assignment->ValueType); ExpEmit pointer = Assignment->Address; // FxAssign should have already emitted it if (!pointer.Target) { ExpEmit out(build, ValueType->GetRegType()); - build->Emit(ValueType->GetLoadOp(), out.RegNum, pointer.RegNum, build->GetConstantInt(0)); + if (Assignment->IsBitWrite != -1) + { + build->Emit(OP_LBIT, out.RegNum, pointer.RegNum, 1 << Assignment->IsBitWrite); + } + else + { + build->Emit(ValueType->GetLoadOp(), out.RegNum, pointer.RegNum, build->GetConstantInt(0)); + } return out; } else @@ -1999,11 +2035,29 @@ bool FxBinary::ResolveLR(FCompileContext& ctx, bool castnumeric) if (left->ValueType == TypeBool && right->ValueType == TypeBool) { - ValueType = TypeBool; + if (Operator == '&' || Operator == '|' || Operator == '^' || ctx.FromDecorate) + { + ValueType = TypeBool; + } + else + { + ValueType = TypeSInt32; // math operations on bools result in integers. + } } else if (left->ValueType == TypeName && right->ValueType == TypeName) { - ValueType = TypeName; + // pointers can only be compared for equality. + if (Operator == TK_Eq || Operator == TK_Neq) + { + ValueType = TypeBool; + return true; + } + else + { + ScriptPosition.Message(MSG_ERROR, "Invalid operation for names"); + delete this; + return false; + } } else if (left->IsNumeric() && right->IsNumeric()) { @@ -2022,13 +2076,25 @@ bool FxBinary::ResolveLR(FCompileContext& ctx, bool castnumeric) AreCompatiblePointerTypes(left->ValueType, right->ValueType)) { // pointers can only be compared for equality. - assert(Operator == TK_Eq || Operator == TK_Neq); - ValueType = TypeBool; + if (Operator == TK_Eq || Operator == TK_Neq) + { + ValueType = TypeBool; + return true; + } + else + { + ScriptPosition.Message(MSG_ERROR, "Invalid operation for pointers"); + delete this; + return false; + } } } else { - ValueType = TypeVoid; + // To check: It may be that this could pass in DECORATE, although setting TypeVoid here would pretty much prevent that. + ScriptPosition.Message(MSG_ERROR, "Incompatible operator"); + delete this; + return false; } assert(ValueType > nullptr && ValueType < (PType*)0xfffffffffffffff); @@ -2077,7 +2143,7 @@ FxExpression *FxAddSub::Resolve(FCompileContext& ctx) { ScriptPosition.Message(MSG_ERROR, "Numeric type expected"); delete this; - return NULL; + return nullptr; } else if (left->isConstant() && right->isConstant()) { @@ -2771,7 +2837,7 @@ FxLtGtEq::FxLtGtEq(FxExpression *l, FxExpression *r) FxExpression *FxLtGtEq::Resolve(FCompileContext& ctx) { CHECKRESOLVED(); - if (!ResolveLR(ctx, false)) return NULL; + if (!ResolveLR(ctx, true)) return NULL; if (!left->IsNumeric() || !right->IsNumeric()) { @@ -4456,21 +4522,21 @@ FxExpression *FxStructMember::Resolve(FCompileContext &ctx) if (classx->ValueType->IsKindOf(RUNTIME_CLASS(PPointer))) { PPointer *ptrtype = dyn_cast(classx->ValueType); - if (ptrtype == nullptr || !ptrtype->PointedType->IsA(RUNTIME_CLASS(PStruct))) + if (ptrtype == nullptr || !ptrtype->PointedType->IsKindOf(RUNTIME_CLASS(PStruct))) { - ScriptPosition.Message(MSG_ERROR, "Member variable requires a struct"); + ScriptPosition.Message(MSG_ERROR, "Member variable requires a struct or class object."); delete this; return nullptr; } } - else if (classx->ValueType->IsA(RUNTIME_CLASS(PStruct))) + else if (classx->ValueType->IsA(RUNTIME_CLASS(PStruct))) // Classes can never be used as value types so we do not have to consider that case. { // if this is a struct within a class or another struct we can simplify the expression by creating a new PField with a cumulative offset. if (classx->ExprType == EFX_ClassMember || classx->ExprType == EFX_StructMember) { auto parentfield = static_cast(classx)->membervar; // PFields are garbage collected so this will be automatically taken care of later. - auto newfield = new PField(membervar->SymbolName, membervar->Type, membervar->Flags | parentfield->Flags, membervar->Offset + parentfield->Offset); + auto newfield = new PField(membervar->SymbolName, membervar->Type, membervar->Flags | parentfield->Flags, membervar->Offset + parentfield->Offset, membervar->BitValue); static_cast(classx)->membervar = newfield; classx->isresolved = false; // re-resolve the parent so it can also check if it can be optimized away. auto x = classx->Resolve(ctx); @@ -4522,7 +4588,17 @@ ExpEmit FxStructMember::Emit(VMFunctionBuilder *build) int offsetreg = build->GetConstantInt((int)membervar->Offset); ExpEmit loc(build, membervar->Type->GetRegType()); - build->Emit(membervar->Type->GetLoadOp(), loc.RegNum, obj.RegNum, offsetreg); + if (membervar->BitValue == -1) + { + build->Emit(membervar->Type->GetLoadOp(), loc.RegNum, obj.RegNum, offsetreg); + } + else + { + ExpEmit out(build, REGT_POINTER); + build->Emit(OP_ADDA_RK, out.RegNum, obj.RegNum, offsetreg); + build->Emit(OP_LBIT, loc.RegNum, out.RegNum, 1 << membervar->BitValue); + out.Free(build); + } obj.Free(build); return loc; } @@ -4530,7 +4606,8 @@ ExpEmit FxStructMember::Emit(VMFunctionBuilder *build) //========================================================================== // -// +// not really needed at the moment but may become useful with meta properties +// and some other class-specific extensions. // //========================================================================== @@ -4547,28 +4624,6 @@ FxClassMember::FxClassMember(FxExpression *x, PField* mem, const FScriptPosition // //========================================================================== -FxExpression *FxClassMember::Resolve(FCompileContext &ctx) -{ - CHECKRESOLVED(); - SAFE_RESOLVE(classx, ctx); - - PPointer *ptrtype = dyn_cast(classx->ValueType); - if (ptrtype == NULL || !ptrtype->PointedType->IsKindOf(RUNTIME_CLASS(PClass))) - { - ScriptPosition.Message(MSG_ERROR, "Member variable requires a class or object"); - delete this; - return NULL; - } - ValueType = membervar->Type; - return this; -} - -//========================================================================== -// -// -// -//========================================================================== - FxArrayElement::FxArrayElement(FxExpression *base, FxExpression *_index) :FxExpression(EFX_ArrayElement, base->ScriptPosition) { diff --git a/src/scripting/codegeneration/codegen.h b/src/scripting/codegeneration/codegen.h index 771faa2c8a..3fb1b15761 100644 --- a/src/scripting/codegeneration/codegen.h +++ b/src/scripting/codegeneration/codegen.h @@ -703,14 +703,18 @@ class FxAssign : public FxExpression { FxExpression *Base; FxExpression *Right; + int IsBitWrite; bool AddressRequested; bool AddressWritable; + bool IsModifyAssign; + + friend class FxAssignSelf; public: - FxAssign(FxExpression *base, FxExpression *right); + FxAssign(FxExpression *base, FxExpression *right, bool ismodify = false); ~FxAssign(); FxExpression *Resolve(FCompileContext&); - bool RequestAddress(bool *writable); + //bool RequestAddress(bool *writable); ExpEmit Emit(VMFunctionBuilder *build); ExpEmit Address; @@ -1064,7 +1068,6 @@ class FxClassMember : public FxStructMember public: FxClassMember(FxExpression*, PField*, const FScriptPosition&); - FxExpression *Resolve(FCompileContext&); }; //========================================================================== diff --git a/src/scripting/decorate/thingdef_exp.cpp b/src/scripting/decorate/thingdef_exp.cpp index 2d1d189b4a..84e1200e9e 100644 --- a/src/scripting/decorate/thingdef_exp.cpp +++ b/src/scripting/decorate/thingdef_exp.cpp @@ -170,7 +170,7 @@ static FxExpression *ParseExpressionM (FScanner &sc, PClassActor *cls) exp->right = ParseExpressionM(sc, cls); - FxAssign *ret = new FxAssign(base, exp); + FxAssign *ret = new FxAssign(base, exp, true); left->Assignment = ret; return ret; } diff --git a/src/scripting/thingdef.h b/src/scripting/thingdef.h index 51df4821ea..a8c9b0b9be 100644 --- a/src/scripting/thingdef.h +++ b/src/scripting/thingdef.h @@ -24,6 +24,7 @@ struct FFlagDef const char *name; int structoffset; int fieldsize; + bool deprecated; }; FFlagDef *FindFlag (const PClass *type, const char *part1, const char *part2, bool strict = false); diff --git a/src/scripting/thingdef_data.cpp b/src/scripting/thingdef_data.cpp index 03e88b1492..063b6e2042 100644 --- a/src/scripting/thingdef_data.cpp +++ b/src/scripting/thingdef_data.cpp @@ -54,10 +54,11 @@ static TArray AFTable; //========================================================================== // [RH] Keep GCC quiet by not using offsetof on Actor types. -#define DEFINE_FLAG(prefix, name, type, variable) { (unsigned int)prefix##_##name, #name, (int)(size_t)&((type*)1)->variable - 1, sizeof(((type *)0)->variable) } -#define DEFINE_FLAG2(symbol, name, type, variable) { (unsigned int)symbol, #name, (int)(size_t)&((type*)1)->variable - 1, sizeof(((type *)0)->variable) } -#define DEFINE_DEPRECATED_FLAG(name) { DEPF_##name, #name, -1, 0 } -#define DEFINE_DUMMY_FLAG(name) { DEPF_UNUSED, #name, -1, 0 } +#define DEFINE_FLAG(prefix, name, type, variable) { (unsigned int)prefix##_##name, #name, (int)(size_t)&((type*)1)->variable - 1, sizeof(((type *)0)->variable), false } +#define DEFINE_FLAG2(symbol, name, type, variable) { (unsigned int)symbol, #name, (int)(size_t)&((type*)1)->variable - 1, sizeof(((type *)0)->variable), false } +#define DEFINE_FLAG2_DEPRECATED(symbol, name, type, variable) { (unsigned int)symbol, #name, (int)(size_t)&((type*)1)->variable - 1, sizeof(((type *)0)->variable), true } +#define DEFINE_DEPRECATED_FLAG(name) { DEPF_##name, #name, -1, 0, true } +#define DEFINE_DUMMY_FLAG(name, deprec) { DEPF_UNUSED, #name, -1, 0, deprec } static FFlagDef ActorFlagDefs[]= { @@ -171,7 +172,6 @@ static FFlagDef ActorFlagDefs[]= DEFINE_FLAG(MF4, SYNCHRONIZED, AActor, flags4), DEFINE_FLAG(MF4, NOTARGETSWITCH, AActor, flags4), DEFINE_FLAG(MF4, DONTHARMCLASS, AActor, flags4), - DEFINE_FLAG2(MF4_DONTHARMCLASS, DONTHURTSPECIES, AActor, flags4), // Deprecated name as an alias DEFINE_FLAG(MF4, SHIELDREFLECT, AActor, flags4), DEFINE_FLAG(MF4, DEFLECT, AActor, flags4), DEFINE_FLAG(MF4, ALLOWPARTICLES, AActor, flags4), @@ -278,7 +278,11 @@ static FFlagDef ActorFlagDefs[]= DEFINE_FLAG(RF, MASKROTATION, AActor, renderflags), DEFINE_FLAG(RF, ABSMASKANGLE, AActor, renderflags), DEFINE_FLAG(RF, ABSMASKPITCH, AActor, renderflags), +}; +// These won't be accessible through bitfield variables +static FFlagDef MoreFlagDefs[] = +{ // Bounce flags DEFINE_FLAG2(BOUNCE_Walls, BOUNCEONWALLS, AActor, BounceFlags), DEFINE_FLAG2(BOUNCE_Floors, BOUNCEONFLOORS, AActor, BounceFlags), @@ -308,15 +312,16 @@ static FFlagDef ActorFlagDefs[]= DEFINE_DEPRECATED_FLAG(DOOMBOUNCE), // Deprecated flags with no more existing functionality. - DEFINE_DUMMY_FLAG(FASTER), // obsolete, replaced by 'Fast' state flag - DEFINE_DUMMY_FLAG(FASTMELEE), // obsolete, replaced by 'Fast' state flag + DEFINE_DUMMY_FLAG(FASTER, true), // obsolete, replaced by 'Fast' state flag + DEFINE_DUMMY_FLAG(FASTMELEE, true), // obsolete, replaced by 'Fast' state flag // Various Skulltag flags that are quite irrelevant to ZDoom - DEFINE_DUMMY_FLAG(NONETID), // netcode-based - DEFINE_DUMMY_FLAG(ALLOWCLIENTSPAWN), // netcode-based - DEFINE_DUMMY_FLAG(CLIENTSIDEONLY), // netcode-based - DEFINE_DUMMY_FLAG(SERVERSIDEONLY), // netcode-based - DEFINE_DUMMY_FLAG(EXPLODEONDEATH), // seems useless + DEFINE_DUMMY_FLAG(NONETID, false), // netcode-based + DEFINE_DUMMY_FLAG(ALLOWCLIENTSPAWN, false), // netcode-based + DEFINE_DUMMY_FLAG(CLIENTSIDEONLY, false), // netcode-based + DEFINE_DUMMY_FLAG(SERVERSIDEONLY, false), // netcode-based + DEFINE_DUMMY_FLAG(EXPLODEONDEATH, true), // seems useless + DEFINE_FLAG2_DEPRECATED(MF4_DONTHARMCLASS, DONTHURTSPECIES, AActor, flags4), // Deprecated name as an alias }; static FFlagDef InventoryFlagDefs[] = @@ -372,9 +377,9 @@ static FFlagDef WeaponFlagDefs[] = DEFINE_FLAG(WIF, NODEATHDESELECT, AWeapon, WeaponFlags), DEFINE_FLAG(WIF, NODEATHINPUT, AWeapon, WeaponFlags), - DEFINE_DUMMY_FLAG(NOLMS), + DEFINE_DUMMY_FLAG(NOLMS, false), DEFINE_FLAG(WIF, ALT_USES_BOTH, AWeapon, WeaponFlags), - DEFINE_DUMMY_FLAG(ALLOW_WITH_RESPAWN_INVUL), + DEFINE_DUMMY_FLAG(ALLOW_WITH_RESPAWN_INVUL, false), }; static FFlagDef PlayerPawnFlagDefs[] = @@ -394,6 +399,7 @@ static FFlagDef PowerSpeedFlagDefs[] = static const struct FFlagList { const PClass * const *Type; FFlagDef *Defs; int NumDefs; } FlagLists[] = { { &RUNTIME_CLASS_CASTLESS(AActor), ActorFlagDefs, countof(ActorFlagDefs) }, + { &RUNTIME_CLASS_CASTLESS(AActor), MoreFlagDefs, countof(MoreFlagDefs) }, { &RUNTIME_CLASS_CASTLESS(AInventory), InventoryFlagDefs, countof(InventoryFlagDefs) }, { &RUNTIME_CLASS_CASTLESS(AWeapon), WeaponFlagDefs, countof(WeaponFlagDefs) }, { &RUNTIME_CLASS_CASTLESS(APlayerPawn), PlayerPawnFlagDefs, countof(PlayerPawnFlagDefs) }, @@ -443,7 +449,7 @@ FFlagDef *FindFlag (const PClass *type, const char *part1, const char *part2, bo if (part2 == NULL) { // Search all lists - int max = strict ? 1 : NUM_FLAG_LISTS; + int max = strict ? 2 : NUM_FLAG_LISTS; for (i = 0; i < max; ++i) { if (type->IsDescendantOf (*FlagLists[i].Type)) @@ -683,4 +689,14 @@ void InitThingdef() symt.AddSymbol(new PField(NAME_Target, TypeActor, VARF_Native, myoffsetof(AActor, target))); symt.AddSymbol(new PField(NAME_Master, TypeActor, VARF_Native, myoffsetof(AActor, master))); symt.AddSymbol(new PField(NAME_Tracer, TypeActor, VARF_Native, myoffsetof(AActor, tracer))); + + // synthesize a symbol for each flag. The bounce flags are excluded on purpose. + for (size_t i = 0; i < countof(ActorFlagDefs); i++) + { + int bit = 0; + unsigned val = ActorFlagDefs[i].flagbit; + while ((val >>= 1)) bit++; + symt.AddSymbol(new PField(FStringf("b%s", ActorFlagDefs[i].name), (ActorFlagDefs[i].fieldsize == 4? TypeSInt32 : TypeSInt16), VARF_Native, ActorFlagDefs[i].structoffset, bit)); + } + } diff --git a/src/scripting/zscript/zcc_compile.cpp b/src/scripting/zscript/zcc_compile.cpp index 274778dfee..49ba68f0a7 100644 --- a/src/scripting/zscript/zcc_compile.cpp +++ b/src/scripting/zscript/zcc_compile.cpp @@ -1836,9 +1836,12 @@ void ZCCCompiler::ProcessDefaultFlag(PClassActor *cls, ZCC_FlagStmt *flg) auto fd = FindFlag(cls, n1, n2, true); if (fd != nullptr) { - if (fd->structoffset == -1) + if (fd->deprecated) { Warn(flg, "Deprecated flag '%s%s%s' used", n1, n2 ? "." : "", n2 ? n2 : ""); + } + if (fd->structoffset == -1) + { HandleDeprecatedFlags((AActor*)cls->Defaults, cls, flg->set, fd->flagbit); } else @@ -2384,7 +2387,7 @@ static int Pex2Tok[] = { static FxExpression *ModifyAssign(FxBinary *operation, FxExpression *left) { auto assignself = static_cast(operation->left); - auto assignment = new FxAssign(left, operation); + auto assignment = new FxAssign(left, operation, true); assignself->Assignment = assignment; return assignment; }