Stack traces on exceptions & assert

[maximecb]

Currently, every call site has an src_pos attribute which we can report in the stack trace.

@krypt-n pointed out that we may want to make sure to set the name attribute on every function, as much as possible. This will have to be handled at the parser level.

Another issue we need to discuss is that currently, assert maps to the abort instruction in the VM, which halts program execution. We have to decide whether we want to make assert simply throw an exception as in Python or keep the current behavior. The current behavior is based on the idea that assert is for circumstances which we really expect to be impossible, for sanity checks, whereas exceptions are for errors which we anticipate people could trigger.

If we make assert simply map to exceptions, things may be simpler, implementation-wise, because we wouldn't have to add stack traces to abort. However, if we keep the abort instruction, we probably want stack traces anyway, so there is also a discussion to be had as to whether that instruction should be kept or not.

[maximecb]

@krypt-n Do you have an opinion on the assert vs throw issue?

[krypt-n]

Currently, every call site has an src_pos attribute which we can report in the stack trace.

It's a bit hard to use, since we can't look at the call site during stack unwinding. Only the return adress is known. My current solution emits additional src_pos at the first instruction of the return block. But maybe I should add call site information to the stack somehow?

Do you have an opinion on the assert vs throw issue?

The central question seems to be whether we want to keep an abort instruction in the VM, since removing it would be an backwards incompatible change. If it exists it needs to provide a stacktrace for basic usability reasons.

The other question is what the assert-statement in plush maps to in the vm. Allowing any library to completely abort the execution of the entire vm if there is an otherwise unnoticable bug doesn't seem to be user friendly (depends, of course, on the usage of the asserts). Throwing an exception from libraries seems much more useful, compared to my experiences with assertion failure in closed-source c libraries.

[maximecb]

It's a bit hard to use, since we can't look at the call site during stack unwinding. Only the return address is known.

This is why I added a retEntry map in vm/interp.cpp. So that we can associate meta-information with return addresses (each one has an associated RetEntry). We should probably store the associated call instruction in there too, so that we can call getSrcPos on it.

The central question seems to be whether we want to keep an abort instruction in the VM, since removing it would be an backwards incompatible change. If it exists it needs to provide a stacktrace for basic usability reasons.

I think you're right that this is the important question. At this point I'm not worried about backwards incompatibility. Zeta is too young and not-fully-fleshed-out to worry about avoiding breaking changes IMO. We're still at a stage where we should be trying to figure out what's the best design long-term. I'm intentionally being careful about how much I promote Zeta to avoid drawing too many people before the basics are well-defined.

Allowing any library to completely abort the execution of the entire vm if there is an otherwise unnoticable bug doesn't seem to be user friendly

That rings true. Historically speaking, I implemented abort before Zeta had exceptions, because there was a need for some error handling mechanism, and this was the simplest thing to do. I was just thinking, too, that if we remove abort, it's always possible to add it back in later. However, if we keep it, people will make use of it, and it removing it would break people's code.

So, I think we should remove abort and make Plush assert map to an exception being thrown instead. It has the benefit that this will be simpler, as there will be only one stack-unwinding-on-error implementation.

[krypt-n]

This is why I added a retEntry map in vm/interp.cpp. So that we can associate meta-information with return addresses (each one has an associated RetEntry). We should probably store the associated call instruction in there too, so that we can call getSrcPos on it.

Multiple calls could return to the same block. A possiblity would be to always store the latest call instruction in the RetEntry, but I am sure that one could construct a recursive control flow where this causes wrong stacktraces.

Pseudo edit: I just noticed that callFun pushes the call instruction to the stack, funCall does not.

[maximecb]

Multiple calls could return to the same block. A possiblity would be to always store the latest call instruction in the RetEntry, but I am sure that one could construct a recursive control flow where this causes wrong stacktraces.

Ah! Thanks for pointing out this issue. I actually want to make it so each return address maps to a different call site. I can force the internal codegen to behave that way. It should simplify things a bit.

I just noticed that callFun pushes the call instruction to the stack, funCall does not.

And I think I should probably rename one of these functions to avoid the naming confusion :O

[maximecb]

@krypt-n I made it so RetEntry objects are unique to a given call instruction, and the RetEntry struct has a callInstr field: ad68e99

