 |
How to reproduce on smaller scale: 1) Add the following spawnargs to any entity on the map: "scriptobject" "test" "start" "start" //this is probably not needed? 2) Add the following to file {mapname}.script near map: object test { float v0; float v1; float v2; float v3; void init(); void printText(); }; void test::init() { thread printText(); } void test::printText() { sys.println("thread done"); } When you start map, the init method is called, and the problem happens. |
|
|
I tried to print out some members of the current class straight in the method where thread was created. The members are printed properly. It means that despite the assert, the "this" pointer is properly passed into the new thread (as expected). It remains to understand what these asserts are about... |
|
|
Gathered all types of calls in one place: ----------------------------------------------------------------------------- object test { float v0; float v1; float v2; float v3; void init(); void printText(float x0, float x1); }; void freefunc(float x0, float x1) { sys.println("free func"); sys.println(x0); sys.println(x1); } void test::init() { v0 = 10; v1 = 11; v2 = 12; v3 = 13; test e = self; printText(-3.0, -4.3); e.printText(-0.3, -3.4); freefunc(-0.97, -9.7); thread printText(3.0, 4.3); thread e.printText(0.3, 3.4); thread freefunc(0.97, 9.7); } void test::printText(float x0, float x1) { sys.println("method"); sys.println(v0); sys.println(v1); sys.println(v2); sys.println(v3); sys.println(x0); sys.println(x1); } ----------------------------------------------------------------------------- And looked at the disassembly ("g_disasm 1"): ----------------------------------------------------------------------------- function test::init() 24 stack used, 4 parms, 20 locals { # stack[0] contains "this" pointer # save 10.0 into address (this + 0) (which is address of this->v0) (L17): 20896: ADDRESS a: test stack[0] b: field 0 c: pointer stack[4] (L17): 20897: STOREP_F a: const float 10.000000 b: pointer stack[4] # same for v1, v2, v3 (L18): 20898: ADDRESS a: test stack[0] b: field 4 c: pointer stack[8] (L18): 20899: STOREP_F a: const float 11.000000 b: pointer stack[8] (L19): 20900: ADDRESS a: test stack[0] b: field 8 c: pointer stack[12] (L19): 20901: STOREP_F a: const float 12.000000 b: pointer stack[12] (L20): 20902: ADDRESS a: test stack[0] b: field 12 c: pointer stack[16] (L20): 20903: STOREP_F a: const float 13.000000 b: pointer stack[16] # copy "this" into local variable "e" (which has offset 20) (L21): 20904: STORE_OBJ a: test stack[0] b: test stack[20] # method(c1, c2); //implicit object (L23): 20905: PUSH_OBJ a: test stack[0] (L23): 20906: PUSH_F a: const float -3.000000 (L23): 20907: PUSH_F a: const float -4.300000 # note that function is specified as index of VFT, since all methods are automatically virtual (L23): 20908: OBJECTCALL a: test stack[0] b: const virtual function vtable[ 1 ] c: const args 0 # e.method(c1, c2); //explicit object (L24): 20909: PUSH_OBJ a: test stack[20] (L24): 20910: PUSH_F a: const float -0.300000 (L24): 20911: PUSH_F a: const float -3.400000 (L24): 20912: OBJECTCALL a: test stack[20] b: const virtual function vtable[ 1 ] c: const args 0 # freefunc(c1, c2); //no object (L25): 20913: PUSH_F a: const float -0.970000 (L25): 20914: PUSH_F a: const float -9.700000 (L25): 20915: CALL a: const function freefunc # thread method(c1, c2); //implicit object (L27): 20916: PUSH_OBJ a: test stack[0] (L27): 20917: PUSH_F a: const float 3.000000 (L27): 20918: PUSH_F a: const float 4.300000 (L27): 20919: OBJTHREAD a: test stack[0] b: const virtual function vtable[ 1 ] c: const args 0 # thread e.method(c1, c2); //explicit object (L28): 20920: PUSH_OBJ a: test stack[20] (L28): 20921: PUSH_F a: const float 0.300000 (L28): 20922: PUSH_F a: const float 3.400000 (L28): 20923: OBJTHREAD a: test stack[20] b: const virtual function vtable[ 1 ] c: const args 0 # thread freefunc(c1, c2); //no object (L29): 20924: PUSH_F a: const float 0.970000 (L29): 20925: PUSH_F a: const float 9.700000 (L29): 20926: THREAD a: const function freefunc b: const args 8 (L30): 20927: RETURN ----------------------------------------------------------------------------- Obviously: 1) Creating thread by object + method is supported by its own opcode OBJTHREAD, so it should be legal. 2) The pointer to object and all arguments are properly passed in all cases. |
|
|
It seems that the "c" argument to object calls is wrong. It is zero even though there are some parameters here. For ordinary method calls (OP_OBJECTCALL), the "c" value is used only if method is called on entity, which is dead, null, or misses script object. It is used to pop function parameters from the locals stack: PopParms( st->c->value.argSize ); If the call succeeds, then callee allocates additional space for local variables (which are not parameters), see EnterFunction: // allocate space on the stack for locals // parms are already on stack c = func->locals - func->parmTotal; localstackUsed += c; and callee also removes both local variables and parameters from stack, see LeaveFunction: PopParms( currentFunction->locals ); For some reason, I see "c" = 0 in the disassembly, which seems to be wrong. Calls of free functions (CALL) don't have such argument. But free functions called in a newly created thread (THREAD) do have this argument as "b". Its value seems to be set correctly to the total size of arguments (8 bytes in the example). In the C++ code, it is used to pop back parameters of the called function (unconditionally): PopParms( st->b->value.argSize ); Finally, there are method calls in newly created thread (OP_OBJTHREAD). They also seem to have wrong value "c" = 0 right now. This "c" value is also used to remove function parameters from stack: PopParms( st->c->value.argSize ); Also, it is checked against some function-defined number: assert( st->c->value.argSize == func->parmTotal ); In the current case, it is 12 --- obviously 12 bytes is used for two float arguments + object pointer. So I think the "c" value should be 12 (size of all arguments including object pointer) both in OBJECTCALL and OBJTHREAD calls. |
|
|
The problem happens because func->parmTotal is computed only when function definition is parsed. When function declaration is met, the function is created, but all of its members like "parmTotal" are zero. See idCompiler::ParseFunctionDef. Also, the sample given above is fully fixed (no asserts) if I swap the definitions of test::init and test::printText methods. |
|
|
Fixed in svn rev 9226. The fix is inside idCompiler::ParseFunctionDef. Now when we parse function declaration/prototype, we compute parmSize and parmTotal too. It is not critical that we can compute it twice (or maybe even more times), but it is critical to have it computed when we call the method (which can happen before we see definition). This fix removed all asserts both on the testing code I attached, and on the Accountant 2 FM. I have skimmed through this FM, and all scripted events seem to work properly. So the bottom line is: creating new thread with object method call is perfectly valid! Specifying the object explicitly is not required, "this" is implicitly passed --- just like for ordinary method calls. |
- Anonymous
