Argument clobbering & bad return value for DoesReturnOnStack

[Coder666]

Hi,

I found an issue when using AngelScript 2.35.1 (note that this is probably not version specific) and MSVC

In the function SystemCall::call_64conv in as_jit.cpp there is this code:

if(sFunc->DoesReturnOnStack()) {
		Register arg0 = as<void*>(cpu.intArg64(firstPos, firstPos, pax));
		if(pos == OP_None || objPointer)
			arg0 = as<void*>(*esi);
		else
			arg0 = as<void*>(*esi + sizeof(asPWORD));
		if(acceptReturn)
			as<void*>(*esp + local::retPointer) = arg0;
		retPointer = true;
		argOffset += sizeof(void*);

		if(func->hostReturnInMemory) {
			if(!cpu.isIntArg64Register(firstPos, firstPos)) {
				stackBytes += cpu.pushSize();
				retOnStack = true;
			}

			++intCount;
			++a;

			firstPos += 1;
		}
	}

I found that for pos == OP_Last, pos == OP_First and pos == OP_This the return value was not being placed into the correct VM stack location and was infact clobbering an argument instead of using the reserved return space.

This works fine with the following AngelScript function (presumably) because the arguments are local copies, releasing the return value string generally has no ill effect:

string foo( string, string )

however if the function is changed to:

string foo( const string& in, const string& in )

then the return value is released and this corrupts the overwritten string reference and causes a crash.

I have modified our version of call_64conv to the following:

if(sFunc->DoesReturnOnStack()) {
		Register arg0 = as<void*>(cpu.intArg64(firstPos, firstPos, pax));
		switch (pos)
		{
		case OP_First:
		case OP_Last:
		{
			//always second item in the array
			arg0 = as<void*>(*esi + sizeof(asPWORD));
		}
		case OP_This:
		case OP_None:
		{
			arg0 = as<void*>(*esi);
	        }
		break;
		default:
			throw std::exception("unknown operation");
		}

		if(acceptReturn)
			as<void*>(*esp + local::retPointer) = arg0;
		retPointer = true;
		argOffset += sizeof(void*);

		if(func->hostReturnInMemory) {
			if(!cpu.isIntArg64Register(firstPos, firstPos)) {
				stackBytes += cpu.pushSize();
				retOnStack = true;
			}

			++intCount;
			++a;

			firstPos += 1;
		}
	}

This is now working for me and the arguments are written into the correct VM stack location. Maybe you can come up with a better fix for this?

Thanks,

Tom

[Coder666]

Just another note on this one as I realised it is not very clear

The issue is with OP_This, where the return value space is in the first stack position stack[0] and objPointer is NULL so therefore the original code falls through and generates the code for the second stack poition.