Improve builtin error handler to return more information about problem

belliash commented

2018-07-22 22:41:29 +02:00

Owner

An example of error thrown by the current implementation of error handler looks as follows:

test.php Error: Invalid number of arguments passed to function/method 'abc'

We miss a lot of information, especially a line number and some stacktrace.

An example of error thrown by the current implementation of error handler looks as follows: test.php Error: Invalid number of arguments passed to function/method 'abc' We miss a lot of information, especially a line number and some stacktrace.

belliash added the

enhancement

label 2018-07-22 22:41:29 +02:00

belliash commented

2018-07-23 15:22:33 +02:00

Author

Owner

17c486d599 makes error reports more like in PHP

17c486d599 makes error reports more like in PHP

likoski commented

2018-08-06 17:54:36 +02:00

Owner

There are two kind of error reports. First one comes from compiler, second from virtual machine. As during compilation, interpreter does not know the context at all, there is not much we can do. Later, during execution all kind of information should be available and we should focus on that.

belliash self-assigned this 2018-08-23 13:39:40 +02:00

belliash commented

2018-09-01 09:25:25 +02:00

Author

Owner

TODO:

write new generic error handler, that will automatically handle file, line number and stacktrace when possible,
write new memory error handler that will automatically handle failed operation, file and line number,
implement memory error callback in SAPI that will act as fallback to memory limit.

Last point is very important, because output consumer takes a blob that needs memory allocation. If we exhaust all memory and some operation failed, it is almost certain that another try to allocate a memory for error blob will also fail. Then the callback from SAPI should be called. It should be implemented in SAPI, because it determines what to do with output.

Additionally, we can think about logging feature in the future.

TODO: * write new generic error handler, that will automatically handle file, line number and stacktrace when possible, * write new memory error handler that will automatically handle failed operation, file and line number, * implement memory error callback in SAPI that will act as fallback to memory limit. Last point is very important, because output consumer takes a blob that needs memory allocation. If we exhaust all memory and some operation failed, it is almost certain that another try to allocate a memory for error blob will also fail. Then the callback from SAPI should be called. It should be implemented in SAPI, because it determines what to do with output. Additionally, we can think about logging feature in the future.

belliash commented

2018-09-01 18:28:31 +02:00

Author

Owner

Last point is not necessary, because it does not call the allocator on OOM. We just need to ensure there is a bit of memory left, because it can step on to the next instruction and throw error. I reserved 10KB for this purpose.

I got some doubt here:
Actually, to get information about file and line, we need to access pVm->aInstrSet, that is currently filled only if debugging is enabled. This is the only place, where we could find information about last or current byte code instruction call. Each instruction contains information about operator, file and line. This information is missing or provides false data on each error thrown. The only trust information can be found in this SySet. However, this needs some memory. When debugging is enabled, all instructions are put there, so they can be dumped after byte code execution. On normal execution, we just need current instruction IMHO. Thus I wonder, how to resolve this, as there are 2 ways:

fill in the pVm->aInstrSet regardless debugging is enabled or not
put the current instruction call into pVm->aInstrSet, and clear it just before putting the new instruction.

The difference in memory usage for below sample code is exactly 5760 bytes! What do you think, which way to go?

/* test2.aer */
    interface B {}
    interface A extends B {}

    class Y implements A {
            function test($f) {
                    print(__FILE__ + "\n");
                    print(__DIR__ + "\n");
                    var_dump(debug_backtrace());
            }
    }

    class X extends Y implements B {
            private $x;

            function test2() {
                    parent::test(3);
            }
    }
/* test.aer */
    class Program {
            function main() {
                    print(__FILE__ + "\n");
                    include('test2.aer');
                    print(__FILE__ + "\n");
                    a();
                    $this->xyz();
                    var_dump(get_memory_usage());
                    $a=4;
            }

            function a() {
                    print("A\n");
            }

            function xyz() {
                $x = new X();
                    $this->a();
                $x->test2();
                    $this->a();
            }

    }

