call on nil segfaults about potion

[rurban]

As you found out on stackoverflow, size needs to be length, as in ruby. A call on nil segfaults unfortunately.
A method on nil just silently fails as you also found out in #79.

https://stackoverflow.com/questions/29091475/printing-odd-prime-every-100k-primes-found/29096987#29096987

[robotii]

Related to this most probably, the comparison operators also segfault on nil. You can see this by running example/palindrome.pn with no arguments. With the -B switch it completes successfully, but segfaults under the jit.

[rurban]

The question is if we should slow down jit by checking argument types or not.
So far I check with a debugging potion, and not without.

[robotii]

I don't think it's necessary to type check arguments.

The problem is in potion_x86_call, where it doesn't check for nil before attempting to dereference memory. I think this needs to be added to the beginning of the function, otherwise it attempts to call potion_obj_get_call which returns nil, which is then attempted to be deferenced as a memory location.

Alternatively, the necessary code could be added just after the lines (in vm-x86.c)

      //[b]: got the method, call it (first special slot from PNClosure)
      TAG_LABEL(tag_b);

The memory deference for nil is in the line immediately following this.

I'm not sure where the best place is to put it, nor do I have the necessary ASM skills to do the modification myself.

[rurban]

On Oct 16, 2015, at 5:44 PM, Peter Arthur [email protected] wrote:

I don't think it's necessary to type check arguments.

The problem is in potion_x86_call, where it doesn't check for nil before attempting to dereference memory. I think this needs to be added to the beginning of the function, otherwise it attempts to call potion_obj_get_call which returns nil, which is then attempted to be deferenced as a memory location.

Alternatively, the necessary code could be added just after the lines (in vm-x86.c)

  //[b]: got the method, call it (first special slot from PNClosure)

TAG_LABEL(tag_b);
The memory deference for nil is in the line immediately following this.

Good idea. That would be only 2 lines.

I'm not sure where the best place is to put it, nor do I have the necessary ASM skills to do the modification myself.

I’ll try when I have a bit more time.

[robotii]

Hi @rurban

I've dug a bit deeper into this. This particular issue is caused by potion_object_size, which attempts to return the size of the object.

Unfortunately, there are a few problems with this function.

1. The number returned is a raw number, and needs to be wrapped in PN_NUM() macro in order for it to return the correct value to the caller.
2. The function is not GC aware, which means that if it gets passed a FWD struct, it will fail horribly.
3. The function does not consider non-pointer objects, such as PN_TRUE, PN_FALSE and PN_NUM.

I can confirm that the original code does not segfault when the line

potion_method(obj_vt, "size", potion_object_size, 0);

is commented out, indicating this is the problem.

Changing the vm call function would not fix this problem, although it wouldn't hurt to still make those changes. In fact, I think the current code correctly handles null

I really don't think this function is that useful and personally I would remove it unless we can find a reason to keep it. It can quite easily be confused with the length method, as we have already seen.

[rurban]

Just fixed it, thanks.
length vs size methods:

• length is defined for tuples, tables, str and bytes, returning the number of elements
• size is defined of all objects and now also for all primitives, returning the number of used bytes for the members and fields, but not for variable length data, like str, bytes.

size is a bit ill-defined and currently not used, yes. But I'll keep it asis for now