131 replies to this topic

[Tzarls]

I´ll try to find the cause, but I´ll need some information, specially since my asm is very very basic.

What do you mean with "cleaning up the stack"? I guess it means setting the stack pointer to where it was when it entered the function? I was thinking that comparing the sp´s value right after the push with the value right before the pop at the end of the function should give the same result, right? If results match, does it mean the stack is being cleared properly?

I´ve had care to keep and restore the values of the registers I use in the functions, and to only use registers marked as "general purpose" or "scratch", BTW.

[Andreas Jonsson]

Yes. I mean exactly that. Making sure the stackpointer is restored to its correct location. Depending on the ABI this may mean that it should be returned to the same position it was when going in to the function, or it may be that the function arguments should be popped by the called function. Sometimes it is only the implicit return pointer that should be popped, leaving the rest of the arguments for the calling function to clean up.

If you backup the registers and stick to general purpose or scratch registers then the problem is most likely to be just the stackpointer.

You mentioned earlier that you access your environment with VPN. If you'd like to give me access to the environment I may be able to do some investigation on my own. I don't promise anything, but I may find a little time to look into this.

[Tzarls]

I´ve been checking the stack pointer´s values and at least with the functions I modified those values are the same after the push and before the pop.

Let me know where to send you the information about remote accessing my RPi and any other information you´d think will be useful and then you can log in any time you want - just let me know when you´re using it so I don´t interfere.

[Andreas Jonsson]

you can send it to my e-mail: andreas@angelcode.com

Of course, I'll need to know the directory where you're testing so I can have a look at the code. and do some tests. Also, I'll need to know what compilation flags you use. Or do you simply call make for the gnuc projects?

I assume I'll have access via SSH or Telnet, right?

[Tzarls]

I use Ultra-VNC of Tight-VNC to acces the RPi. For compilation I just use the default gnuc projects - I only modified the AS makefile to make it include the correct armfunc file.

I´ll send you the details to your email.

[Tzarls]

FWIW, the test_addon_dictionary fails when the script tries to call a native function passing a string by value. The passing of an int and a handle work as expected... maybe we need to add another check for that situation?

[Andreas Jonsson]

This might be that AngelScript tries to call the destructor on the string value after the function returns. If the function already destroyed the string before returning, this could possibly cause a seg fault.

Try commenting out the #define AS_CALLEE_DESTROY_OBJ_BY_VAL on line 741 in as_config.h.

[Tzarls]

Nope, didn´t help. I found the seg fault is happening at line 199 of file scriptdictionary.cpp.

memcpy (value, &it->second.valueInt, size);

It happens the third time this function is called, when passing "c". I suspect "value" isn´t getting the right, well, value (no pun intended).

[Andreas Jonsson]

Sounds like a problem related to the variable argument type. This is a special type, in that in AngelScript is will look like a single argument, but for the native function it will actually be 2 arguments, the first is a pointer, and the second is an int.

The original as_callfunc_arm.cpp should have been working with this, but depending on the changes you made, you may have accidentally broken it.

You can identify the variable argument type with the following condition:

if( descr->parameterTypes[n].GetTokenType() == ttQuestion )
{
  // Treat the variable argument type as two separate args, one pointer and one int
  ...
}

As both the pointer and the int will go to the generic paramBuffer, it should just be a matter of copying both normally. Remember, even though this type would be identified as taking up 64bits on the stack with GetSizeOnStackDWords() it shouldn't be aligned to a 64bit boundary.

[Tzarls]

Andreas:

Apparently I was wrong about the point where the failure happens. On one hand I wasn´t checking for the variable argument type, but that wasnñt causing the problem (I´ve added acheck for that nevertheless). The seg fault is caused by this part of the script:

// Test float with the dictionary
"  float f = 300;				  \n"
"  dict.set('c', f);			 \n"
"  dict.get('c', f);			 \n" // Seg fault
"  assert(f == 300);			   \n"

The float is being received by the app as a double, BTW. Any ideas?

[Andreas Jonsson]

It should be received as a double. The script will call the 'bool get(const string &in, double&out) const' overloaded method on the dictionary object. The C++ method will receive a pointer to a double that it will fill with the value. Then the script will convert that double to a float and assign it to the f variable.

