debug_backtrace() and __FILE__ OOP problem

Ghost commented

2018-08-18 15:48:06 +02:00

I was fixing a memory_limit branch and I realized that get_included_files() returns only one file, passed to interpreter from cli.

Additionally, it returns only filename, without path. This is something we broke, because clean PH7 returns all included files (still without paths).

What is more debug_backtrace() returns always this one, single file.

I was fixing a memory_limit branch and I realized that get_included_files() returns only one file, passed to interpreter from cli. Additionally, it returns only filename, without path. This is something we broke, because clean PH7 returns all included files (still without paths). What is more debug_backtrace() returns always this one, single file.

belliash commented

2018-08-18 18:39:45 +02:00

Owner

Indeed, there is something wrong with it, but I think there is another problem. First of all get_included_files() looks at pVm->aFiles instead of pVm->aIncluded. If you are including test1.aer, then test2.aer is entry file I guess. As it has not been included, there should not be information about it. PH7 shows it, and this is bug in PH7.

Secondly, PH7_VmPushFilePath() alway pushes file to pVm->aFiles, even it is already included. This might work for include(), but for sure not for include_once(). And as we got rid of *_once(), thus in fact require() and include() can load a file just once it is not working any longer.

Finally, I have not checked the problem with debug_backtrace(), but for sure there is something wrong with it.

Indeed, there is something wrong with it, but I think there is another problem. First of all get_included_files() looks at pVm->aFiles instead of pVm->aIncluded. If you are including test1.aer, then test2.aer is entry file I guess. As it has not been included, there should not be information about it. PH7 shows it, and this is bug in PH7. Secondly, PH7_VmPushFilePath() alway pushes file to pVm->aFiles, even it is already included. This might work for include(), but for sure not for include_once(). And as we got rid of *_once(), thus in fact require() and include() can load a file just once it is not working any longer. Finally, I have not checked the problem with debug_backtrace(), but for sure there is something wrong with it.

belliash commented

2018-08-18 20:38:13 +02:00

Owner

I don't think so, as there is (void)SySetPop(&pVm->aFiles); in vm.c:10476.
Anyway, I still don't understand problem with debug_backtrace(). It is not only related file, but also other fields contain some trash.

__FILE__ is also affected.

BTW: I changed the use of pVm->aFiles to pVm->aIncluded in cbb4a0aa5c.

I don't think so, as there is (void)SySetPop(&pVm->aFiles); in vm.c:10476. Anyway, I still don't understand problem with debug_backtrace(). It is not only related file, but also other fields contain some trash. \_\_FILE\_\_ is also affected. BTW: I changed the use of pVm->aFiles to pVm->aIncluded in cbb4a0aa5c.

devnexen commented

2018-08-18 21:06:49 +02:00

Member

Code at aa2d762b29 does not seem to be present in the actual version.

The Xmain presence is it not due to recent work of compiler_rework or implicit class entry (maybe it s "legit" ?)

Code at https://git.codingworkshop.eu.org/AerScript/Aer/commit/aa2d762b2937f8c04c051dcadeadf788ce7b1ac0 does not seem to be present in the actual version. The Xmain presence is it not due to recent work of compiler_rework or implicit class entry (maybe it s "legit" ?)

belliash commented

2018-08-18 21:28:53 +02:00

Owner

$ git reset --hard 80f376af62
HEAD is now at 80f376a Test debug_backtrace() function

I added the following code to make it working:

$main = new Program();
$main->main();

Output is exact as quoted by @likoski, but its strange, as it is working for me, while automated test failed with segmentation failed... Doing something wrong?

Reverted once again to e300575ab1 - it was building as psharp then and was for sure before compiler rework. The problem is still there, or I am an idiot :D

BTW: Code at aa2d762b29 is related to problem with get_included_files(), but we have already established, it is not a problem. It is a wrong behaviour in PH7, we have fixed and reported problem is just a side effect caused by another bug - iterating through pVm->aFiles instead of pVm-aIncluded.

In fact, we are dealing here with two separate problems. One is that get_included_files() was looking for a list of included files in wrong place. This is already fixed in cbb4a0aa5c.

Second problem is that __FILE__ always return an entry file, as well as debug_backtrace() also always returns entry file together with some trash in other rows like function name or class. I think there is some problem with frames if there are more files loaded.

