Skip to content

Commit

Permalink
When asEP_ALLOW_UNSAFE_REFERENCES is turned on the destruction of tem…
Browse files Browse the repository at this point in the history 
…porary objects will be deferred to end of expressions

git-svn-id: http://svn.code.sf.net/p/angelscript/code/trunk@2780 404ce1b2-830e-0410-a2e2-b09542c77caf
  • Loading branch information
angelcode committed Jul 4, 2022
1 parent 2f1d5f2 commit 4fea513
Show file tree
Hide file tree
Showing 3 changed files with 108 additions and 8 deletions.
46 changes: 39 additions & 7 deletions sdk/angelscript/source/as_compiler.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2984,6 +2984,7 @@ bool asCCompiler::CompileInitialization(asCScriptNode *node, asCByteCode *bc, co
ctx.bc.ObjInfo(offset, asOBJ_INIT);
}
}
ProcessDeferredParams(&ctx);
bc->AddCode(&ctx.bc);
}
}
Expand Down Expand Up @@ -3173,6 +3174,7 @@ bool asCCompiler::CompileInitialization(asCScriptNode *node, asCByteCode *bc, co
ctx.bc.ObjInfo(offset, asOBJ_INIT);
}
}
ProcessDeferredParams(&ctx);
bc->AddCode(&ctx.bc);
}
}
Expand Down Expand Up @@ -11108,7 +11110,7 @@ int asCCompiler::CompileConstructCall(asCScriptNode *node, asCExprContext *ctx)

ImplicitConversion(args[0], dt, node->lastChild, asIC_EXPLICIT_VAL_CAST);

ctx->bc.AddCode(&args[0]->bc);
MergeExprBytecode(ctx, args[0]);
ctx->type = args[0]->type;

asDELETE(args[0], asCExprContext);
Expand Down Expand Up @@ -15989,16 +15991,31 @@ void asCCompiler::PerformFunctionCall(int funcId, asCExprContext *ctx, bool isCo
ctx->bc.InstrSHORT(asBC_STOREOBJ, (short)returnOffset);
}

ReleaseTemporaryVariable(tmpExpr, &ctx->bc);
// If the context holds a variable that needs cleanup and the application uses unsafe
// references then store it as a deferred parameter so it will be cleaned up afterwards.
if (tmpExpr.isTemporary && engine->ep.allowUnsafeReferences)
{
asSDeferredParam defer;
defer.argNode = 0;
defer.argType = tmpExpr;
defer.argInOutFlags = asTM_INOUTREF;
defer.origExpr = 0;
ctx->deferredParams.PushLast(defer);
}
else
ReleaseTemporaryVariable(tmpExpr, &ctx->bc);

ctx->type.dataType.MakeReference(IsVariableOnHeap(returnOffset));
ctx->type.isLValue = false; // It is a reference, but not an lvalue

// Clean up arguments
// If application is using unsafe references, then don't clean up arguments yet because
// the returned object might be referencing one of the arguments.
if( args )
AfterFunctionCall(funcId, *args, ctx, false);
AfterFunctionCall(funcId, *args, ctx, engine->ep.allowUnsafeReferences ? true : false);

ProcessDeferredParams(ctx);
if (!engine->ep.allowUnsafeReferences)
ProcessDeferredParams(ctx);

ctx->bc.InstrSHORT(asBC_PSF, (short)returnOffset);
}
Expand Down Expand Up @@ -16078,15 +16095,30 @@ void asCCompiler::PerformFunctionCall(int funcId, asCExprContext *ctx, bool isCo
else
ctx->type.Set(descr->returnType);

ReleaseTemporaryVariable(tmpExpr, &ctx->bc);
// If the context holds a variable that needs cleanup and the application uses unsafe
// references then store it as a deferred parameter so it will be cleaned up afterwards.
if (tmpExpr.isTemporary && engine->ep.allowUnsafeReferences )
{
asSDeferredParam defer;
defer.argNode = 0;
defer.argType = tmpExpr;
defer.argInOutFlags = asTM_INOUTREF;
defer.origExpr = 0;
ctx->deferredParams.PushLast(defer);
}
else
ReleaseTemporaryVariable(tmpExpr, &ctx->bc);

ctx->type.isLValue = false;

// Clean up arguments
// If application is using unsafe references, then don't clean up arguments yet because
// the returned value might represent a reference to one of the arguments, e.g. an integer might in truth be a pointer
if( args )
AfterFunctionCall(funcId, *args, ctx, false);
AfterFunctionCall(funcId, *args, ctx, engine->ep.allowUnsafeReferences ? true : false);

ProcessDeferredParams(ctx);
if( !engine->ep.allowUnsafeReferences )
ProcessDeferredParams(ctx);
}
}