Last point is not necessary, because it does not call the allocator on OOM. We just need to ensure there is a bit of memory left, because it can step on to the next instruction and throw error. I reserved 10KB for this purpose. I got some doubt here: Actually, to get information about file and line, we need to access pVm->aInstrSet, that is currently filled only if debugging is enabled. This is the only place, where we could find information about last or current byte code instruction call. Each instruction contains information about operator, file and line. This information is missing or provides false data on each error thrown. The only trust information can be found in this SySet. However, this needs some memory. When debugging is enabled, all instructions are put there, so they can be dumped after byte code execution. On normal execution, we just need current instruction IMHO. Thus I wonder, how to resolve this, as there are 2 ways: * fill in the pVm->aInstrSet regardless debugging is enabled or not * put the current instruction call into pVm->aInstrSet, and clear it just before putting the new instruction. The difference in memory usage for below sample code is exactly 5760 bytes! What do you think, which way to go? /* test2.aer */ interface B {} interface A extends B {} class Y implements A { function test($f) { print(__FILE__ + "\n"); print(__DIR__ + "\n"); var_dump(debug_backtrace()); } } class X extends Y implements B { private $x; function test2() { parent::test(3); } } /* test.aer */ class Program { function main() { print(__FILE__ + "\n"); include('test2.aer'); print(__FILE__ + "\n"); a(); $this->xyz(); var_dump(get_memory_usage()); $a=4; } function a() { print("A\n"); } function xyz() { $x = new X(); $this->a(); $x->test2(); $this->a(); } }

belliash commented

2018-09-01 19:21:11 +02:00

Author

Owner

OK, I checked this and definitely we should NOT store all instructions if this is not necessary! 10000 times empty 'for' loop iteration generated 80016 instructions. With debugging disabled, it allocated 415862 bytes, while with debugging enabled, the memory consumption increased to 6706934 bytes. It is almost 7MB! When increasing iterations to 100000, memory consumption grew again to over 50MB (50747126) with 800016 instructions stored.

belliash commented

2018-09-03 09:56:08 +02:00

Author

Owner

Actually the following functions are used to report errors in VM:

PH7_VmThrowError()
VmThrowErrorAp()
VmErrorFormat()
PH7_VmThrowErrorAp()

Additionally, the following functions are used from PH7 context:

ph7_context_throw_error()
ph7_context_throw_error_format()

Actually the following functions are used to report errors in VM: * PH7_VmThrowError() * VmThrowErrorAp() * VmErrorFormat() * PH7_VmThrowErrorAp() Additionally, the following functions are used from PH7 context: * ph7_context_throw_error() * ph7_context_throw_error_format()

belliash commented

2018-09-05 20:18:25 +02:00

Author

Owner

Example errors from new error handler:

Error: Call to non-existent base interface 'F' in /home/belliash/test2.aer:2

Error: Call to non-existent base class 'Z' in /home/belliash/test2.aer:16

Error: Call to undefined function 'z()' in /home/belliash/test2.aer:11
    at {closure_1}() [/home/belliash/test2.aer:11]
    at Y::test() [/home/belliash/test2.aer:13]
    at X->test2() [/home/belliash/test2.aer:21]
    at Program->test3() [/home/belliash/test.aer:18]
    at Program->main() [/home/belliash/test.aer:7]

Warning: Test in /home/belliash/test.aer:9
    at Program->main() [/home/belliash/test.aer:9]

Example errors from new error handler: Error: Call to non-existent base interface 'F' in /home/belliash/test2.aer:2 Error: Call to non-existent base class 'Z' in /home/belliash/test2.aer:16 Error: Call to undefined function 'z()' in /home/belliash/test2.aer:11 at {closure_1}() [/home/belliash/test2.aer:11] at Y::test() [/home/belliash/test2.aer:13] at X->test2() [/home/belliash/test2.aer:21] at Program->test3() [/home/belliash/test.aer:18] at Program->main() [/home/belliash/test.aer:7] Warning: Test in /home/belliash/test.aer:9 at Program->main() [/home/belliash/test.aer:9]

belliash commented

2018-09-05 21:58:36 +02:00

Author

Owner

Branch debug_rewrite merged to master.

belliash closed this issue

2018-09-05 21:58:36 +02:00

Improve builtin error handler to return more information about problem #19