$ git reset --hard 80f376af62c835941ec07a19e6640d2380e2419a HEAD is now at 80f376a Test debug_backtrace() function I added the following code to make it working: $main = new Program(); $main->main(); Output is exact as quoted by @likoski, but its strange, as it is working for me, while automated test failed with segmentation failed... Doing something wrong? Reverted once again to e300575ab16ad4ae6e4dab8eece9c2f93a2038d5 - it was building as psharp then and was for sure before compiler rework. The problem is still there, or I am an idiot :D BTW: Code at aa2d762b29 is related to problem with get_included_files(), but we have already established, it is not a problem. It is a wrong behaviour in PH7, we have fixed and reported problem is just a side effect caused by another bug - iterating through pVm->aFiles instead of pVm-aIncluded. In fact, we are dealing here with two separate problems. One is that get_included_files() was looking for a list of included files in wrong place. This is already fixed in cbb4a0aa5c. Second problem is that \_\_FILE\_\_ always return an entry file, as well as debug_backtrace() also always returns entry file together with some trash in other rows like function name or class. I think there is some problem with frames if there are more files loaded.

belliash commented

2018-08-19 00:40:29 +02:00

Owner

I have iterated through many revisions... and last that is not returning a trash is 397246d2f1. However the #15 has been implemented a commit later - e7b78be8e5. And it is already broken there, what makes no sense to me as I remember this was tested and working.

And..... Eureka! I know what is wrong. In scope of #15, we have improved debug_backtrace() as well as in scope of #2 we fixed __FILE__. However all tests we done were NOT using OOP. For example, lets take a look at code snippet from #2. The code test2.php and test3.php is executed immediately. But what will happen if we rewrite this test to use OOP? We will have 3 classes, one per file. All of them got compiled and executed. But during the execution, only class and its method will be initialized. Class method will be called later - from Program::main() in test.php. Thus the methods from test2.php and test3.php seems to be executed from test.php... Yes, in fact this was never working correctly. Unfortunately we have never tested this with object-oriented paradigm. But it is enough to revert few commits, take back to the point where structural programming was allowed - 8cbfca2bc9 to see that test from #2 is still working. It just needs several changes (no echo, string concatenation with + instead of dot, ...).

Same with #15. If we revert to 8cbfca2bc9, the test from that ticket will be working again. It will be working, because this revision allows structural programming. But it does not matter to which revision we revert, using OOP was always causing this to fail.

To sum up, there are 2 problems so far. First of all when including file, the filename is inserted into pVm->aFiles, later during execution its popped from there and information about file is not saved anywhere. Thus when calling some class method (there is no information about file, it has been defined in). When calling Program::main(), in fact all other files have been already opened, compiled, executed, closed and forgotten. There is only class with its methods in memory available. pVm->aFiles contain just one entry - entry point. This is the reason why __FILE__ and debug_backtrace() return this file only.

Second problem is trash information reported by debug_backtrace(). In fact 3 fields are affected:

file - which has been already described, __FILE__ IS also affected,
class - problem is strictly related to debug_backtrace(), __CLASS__ is NOT affected,
function - problem is strictly related to debug_backtrace(), __FUNCTION__ and __METHOD__ are NOT affected,

