From 213bdbadad141b251de202211ac1a0b5ebfd2395 Mon Sep 17 00:00:00 2001 From: Christoph Oelckers Date: Tue, 23 May 2023 23:01:17 +0200 Subject: [PATCH] - allocate VMFunction's PrintableName from the ClassDataAllocator arena. This avoids execution order issues on shutdown. VMFunction should not use FString. --- source/common/scripting/backend/codegen.cpp | 2 +- source/common/scripting/backend/vmbuilder.cpp | 2 +- source/common/scripting/core/imports.cpp | 2 +- source/common/scripting/core/scopebarrier.cpp | 2 +- source/common/scripting/core/vmdisasm.cpp | 2 +- source/common/scripting/frontend/zcc_compile.cpp | 2 +- source/common/scripting/jit/jit.cpp | 6 +++--- source/common/scripting/jit/jit_call.cpp | 4 ++-- source/common/scripting/jit/jit_runtime.cpp | 2 +- source/common/scripting/vm/vm.h | 2 +- source/common/scripting/vm/vmexec.h | 4 ++-- source/common/scripting/vm/vmframe.cpp | 8 ++++---- source/common/utility/memarena.cpp | 7 +++++++ source/common/utility/memarena.h | 1 + source/games/duke/src/spawn.cpp | 2 +- 15 files changed, 28 insertions(+), 20 deletions(-) diff --git a/source/common/scripting/backend/codegen.cpp b/source/common/scripting/backend/codegen.cpp index 2659eda26..6718368cd 100644 --- a/source/common/scripting/backend/codegen.cpp +++ b/source/common/scripting/backend/codegen.cpp @@ -9151,7 +9151,7 @@ FxExpression *FxVMFunctionCall::Resolve(FCompileContext& ctx) // [Player701] Catch attempts to call abstract functions directly at compile time if (NoVirtual && Function->Variants[0].Implementation->VarFlags & VARF_Abstract) { - ScriptPosition.Message(MSG_ERROR, "Cannot call abstract function %s", Function->Variants[0].Implementation->PrintableName.GetChars()); + ScriptPosition.Message(MSG_ERROR, "Cannot call abstract function %s", Function->Variants[0].Implementation->PrintableName); delete this; return nullptr; } diff --git a/source/common/scripting/backend/vmbuilder.cpp b/source/common/scripting/backend/vmbuilder.cpp index 7d270af61..35bf88e86 100644 --- a/source/common/scripting/backend/vmbuilder.cpp +++ b/source/common/scripting/backend/vmbuilder.cpp @@ -790,7 +790,7 @@ VMFunction *FFunctionBuildList::AddFunction(PNamespace *gnspc, const VersionInfo it.PrintableName = name; it.Function = new VMScriptFunction; it.Function->Name = functype->SymbolName; - it.Function->PrintableName = name; + it.Function->PrintableName = ClassDataAllocator.Strdup(name); it.Function->ImplicitArgs = functype->GetImplicitArgs(); it.Proto = nullptr; it.FromDecorate = fromdecorate; diff --git a/source/common/scripting/core/imports.cpp b/source/common/scripting/core/imports.cpp index c5a8cf9c5..cc50d22e5 100644 --- a/source/common/scripting/core/imports.cpp +++ b/source/common/scripting/core/imports.cpp @@ -198,7 +198,7 @@ void InitImports() { assert(afunc->VMPointer != NULL); *(afunc->VMPointer) = new VMNativeFunction(afunc->Function, afunc->FuncName); - (*(afunc->VMPointer))->PrintableName.Format("%s.%s [Native]", afunc->ClassName+1, afunc->FuncName); + (*(afunc->VMPointer))->PrintableName = ClassDataAllocator.Strdup(FStringf("%s.%s [Native]", afunc->ClassName+1, afunc->FuncName)); (*(afunc->VMPointer))->DirectNativeCall = afunc->DirectNative; AFTable.Push(*afunc); }); diff --git a/source/common/scripting/core/scopebarrier.cpp b/source/common/scripting/core/scopebarrier.cpp index a6c5a2370..8e03f757f 100644 --- a/source/common/scripting/core/scopebarrier.cpp +++ b/source/common/scripting/core/scopebarrier.cpp @@ -221,5 +221,5 @@ void FScopeBarrier::ValidateCall(PClass* selftype, VMFunction *calledfunc, int o { int innerside = FScopeBarrier::SideFromObjectFlags(selftype->VMType->ScopeFlags); if ((outerside != innerside) && (innerside != FScopeBarrier::Side_PlainData)) - ThrowAbortException(X_OTHER, "Cannot call %s function %s from %s context", FScopeBarrier::StringFromSide(innerside), calledfunc->PrintableName.GetChars(), FScopeBarrier::StringFromSide(outerside)); + ThrowAbortException(X_OTHER, "Cannot call %s function %s from %s context", FScopeBarrier::StringFromSide(innerside), calledfunc->PrintableName, FScopeBarrier::StringFromSide(outerside)); } \ No newline at end of file diff --git a/source/common/scripting/core/vmdisasm.cpp b/source/common/scripting/core/vmdisasm.cpp index 433a2fd1a..86d566985 100644 --- a/source/common/scripting/core/vmdisasm.cpp +++ b/source/common/scripting/core/vmdisasm.cpp @@ -528,7 +528,7 @@ void VMDisasm(FILE *out, const VMOP *code, int codesize, const VMScriptFunction } else if (code[i].op == OP_CALL_K && callfunc) { - printf_wrapper(out, " [%s]\n", callfunc->PrintableName.GetChars()); + printf_wrapper(out, " [%s]\n", callfunc->PrintableName); } else { diff --git a/source/common/scripting/frontend/zcc_compile.cpp b/source/common/scripting/frontend/zcc_compile.cpp index ffba4c301..fb21526f1 100644 --- a/source/common/scripting/frontend/zcc_compile.cpp +++ b/source/common/scripting/frontend/zcc_compile.cpp @@ -2798,7 +2798,7 @@ void ZCCCompiler::InitFunctions() { if (v->VarFlags & VARF_Abstract) { - Error(c->cls, "Non-abstract class %s must override abstract function %s", c->Type()->TypeName.GetChars(), v->PrintableName.GetChars()); + Error(c->cls, "Non-abstract class %s must override abstract function %s", c->Type()->TypeName.GetChars(), v->PrintableName); } } } diff --git a/source/common/scripting/jit/jit.cpp b/source/common/scripting/jit/jit.cpp index 13679d59a..b78c8137a 100644 --- a/source/common/scripting/jit/jit.cpp +++ b/source/common/scripting/jit/jit.cpp @@ -35,7 +35,7 @@ JitFuncPtr JitCompile(VMScriptFunction *sfunc) catch (const CRecoverableError &e) { OutputJitLog(logger); - Printf("%s: Unexpected JIT error: %s\n",sfunc->PrintableName.GetChars(), e.what()); + Printf("%s: Unexpected JIT error: %s\n",sfunc->PrintableName, e.what()); return nullptr; } } @@ -237,7 +237,7 @@ void JitCompiler::Setup() cc.comment(marks, 56); FString funcname; - funcname.Format("Function: %s", sfunc->PrintableName.GetChars()); + funcname.Format("Function: %s", sfunc->PrintableName); cc.comment(funcname.GetChars(), funcname.Len()); cc.comment(marks, 56); @@ -364,7 +364,7 @@ void JitCompiler::SetupSimpleFrame() if (errorDetails) { - I_FatalError("JIT: inconsistent number of %s for function %s", errorDetails, sfunc->PrintableName.GetChars()); + I_FatalError("JIT: inconsistent number of %s for function %s", errorDetails, sfunc->PrintableName); } for (int i = regd; i < sfunc->NumRegD; i++) diff --git a/source/common/scripting/jit/jit_call.cpp b/source/common/scripting/jit/jit_call.cpp index e6c1feb0b..d7074edc9 100644 --- a/source/common/scripting/jit/jit_call.cpp +++ b/source/common/scripting/jit/jit_call.cpp @@ -97,7 +97,7 @@ void JitCompiler::EmitVMCall(asmjit::X86Gp vmfunc, VMFunction *target) call->setArg(2, Imm(B)); call->setArg(3, GetCallReturns()); call->setArg(4, Imm(C)); - call->setInlineComment(target ? target->PrintableName.GetChars() : "VMCall"); + call->setInlineComment(target ? target->PrintableName : "VMCall"); LoadInOuts(); LoadReturns(pc + 1, C); @@ -360,7 +360,7 @@ void JitCompiler::EmitNativeCall(VMNativeFunction *target) asmjit::CBNode *cursorBefore = cc.getCursor(); auto call = cc.call(imm_ptr(target->DirectNativeCall), CreateFuncSignature()); - call->setInlineComment(target->PrintableName.GetChars()); + call->setInlineComment(target->PrintableName); asmjit::CBNode *cursorAfter = cc.getCursor(); cc.setCursor(cursorBefore); diff --git a/source/common/scripting/jit/jit_runtime.cpp b/source/common/scripting/jit/jit_runtime.cpp index 89672b940..cdfbedb67 100644 --- a/source/common/scripting/jit/jit_runtime.cpp +++ b/source/common/scripting/jit/jit_runtime.cpp @@ -306,7 +306,7 @@ void *AddJitFunction(asmjit::CodeHolder* code, JitCompiler *compiler) if (result == 0) I_Error("RtlAddFunctionTable failed"); - JitDebugInfo.Push({ compiler->GetScriptFunction()->PrintableName, compiler->GetScriptFunction()->SourceFileName, compiler->LineInfo, startaddr, endaddr }); + JitDebugInfo.Push({ FString(compiler->GetScriptFunction()->PrintableName), compiler->GetScriptFunction()->SourceFileName, compiler->LineInfo, startaddr, endaddr }); #endif return p; diff --git a/source/common/scripting/vm/vm.h b/source/common/scripting/vm/vm.h index 01e98fb61..2c42bd1f7 100644 --- a/source/common/scripting/vm/vm.h +++ b/source/common/scripting/vm/vm.h @@ -450,7 +450,7 @@ public: FName Name; const uint8_t *RegTypes = nullptr; TArray DefaultArgs; - FString PrintableName; // so that the VM can print meaningful info if something in this function goes wrong. + const char * PrintableName = nullptr; // so that the VM can print meaningful info if something in this function goes wrong. (allocated from the memory arena) class PPrototype *Proto; TArray ArgFlags; // Should be the same length as Proto->ArgumentTypes diff --git a/source/common/scripting/vm/vmexec.h b/source/common/scripting/vm/vmexec.h index f303304b8..5ddac8bd4 100644 --- a/source/common/scripting/vm/vmexec.h +++ b/source/common/scripting/vm/vmexec.h @@ -895,7 +895,7 @@ static int ExecScriptFunc(VMFrameStack *stack, VMReturn *ret, int numret) catch (CVMAbortException &err) { err.MaybePrintMessage(); - err.stacktrace.AppendFormat("Called from %s\n", call->PrintableName.GetChars()); + err.stacktrace.AppendFormat("Called from %s\n", call->PrintableName); // PrintParameters(reg.param + f->NumParam - B, B); throw; } @@ -2000,7 +2000,7 @@ static int ExecScriptFunc(VMFrameStack *stack, VMReturn *ret, int numret) catch (CVMAbortException &err) { err.MaybePrintMessage(); - err.stacktrace.AppendFormat("Called from %s at %s, line %d\n", sfunc->PrintableName.GetChars(), sfunc->SourceFileName.GetChars(), sfunc->PCToLine(pc)); + err.stacktrace.AppendFormat("Called from %s at %s, line %d\n", sfunc->PrintableName, sfunc->SourceFileName.GetChars(), sfunc->PCToLine(pc)); // PrintParameters(reg.param + f->NumParam - B, B); throw; } diff --git a/source/common/scripting/vm/vmframe.cpp b/source/common/scripting/vm/vmframe.cpp index a2c972820..3c3ef95b3 100644 --- a/source/common/scripting/vm/vmframe.cpp +++ b/source/common/scripting/vm/vmframe.cpp @@ -277,7 +277,7 @@ static bool CanJit(VMScriptFunction *func) if (func->NumRegA + func->NumRegD + func->NumRegF + func->NumRegS < maxregs) return true; - Printf(TEXTCOLOR_ORANGE "%s is using too many registers (%d of max %d)! Function will not use native code.\n", func->PrintableName.GetChars(), func->NumRegA + func->NumRegD + func->NumRegF + func->NumRegS, maxregs); + Printf(TEXTCOLOR_ORANGE "%s is using too many registers (%d of max %d)! Function will not use native code.\n", func->PrintableName, func->NumRegA + func->NumRegD + func->NumRegF + func->NumRegS, maxregs); return false; } @@ -289,7 +289,7 @@ int VMScriptFunction::FirstScriptCall(VMFunction *func, VMValue *params, int num // rather than let GZDoom crash. if (func->VarFlags & VARF_Abstract) { - ThrowAbortException(X_OTHER, "attempt to call abstract function %s.", func->PrintableName.GetChars()); + ThrowAbortException(X_OTHER, "attempt to call abstract function %s.", func->PrintableName); } #ifdef HAVE_VM_JIT if (vm_jit && CanJit(static_cast(func))) @@ -320,7 +320,7 @@ int VMNativeFunction::NativeScriptCall(VMFunction *func, VMValue *params, int nu catch (CVMAbortException &err) { err.MaybePrintMessage(); - err.stacktrace.AppendFormat("Called from %s\n", func->PrintableName.GetChars()); + err.stacktrace.AppendFormat("Called from %s\n", func->PrintableName); throw; } } @@ -702,7 +702,7 @@ void CVMAbortException::MaybePrintMessage() CVMAbortException err(reason, moreinfo, ap); - err.stacktrace.AppendFormat("Called from %s at %s, line %d\n", sfunc->PrintableName.GetChars(), sfunc->SourceFileName.GetChars(), sfunc->PCToLine(line)); + err.stacktrace.AppendFormat("Called from %s at %s, line %d\n", sfunc->PrintableName, sfunc->SourceFileName.GetChars(), sfunc->PCToLine(line)); throw err; } diff --git a/source/common/utility/memarena.cpp b/source/common/utility/memarena.cpp index 2a9467dc2..767824a64 100644 --- a/source/common/utility/memarena.cpp +++ b/source/common/utility/memarena.cpp @@ -132,6 +132,13 @@ void* FMemArena::Calloc(size_t size) return mem; } +const char* FMemArena::Strdup(const char* str) +{ + char* p = (char*)Alloc(strlen(str) + 1); + strcpy(p, str); + return p; +} + //========================================================================== // // FMemArena :: FreeAll diff --git a/source/common/utility/memarena.h b/source/common/utility/memarena.h index bb7f60b9a..8c1def64d 100644 --- a/source/common/utility/memarena.h +++ b/source/common/utility/memarena.h @@ -45,6 +45,7 @@ public: void *Alloc(size_t size); void* Calloc(size_t size); + const char* Strdup(const char*); void FreeAll(); void FreeAllBlocks(); FString DumpInfo(); diff --git a/source/games/duke/src/spawn.cpp b/source/games/duke/src/spawn.cpp index 8a46716e1..3afeaa48d 100644 --- a/source/games/duke/src/spawn.cpp +++ b/source/games/duke/src/spawn.cpp @@ -198,7 +198,7 @@ bool initspriteforspawn(DDukeActor* act) IFVIRTUALPTR(act, DDukeActor, TriggerSwitch) { - if (func->PrintableName.CompareNoCase("DukeActor.TriggerSwitch") != 0) + if (stricmp(func->PrintableName, "DukeActor.TriggerSwitch") != 0) overrideswitch = true; }