From 38c94838b98e216f9ef5b5c933a974c535819852 Mon Sep 17 00:00:00 2001 From: David Anderson Date: Sat, 10 Mar 2007 21:02:40 +0000 Subject: [PATCH] IPluginFunction implementation is re-entrant across native calls, as heap allocations are delayed until execution removed ICallable::GetAddressOfPushedParam removed phys_addr from ICallable::PushArray fixed a bug where sp_context_t::n_idx was overwritten upon re-entrant calls --HG-- extra : convert_revision : svn%3A39bc706e-5318-0410-9160-8a85361fbb7c/trunk%40595 --- core/CMsgListenerWrapper.h | 4 +- core/systems/ForwardSys.cpp | 9 +- core/systems/ForwardSys.h | 2 +- core/vm/sp_vm_basecontext.cpp | 3 + core/vm/sp_vm_function.cpp | 191 ++++++++++++++++++---------------- core/vm/sp_vm_function.h | 8 +- public/IForwardSys.h | 16 +-- public/sourcepawn/sp_vm_api.h | 28 ++--- 8 files changed, 137 insertions(+), 124 deletions(-) diff --git a/core/CMsgListenerWrapper.h b/core/CMsgListenerWrapper.h index 254d24b9..d06fe8e2 100644 --- a/core/CMsgListenerWrapper.h +++ b/core/CMsgListenerWrapper.h @@ -105,7 +105,7 @@ inline void MsgListenerWrapper::OnUserMessage(int msg_id, bf_write *bf, IRecipie m_Hook->PushCell(msg_id); m_Hook->PushCell(0); //:TODO: push handle! - m_Hook->PushArray(g_MsgPlayers, size, NULL); + m_Hook->PushArray(g_MsgPlayers, size); m_Hook->PushCell(size); m_Hook->PushCell(pFilter->IsReliable()); m_Hook->PushCell(pFilter->IsInitMessage()); @@ -119,7 +119,7 @@ inline ResultType MsgListenerWrapper::InterceptUserMessage(int msg_id, bf_write m_Intercept->PushCell(msg_id); m_Intercept->PushCell(0); //:TODO: push handle! - m_Intercept->PushArray(g_MsgPlayers, size, NULL); + m_Intercept->PushArray(g_MsgPlayers, size); m_Intercept->PushCell(size); m_Intercept->PushCell(pFilter->IsReliable()); m_Intercept->PushCell(pFilter->IsInitMessage()); diff --git a/core/systems/ForwardSys.cpp b/core/systems/ForwardSys.cpp index d43c15d9..bbbee3f5 100644 --- a/core/systems/ForwardSys.cpp +++ b/core/systems/ForwardSys.cpp @@ -299,7 +299,7 @@ int CForward::Execute(cell_t *result, IForwardFilter *filter) } else if (type == Param_Float || type == Param_Cell) { func->PushCellByRef(¶m->val); } else { - func->PushArray(param->byref.orig_addr, param->byref.cells, NULL, param->byref.flags); + func->PushArray(param->byref.orig_addr, param->byref.cells, param->byref.flags); assert(type == Param_Array || type == Param_FloatByRef || type == Param_CellByRef); } } else { @@ -489,7 +489,7 @@ void CForward::_Int_PushArray(cell_t *inarray, unsigned int cells, int flags) m_params[m_curparam].byref.orig_addr = inarray; } -int CForward::PushArray(cell_t *inarray, unsigned int cells, cell_t **phys_addr, int flags) +int CForward::PushArray(cell_t *inarray, unsigned int cells, int flags) { /* We don't allow this here */ if (!inarray) @@ -513,11 +513,6 @@ int CForward::PushArray(cell_t *inarray, unsigned int cells, cell_t **phys_addr, m_params[m_curparam].pushedas = Param_Array; } - if (phys_addr) - { - *phys_addr = NULL; - } - _Int_PushArray(inarray, cells, flags); m_curparam++; diff --git a/core/systems/ForwardSys.h b/core/systems/ForwardSys.h index e0ce371b..2dbde5c8 100644 --- a/core/systems/ForwardSys.h +++ b/core/systems/ForwardSys.h @@ -50,7 +50,7 @@ public: //ICallable virtual int PushCellByRef(cell_t *cell, int flags); virtual int PushFloat(float number); virtual int PushFloatByRef(float *number, int flags); - virtual int PushArray(cell_t *inarray, unsigned int cells, cell_t **phys_addr, int flags); + virtual int PushArray(cell_t *inarray, unsigned int cells, int flags); virtual int PushString(const char *string); virtual int PushStringEx(char *buffer, size_t length, int sz_flags, int cp_flags); virtual void Cancel(); diff --git a/core/vm/sp_vm_basecontext.cpp b/core/vm/sp_vm_basecontext.cpp index eb6a52b7..0c35b182 100644 --- a/core/vm/sp_vm_basecontext.cpp +++ b/core/vm/sp_vm_basecontext.cpp @@ -167,6 +167,7 @@ int BaseContext::Execute(uint32_t code_addr, cell_t *result) cell_t save_sp = ctx->sp; cell_t save_hp = ctx->hp; + uint32_t n_idx = ctx->n_idx; bool wasExec = m_InExec; @@ -202,6 +203,8 @@ int BaseContext::Execute(uint32_t code_addr, cell_t *result) ctx->hp = save_hp; } + ctx->n_idx = n_idx; + return err; } diff --git a/core/vm/sp_vm_function.cpp b/core/vm/sp_vm_function.cpp index 1fe2f25b..9ee3d29a 100644 --- a/core/vm/sp_vm_function.cpp +++ b/core/vm/sp_vm_function.cpp @@ -86,7 +86,7 @@ int CFunction::PushCellByRef(cell_t *cell, int flags) return SetError(SP_ERROR_PARAMS_MAX); } - return PushArray(cell, 1, NULL, flags); + return PushArray(cell, 1, flags); } int CFunction::PushFloat(float number) @@ -101,7 +101,7 @@ int CFunction::PushFloatByRef(float *number, int flags) return PushCellByRef((cell_t *)number, flags); } -int CFunction::PushArray(cell_t *inarray, unsigned int cells, cell_t **phys_addr, int copyback) +int CFunction::PushArray(cell_t *inarray, unsigned int cells, int copyback) { if (m_curparam >= SP_MAX_EXEC_PARAMS) { @@ -109,32 +109,15 @@ int CFunction::PushArray(cell_t *inarray, unsigned int cells, cell_t **phys_addr } ParamInfo *info = &m_info[m_curparam]; - int err; - - if ((err=m_pContext->HeapAlloc(cells, &info->local_addr, &info->phys_addr)) != SP_ERROR_NONE) - { - return SetError(err); - } info->flags = inarray ? copyback : 0; info->marked = true; - info->size = cells * sizeof(cell_t); - m_params[m_curparam] = info->local_addr; + info->size = cells; + info->str.is_sz = false; + info->orig_addr = inarray; + m_curparam++; - if (inarray) - { - memcpy(info->phys_addr, inarray, sizeof(cell_t) * cells); - info->orig_addr = inarray; - } else { - info->orig_addr = info->phys_addr; - } - - if (phys_addr) - { - *phys_addr = info->phys_addr; - } - return SP_ERROR_NONE; } @@ -157,39 +140,15 @@ int CFunction::_PushString(const char *string, int sz_flags, int cp_flags, size_ ParamInfo *info = &m_info[m_curparam]; size_t cells = (len + sizeof(cell_t) - 1) / sizeof(cell_t); - int err; - - if ((err=m_pContext->HeapAlloc(cells, &info->local_addr, &info->phys_addr)) != SP_ERROR_NONE) - { - return SetError(err); - } info->marked = true; - m_params[m_curparam] = info->local_addr; - m_curparam++; /* Prevent a leak */ - - if (!(sz_flags & SM_PARAM_STRING_COPY)) - { - goto skip_localtostr; - } - - if (sz_flags & SM_PARAM_STRING_UTF8) - { - if ((err=m_pContext->StringToLocalUTF8(info->local_addr, len, string, NULL)) != SP_ERROR_NONE) - { - return SetError(err); - } - } else { - if ((err=m_pContext->StringToLocal(info->local_addr, len, string)) != SP_ERROR_NONE) - { - return SetError(err); - } - } - -skip_localtostr: - info->flags = cp_flags; info->orig_addr = (cell_t *)string; + info->flags = cp_flags; info->size = len; + info->str.sz_flags = sz_flags; + info->str.is_sz = true; + + m_curparam++; return SP_ERROR_NONE; } @@ -201,21 +160,13 @@ void CFunction::Cancel() return; } - while (m_curparam--) - { - if (m_info[m_curparam].marked) - { - m_pContext->HeapRelease(m_info[m_curparam].local_addr); - m_info[m_curparam].marked = false; - } - } - m_errorstate = SP_ERROR_NONE; + m_curparam = 0; } int CFunction::Execute(cell_t *result) { - int err; + int err = SP_ERROR_NONE; if (!IsRunnable()) { m_errorstate = SP_ERROR_NOT_RUNNABLE; @@ -231,56 +182,122 @@ int CFunction::Execute(cell_t *result) cell_t temp_params[SP_MAX_EXEC_PARAMS]; ParamInfo temp_info[SP_MAX_EXEC_PARAMS]; unsigned int numparams = m_curparam; + unsigned int i; bool docopies = true; if (numparams) { //Save the info locally, then reset it for re-entrant calls. - memcpy(temp_params, m_params, numparams * sizeof(cell_t)); memcpy(temp_info, m_info, numparams * sizeof(ParamInfo)); } m_curparam = 0; - if ((err = CallFunction(temp_params, numparams, result)) != SP_ERROR_NONE) + /* Browse the parameters and build arrays */ + for (i=0; iHeapAlloc(temp_info[i].size, + &(temp_info[i].local_addr), + &(temp_info[i].phys_addr))) + != SP_ERROR_NONE) + { + break; + } + if (temp_info[i].orig_addr) + { + memcpy(temp_info[i].phys_addr, temp_info[i].orig_addr, sizeof(cell_t) * temp_info[i].size); + } + } else { + /* Calculate cells required for the string */ + size_t cells = (temp_info[i].size + sizeof(cell_t) - 1) / sizeof(cell_t); + + /* Allocate the buffer */ + if ((err=m_pContext->HeapAlloc(cells, + &(temp_info[i].local_addr), + &(temp_info[i].phys_addr))) + != SP_ERROR_NONE) + { + break; + } + /* Copy original string if necessary */ + if ((temp_info[i].str.sz_flags & SM_PARAM_STRING_COPY) && (temp_info[i].orig_addr != NULL)) + { + if (temp_info[i].str.sz_flags & SM_PARAM_STRING_UTF8) + { + if ((err=m_pContext->StringToLocalUTF8(temp_info[i].local_addr, + temp_info[i].size, + (const char *)temp_info[i].orig_addr, + NULL)) + != SP_ERROR_NONE) + { + break; + } + } else { + if ((err=m_pContext->StringToLocal(temp_info[i].local_addr, + temp_info[i].size, + (const char *)temp_info[i].orig_addr)) + != SP_ERROR_NONE) + { + break; + } + } + } + } /* End array/string calculation */ + /* Update the pushed parameter with the byref local address */ + temp_params[i] = temp_info[i].local_addr; + } else { + /* Just copy the value normally */ + temp_params[i] = m_params[i]; + } + } + + /* Make the call if we can */ + if (err == SP_ERROR_NONE) + { + if ((err = CallFunction(temp_params, numparams, result)) != SP_ERROR_NONE) + { + docopies = false; + } + } else { docopies = false; } - while (numparams--) + /* i should be equal to the last valid parameter + 1 */ + while (i--) { - if (!temp_info[numparams].marked) + if (!temp_info[i].marked) { continue; } - if (docopies && temp_info[numparams].flags) + if (docopies && (temp_info[i].flags & SM_PARAM_COPYBACK)) { - if (temp_info[numparams].orig_addr) + if (temp_info[i].orig_addr) { - if (temp_info[numparams].size == sizeof(cell_t)) + if (temp_info[i].str.is_sz) { - *temp_info[numparams].orig_addr = *temp_info[numparams].phys_addr; + memcpy(temp_info[i].orig_addr, temp_info[i].phys_addr, temp_info[i].size); } else { - memcpy(temp_info[numparams].orig_addr, - temp_info[numparams].phys_addr, - temp_info[numparams].size); + if (temp_info[i].size == 1) + { + *temp_info[i].orig_addr = *(temp_info[i].phys_addr); + } else { + memcpy(temp_info[i].orig_addr, + temp_info[i].phys_addr, + temp_info[i].size * sizeof(cell_t)); + } } } } - m_pContext->HeapPop(temp_info[numparams].local_addr); - temp_info[numparams].marked = false; + if ((err=m_pContext->HeapPop(temp_info[i].local_addr)) != SP_ERROR_NONE) + { + return err; + } } return err; } - -cell_t *CFunction::GetAddressOfPushedParam(unsigned int param) -{ - if (m_errorstate != SP_ERROR_NONE - || param >= m_curparam - || !m_info[param].marked) - { - return NULL; - } - - return m_info[param].phys_addr; -} diff --git a/core/vm/sp_vm_function.h b/core/vm/sp_vm_function.h index 4b13f66d..56d91b67 100644 --- a/core/vm/sp_vm_function.h +++ b/core/vm/sp_vm_function.h @@ -24,6 +24,11 @@ struct ParamInfo cell_t *phys_addr; /* Physical address of our copy */ cell_t *orig_addr; /* Original address to copy back to */ ucell_t size; /* Size of array in bytes */ + struct + { + bool is_sz; /* is a string */ + int sz_flags; /* has sz flags */ + } str; }; class CPlugin; @@ -38,10 +43,9 @@ public: virtual int PushCellByRef(cell_t *cell, int flags); virtual int PushFloat(float number); virtual int PushFloatByRef(float *number, int flags); - virtual int PushArray(cell_t *inarray, unsigned int cells, cell_t **phys_addr, int copyback); + virtual int PushArray(cell_t *inarray, unsigned int cells, int copyback); virtual int PushString(const char *string); virtual int PushStringEx(char *buffer, size_t length, int sz_flags, int cp_flags); - virtual cell_t *GetAddressOfPushedParam(unsigned int param); virtual int Execute(cell_t *result); virtual void Cancel(); virtual int CallFunction(const cell_t *params, unsigned int num_params, cell_t *result); diff --git a/public/IForwardSys.h b/public/IForwardSys.h index fc903fe4..a6e9b018 100644 --- a/public/IForwardSys.h +++ b/public/IForwardSys.h @@ -37,7 +37,7 @@ using namespace SourcePawn; #define SMINTERFACE_FORWARDMANAGER_NAME "IForwardManager" -#define SMINTERFACE_FORWARDMANAGER_VERSION 1 +#define SMINTERFACE_FORWARDMANAGER_VERSION 2 /* * There is some very important documentation at the bottom of this file. @@ -177,14 +177,10 @@ namespace SourceMod * * @param inarray Array to copy. Cannot be NULL, unlike ICallable's version. * @param cells Number of cells to allocate and optionally read from the input array. - * @param phys_addr Unused. If a value is passed, it will be filled with NULL. * @param flags Whether or not changes should be copied back to the input array. * @return Error code, if any. */ - virtual int PushArray(cell_t *inarray, - unsigned int cells, - cell_t **phys_addr, - int flags=0) =0; + virtual int PushArray(cell_t *inarray, unsigned int cells, int flags=0) =0; }; /** @@ -270,6 +266,14 @@ namespace SourceMod { return SMINTERFACE_FORWARDMANAGER_VERSION; } + virtual bool IsVersionCompatible(unsigned int version) + { + if (version < 2 || version > GetInterfaceVersion()) + { + return false; + } + return true; + } public: /** * @brief Creates a managed forward. This forward exists globally. diff --git a/public/sourcepawn/sp_vm_api.h b/public/sourcepawn/sp_vm_api.h index 7ec0ace2..3db0b0bc 100644 --- a/public/sourcepawn/sp_vm_api.h +++ b/public/sourcepawn/sp_vm_api.h @@ -94,23 +94,21 @@ namespace SourcePawn virtual int PushFloatByRef(float *number, int flags=SM_PARAM_COPYBACK) =0; /** - * @brief Pushes an array of cells onto the current call. - * NOTE: On Execute, the pointer passed will be modified if non-NULL and copy-back - * is enabled. - * NOTE: By reference parameters are cached and thus are not read until execution. - * This means you cannot push a pointer, change it, and push it again and expect - * two different values to come out. + * @brief Pushes an array of cells onto the current call. + * + * On Execute, the pointer passed will be modified if non-NULL and copy-back + * is enabled. + * + * By reference parameters are cached and thus are not read until execution. + * This means you cannot push a pointer, change it, and push it again and expect + * two different values to come out. * * @param inarray Array to copy, NULL if no initial array should be copied. * @param cells Number of cells to allocate and optionally read from the input array. - * @param phys_addr Optional return address for physical array, if one was made. * @param flags Whether or not changes should be copied back to the input array. * @return Error code, if any. */ - virtual int PushArray(cell_t *inarray, - unsigned int cells, - cell_t **phys_addr, - int flags=0) =0; + virtual int PushArray(cell_t *inarray, unsigned int cells, int flags=0) =0; /** * @brief Pushes a string onto the current call. @@ -177,14 +175,6 @@ namespace SourcePawn */ virtual IPluginContext *GetParentContext() =0; - /** - * @brief Returns the physical address of a by-reference parameter. - * - * @param param Parameter index to read (beginning at 0). - * @return Address, or NULL if invalid parameter specified. - */ - virtual cell_t *GetAddressOfPushedParam(unsigned int param) =0; - /** * @brief Returns whether the parent plugin is paused. *