I have iterated through many revisions... and last that is not returning a trash is 397246d2f1. However the #15 has been implemented a commit later - e7b78be8e5. And it is already broken there, what makes no sense to me as I remember this was tested and working. And..... Eureka! I know what is wrong. In scope of #15, we have improved debug_backtrace() as well as in scope of #2 we fixed \_\_FILE\_\_. However all tests we done were NOT using OOP. For example, lets take a look at code snippet from #2. The code test2.php and test3.php is executed immediately. But what will happen if we rewrite this test to use OOP? We will have 3 classes, one per file. All of them got compiled and executed. But during the execution, only class and its method will be initialized. Class method will be called later - from Program::main() in test.php. Thus the methods from test2.php and test3.php seems to be executed from test.php... Yes, in fact this was never working correctly. Unfortunately we have never tested this with object-oriented paradigm. But it is enough to revert few commits, take back to the point where structural programming was allowed - 8cbfca2bc9 to see that test from #2 is still working. It just needs several changes (no echo, string concatenation with + instead of dot, ...). Same with #15. If we revert to 8cbfca2bc9, the test from that ticket will be working again. It will be working, because this revision allows structural programming. But it does not matter to which revision we revert, using OOP was always causing this to fail. To sum up, there are 2 problems so far. First of all when including file, the filename is inserted into pVm->aFiles, later during execution its popped from there and information about file is not saved anywhere. Thus when calling some class method (there is no information about file, it has been defined in). When calling Program::main(), in fact all other files have been already opened, compiled, executed, closed and forgotten. There is only class with its methods in memory available. pVm->aFiles contain just one entry - entry point. This is the reason why \_\_FILE\_\_ and debug_backtrace() return this file only. Second problem is trash information reported by debug_backtrace(). In fact 3 fields are affected: * file - which has been already described, \_\_FILE\_\_ IS also affected, * class - problem is strictly related to debug_backtrace(), \_\_CLASS\_\_ is NOT affected, * function - problem is strictly related to debug_backtrace(), \_\_FUNCTION\_\_ and \_\_METHOD\_\_ are NOT affected,

belliash added the

bug

label 2018-08-20 06:32:04 +02:00

belliash changed title from ~~include() and require() do not register included files~~ to debug_backtrace() and __FILE__ OOP problem

2018-08-20 06:37:07 +02:00

belliash commented

2018-08-20 10:37:49 +02:00

Owner

There is one more problem with debug_backtrace(). Causes segmentation fault, if called from closure:

class X {
function test() {
    $dump = function() {
        var_dump(debug_backtrace());
    };
    $dump();
}}

class Program {
function main() {
    include('test1.aer');
    var_dump(get_included_files());
    X::test();
}}

There is one more problem with debug_backtrace(). Causes segmentation fault, if called from closure: class X { function test() { $dump = function() { var_dump(debug_backtrace()); }; $dump(); }} class Program { function main() { include('test1.aer'); var_dump(get_included_files()); X::test(); }}

belliash self-assigned this 2018-08-21 06:30:39 +02:00

belliash commented

2018-08-21 10:06:49 +02:00

Owner

I think, I found all bugs. So to sum up, now:

debug_backtrace()'s function shows some trash, because pValue is not cleared correctly. __FUNCTION__ and __METHOD__ are not affected as they are processed during compilation.
debug_backtrace() causes segmentation fault, if called from closure, because some checks are missing before trying to extract class name (which does not exist in this case).
debug_backtrace() as well as __CLASS__ use PH7_VmPeekTopClass(). Even we iterate through frames, we simply call this function and get the same result. Same with __CLASS__ magic constant. Here is another problem with inheritance. __CLASS__ and debug_backtrace() will always return an instantiated class, even __CLASS__ was called from the base class.
debug_backtrace() and __FILE__ base results on pVm->aFiles. Filename is pushed there on include, compiled, executed and popped out. Execution if methods is not done at this place, but later, when pVm->aFiles contain just the base, entry file. We could of course do not pop them out, however the order of called class methods does not have to follow the order of included files. What is more, we cannot point to pVm->aFiles from frame, because new frame is created on function/method call and exception only. In other word, the frame is NOT created on file include. In my opinion, the only way to fix this is to link filename with class, however this would highly rely on the __CLASS__ that is actually also broken.
__DIR__ is also broken, because it rely on __FILE__. If we fix __FILE__ it will be also easy to fix __DIR__.

Points 1, 2 and 5 are/seem easy to fix.
I think actually the biggest challenge is to fix problem described in point 3. I'm not sure if we can propose a single solution to both debug_backtrace() and __CLASS__. Actually I think that we should move __CLASS__ from execution phase to compilation phase, like it is already done with __FUNCTION__. On the other side, if we find a class, we could check if it implements such method and return it or iterate through base classes until we find a proper one. Unfortunately, dealing with parent:: can be a bit tricky.