That gives me an idea of the problem. When you check if a parameter is a float or double, do you also check to make sure it is not a reference? I.e. !descr->parameterTypes[n].IsReference()?

If the argument is a reference, it should be treated as a integer as it is the address that is being passed to the function, not the float value.

I think this was my mistake, I forgot to tell you about checking for references earlier.

[Tzarls]

Added the check for references, and the seg fault is gone! I still have some tests that fail and the GC still complaints about not being able to release an object of type "_builtin_function_" after the serializer test is passed.

The tests that are still failling are:

TestScriptString
TestRegisterType
TestVector3
TestStdString

Can you see anything in common between these tests?

The TestCompiler is also crashing - what´s more, this one even crashes the app but I have a very good idea of what´s wrong and how ti fix it.

I´ll begin by looking at the TestScriptString. It fails here:

ExecuteString(engine, "print(1.2 + \"a\")");
if( printOutput != "1.2a")
{
  printf("Get '%s'\n", printOutput.c_str());
  TEST_FAILED;
}

Edited by Tzarls, 03 December 2012 - 09:52 PM.

[Andreas Jonsson]

Does it manage to print something? What does it print? As the test shows, the expected output is "1.2a", what did you get?

The test will call the following registered function:

static CScriptString *AddFloatString(float f, const CScriptString &str)
{
  char buf[100];
  sprintf(buf, "%g", f);
  return new CScriptString(buf + str.buffer);
}

So it should pass the float f in s0, the string pointer str in r0, and then return the new string pointer in r0. Is this what is happening?

The function is registered with asCALL_CDECL_OBJLAST, so perhaps that may be the reason for the failure.

All of the tests that fail actually test a lot of different functionality, so I need to know on which particular test that they fail on. The best would be if you can tell me the line number where the test is reporting that it is failing. Then I can have a look at the code and try to think of what can be wrong.

Regards,
Andreas

Edited by Andreas Jonsson, 04 December 2012 - 07:01 AM.

[Tzarls]

This test first calls (ICC_CDECL)

string@ _string_factory_(const int, const uint8&)

Returns an object, and the calls (ICC_CDECL_OBJLAST)

string@ string::opAdd_r(double) const

This also returns an object. Then it prints:

Get '11.8998a'
Failed on line 254... (which is the last line of the code I copied in my previous post)

[Andreas Jonsson]

Yes. It's clear that the problem is that the function didn't receive the correct value on the float argument.

It would appear that armFuncObjLast is not working properly. It's setting the string pointer in the r0 register as it should, but it is not setting s0 with the float value as it should.

[Tzarls]

s0? Shouldn´t it be d0, since it´s a double?

[Andreas Jonsson]

Yes, you're right. My mistake. Sorry about that.

It should be a double, and it should be d0. The function that will be called is the AddDoubleString() and not the AddFloatString() as I said before.

[Tzarls]

I found something. In the as_callfunc_arm_gcc.S file, armFuncObjLast function begins and ends like this:

armFuncObjLast:
	stmfd   sp!, {r4-r8, r10, lr}

// Magic begins....
.......
  
  
	blx	 r4
	fstmiad r10, {d0-d7} // Copy contents of registers d0-d10 to the address stored in r10
	add	 sp, sp, r8		 // sp += r8 sets sp to its original value
  
	str  sp, [r10, #68]  // store final sp value - for checks only, should be taken out in finalcode
  
	ldmfd   sp!, {r4-r8, r10, pc}

This doesn´t work for the TestScriptString test. But if I change:

armFuncObjLast:
	stmfd   sp!, {r4-r8, r10, lr}

To this:

armFuncObjLast:
	stmfd   sp!, {r4-r8, r10, r11, lr}

(With the corresponding change to the epilog) then the test passes! Is this some kind of alignment problem?

Edited by Tzarls, 06 December 2012 - 10:41 AM.

[Andreas Jonsson]

It could be. I see you had originally added r10, to the stmfd instruction. By now adding r11 as well, the instruction would maintain the 8 byte alignment.

Does this change break anything else? Otherwise I'd suggest you leave it like this (just put a comment so we can remember why r11 is being backed up too).

[Tzarls]

I guess this makes it clear:

"The ARM-THUMB Procedure Call Standard

The new ATPCS requires that sp always points to 8-byte boundaries. Your assembly language code must preserve 8-byte alignment of the stack at all external interfaces."