Expand Down
3 changes: 2 additions & 1 deletion sdk/docs/articles/changes2.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@

<h1>AngelScript Change Log</h1>

<h2>Version 2.36.0 WIP - 2022/06/29</h2>
<h2>Version 2.36.0 WIP - 2022/07/04</h2>

<ul>
<li>Bug fixes
Expand All @@ -40,6 +40,7 @@ <h2>Version 2.36.0 WIP - 2022/06/29</h2>
<li>Added engine property asEP_IGNORE_DUPLICATE_SHARED_INTF to add backwards compatibility with 2.35.0 when scripts include declaration of shared interfaces more than once (Thanks Violet CLM)
<li>Compiler now stores info on temporary variables too to allow these to be inspected at runtime
<li>Added engine property to dynamically turn off debug output
<li>When asEP_ALLOW_UNSAFE_REFERENCES is turned on the destruction of temporary objects will be deferred to end of expressions (Thanks Alexander Orefkov)
</ul>
<li>Library interface
<ul>
Expand Down
67 changes: 67 additions & 0 deletions sdk/tests/test_feature/source/test_unsaferef.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,25 @@ class C1
int m1_;
};

std::string toUtf8(const std::string& str)
{
return str;
}

std::string lastString;
asPWORD cstr(const std::string& str)
{
lastString = str;
return (asPWORD)str.c_str();
}

bool validString = false;
int sqlite3_exec(asUINT, asPWORD pstr)
{
validString = (lastString == (char*)pstr);
return 0;
}

bool Test()
{
bool fail = false;
Expand All @@ -111,6 +130,54 @@ bool Test()
CBufferedOutStream bout;
asIScriptEngine *engine;

// Test nested calls that return a camoflaged reference to a temporary object
// The compiler has no way of knowing the temporary object is being referenced
// https://www.gamedev.net/forums/topic/712250-temprary-object-lifetime-bug-or-feature/
{
engine = asCreateScriptEngine();
engine->SetMessageCallback(asMETHOD(CBufferedOutStream, Callback), &bout, asCALL_THISCALL);
engine->SetEngineProperty(asEP_ALLOW_UNSAFE_REFERENCES, true);
bout.buffer = "";

engine->RegisterTypedef("int_ptr", "uint");
RegisterStdString(engine);
engine->RegisterObjectMethod("string", "string toUtf8() const", asFUNCTION(toUtf8), asCALL_CDECL_OBJLAST);
#ifdef AS_PTR_SIZE == 1
engine->RegisterObjectMethod("string", "uint get_cstr() const property", asFUNCTION(cstr), asCALL_CDECL_OBJLAST);
engine->RegisterGlobalFunction("int sqlite3_exec(uint, uint)", asFUNCTION(sqlite3_exec), asCALL_CDECL);
#else
engine->RegisterObjectMethod("string", "uint64 get_cstr() const property", asFUNCTION(cstr), asCALL_CDECL_OBJLAST);
engine->RegisterGlobalFunction("int sqlite3_exec(uint, uint64)", asFUNCTION(sqlite3_exec), asCALL_CDECL);
#endif

asIScriptModule *mod = engine->GetModule("test", asGM_ALWAYS_CREATE);
mod->AddScriptSection("test",
"class sqlite { \n"
" int_ptr db; \n"
" int exec(const string & in query) { \n"
" return sqlite3_exec(db, query.toUtf8().cstr); \n" // As get_cstr returns an uint, the compiler destroys the string returned by toUtf8 before calling sqlite3_exec
" } \n"
"} \n");
r = mod->Build();
if (r < 0)
TEST_FAILED;

r = ExecuteString(engine, "sqlite s; s.exec('query');", mod);
if (r != asEXECUTION_FINISHED)
TEST_FAILED;

if (!validString)
TEST_FAILED;

if (bout.buffer != "")
{
PRINTF("%s", bout.buffer.c_str());
TEST_FAILED;
}

engine->ShutDownAndRelease();
}

// Test cleanup of temporary reference to string constant
// Reported by Polyak Istvan
{
Expand Down

0 comments on commit 4fea513

Please sign in to comment.