I think, I found all bugs. So to sum up, now: 1. debug_backtrace()'s function shows some trash, because pValue is not cleared correctly. \_\_FUNCTION\_\_ and \_\_METHOD\_\_ are not affected as they are processed during compilation. 2. debug_backtrace() causes segmentation fault, if called from closure, because some checks are missing before trying to extract class name (which does not exist in this case). 3. debug_backtrace() as well as \_\_CLASS\_\_ use PH7_VmPeekTopClass(). Even we iterate through frames, we simply call this function and get the same result. Same with \_\_CLASS\_\_ magic constant. Here is another problem with inheritance. \_\_CLASS\_\_ and debug_backtrace() will always return an instantiated class, even \_\_CLASS\_\_ was called from the base class. 4. debug_backtrace() and \_\_FILE__ base results on pVm->aFiles. Filename is pushed there on include, compiled, executed and popped out. Execution if methods is not done at this place, but later, when pVm->aFiles contain just the base, entry file. We could of course do not pop them out, however the order of called class methods does not have to follow the order of included files. What is more, we cannot point to pVm->aFiles from frame, because new frame is created on function/method call and exception only. In other word, the frame is NOT created on file include. In my opinion, the only way to fix this is to link filename with class, however this would highly rely on the \_\_CLASS\_\_ that is actually also broken. 5. \_\_DIR\_\_ is also broken, because it rely on \_\_FILE\_\_. If we fix \_\_FILE\_\_ it will be also easy to fix \_\_DIR\_\_. Points 1, 2 and 5 are/seem easy to fix. I think actually the biggest challenge is to fix problem described in point 3. I'm not sure if we can propose a single solution to both debug_backtrace() and \_\_CLASS\_\_. Actually I think that we should move \_\_CLASS\_\_ from execution phase to compilation phase, like it is already done with \_\_FUNCTION\_\_. On the other side, if we find a class, we could check if it implements such method and return it or iterate through base classes until we find a proper one. Unfortunately, dealing with parent:: can be a bit tricky.

belliash commented

2018-08-21 17:46:49 +02:00

Owner

I have also checked this and I agree this is the way to go with magic constants.

belliash referenced this issue from a commit

2018-08-21 19:47:32 +02:00

Enter the class block during compilation, #45.

belliash referenced this issue from a commit

2018-08-21 20:21:27 +02:00

Resolve __FILE__ at compile time. Ticket: #45: __FILE__ constant must be resolved at compile time, not run time. Otherwise it returns incorrect data.

belliash referenced this issue from a commit

2018-08-22 06:24:20 +02:00

Resolve __DIR__ at compile time. Ticket: #45: __DIR__ constant must be resolved at compile time, not run time. Otherwise it returns incorrect data.

belliash referenced this issue from a commit

2018-08-22 08:52:15 +02:00

Optimize the code, #45.

belliash referenced this issue from a commit

2018-08-22 10:00:09 +02:00

Resolve __CLASS__ at compile time. Ticket: #45: __CLASS__ constant must be resolved at compile time, not run time. Otherwise it returns incorrect data.

belliash commented

2018-08-22 15:15:38 +02:00

Owner

All magic constants should be working correctly now.

belliash referenced this issue from a commit

2018-08-22 15:36:16 +02:00

Implement PH7_VmExtractActiveClass(). This function extracts an active class with specified depth, #45.

belliash commented

2018-08-23 13:07:46 +02:00

Owner

While I have moved all magic constants from runtime to compile time, there is still a lot of work to be done in order to fix this issue. First of all, we need w full list of processed files, thus we cannot pop files out from pVm->aFiles. Every instruction that is emitted to VM, needs to contain a pointer to proper file it was called from. Actually, only line number is stored. Next, assert() and eval() builtin functions evaluate the code, which is not assigned to any file. It should point to file and line from which eval() or assert() was called or simply point to ":MEMORY". Finally, whole error reporting mechanism has to be rewritten. Actually it does not report information about line number. What is more it returns incorrect file name.

belliash referenced this issue from a commit

2018-08-27 15:50:16 +02:00

Rewrite debug_backtrace() builtin function. It makes a use of new VmExtractDebugTrace() function. This probably fixes #45.

belliash closed this issue

2018-08-27 15:50:16 +02:00

belliash reopened this issue

2018-08-30 16:15:49 +02:00

belliash commented

2018-08-31 08:34:39 +02:00

Owner

67ce98d924 fixes another issue with debug_backtrace().

67ce98d924 fixes another issue with debug_backtrace().