[maximecb]

Plush asserts now use exceptions. I removed the abort opcode: eac17b2

[krypt-n]

Great.

Should a stacktrace object/array be attached to the exception value, if it is catched? This would allow manual printing and/or introspection by the user or the runtime. A language runtime could for example look at the src_poss and print the corresponding lines of the program. That would be a nice feature for language implementers. What's your opinion on this vs. simple printing by the interpreter?

[maximecb]

Hmm. I suppose we could. The minor complication here is that Zeta exception objects have to be converted into RunError objects when they are passing through the VM code. Then they need to be re-converted into exception objects when going through Zeta code.

For illustration: import is a host function, which can throw a RunError object. Importing a package may also cause the package's init function to be executed, this is Zeta code, which itself could be throwing an exception.

Sidenote: we should probably specifically test annoying/complicated corner cases like this, importing a module which throws an exception during initialization.

[krypt-n]

Once again, I have to apologise for my absence. I should have at least shared my progress on this issue before staying silent for two months. https://github.com/krypt-n/zetavm/commit/82c0995e97ca5dad5dcbbbae7d39fc0fc3f5ba2b

test.pls:

#language "lang/plush/0"


var a = function() 
{
    throw "blub";
};

var b = function()
{
   a(); 
};

b();

$ ./zeta test.pls
ERROR: blub
in a at unknown location
in b at test.pls@11:5
in top-level at test.pls@14:2

TODO:

• check position information on throw
  • Position information on runtimeCalls was missing
• improve function name heuristics
• handle RunError conversion if thrown inside VM context

[maximecb]

Hi @krypt-n. Welcome back.

That seems like good progress. More than halfway there. Is there anything you're stuck on, need help with?

[krypt-n]

Nothing in particular. Is hostCall the only place that needs to convert from RunError to zeta exception? And do we need an explicit conversion or can we just pass the pointer to a zeta heap object inside of an C++ exception object? (Just asking if I'm missing anything)

[krypt-n]

ERROR: math is currently not available
in rt_throw at unknown location
in top-level at unknown location
in import (host function) at unknown location
in top-level at test.pls@3:9

• cplush needs to emit more positon information (since it is used to compile packages)
• top-level doesn't really contain much information. Maybe replace it with the package name

[maximecb]

Nothing in particular. Is hostCall the only place that needs to convert from RunError to zeta exception?

Yes, I believe so.

And do we need an explicit conversion or can we just pass the pointer to a zeta heap object inside of an C++ exception object?

In the case of RunError -> zeta object, we need to create a zeta object, we can't pass a pointer to a C++ object.

In the case of zeta exception -> RunError, I'm not sure. We technically could wrap zeta exceptions I suppose, but I think at the moment I'm unwrapping them so the rest of the code doesn't have to deal with them. Are you asking because you'd like to preserve the stack trace information?

cplush needs to emit more positon information (since it is used to compile packages)

My plan is to eventually compile everything with the Plush language package, but I guess I've been slacking off with making that transition happen.

top-level doesn't really contain much information. Maybe replace it with the package name

I think you could just say "at test.pls@3:9" ?

[krypt-n]

In the case of zeta exception -> RunError, I'm not sure. We technically could wrap zeta exceptions I suppose, but I think at the moment I'm unwrapping them so the rest of the code doesn't have to deal with them. Are you asking because you'd like to preserve the stack trace information?

In the current prototype, I am storing the zeta throw object and stack information inside of a RunError such that we can get stack information across host functions. (The stack trace from last week shows that)

[krypt-n]

top-level is just the name that plush codegen gives the top-level function. The exception handling doesn't differentiate between it and normal functions

[maximecb]

top-level is just the name that plush codegen gives the top-level function. The exception handling doesn't differentiate between it and normal functions

Seems good to me.

In the current prototype, I am storing the zeta throw object and stack information inside of a RunError such that we can get stack information across host functions. (The stack trace from last week shows that)

That's probably a good idea, because we can run in a situation where Plush code calls into ZetaVM code (C++), which calls back into Plush code. If we can just wrap the Plush exceptions and unwrap them, it seems better than converting to a C++ exception, and then back into a Plush one.