#45 debug_backtrace() and __FILE__ OOP problem

Closed
opened 3 years ago by likoski · 17 comments
likoski commented 3 years ago
Owner

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.
likoski commented 3 years ago
Poster
Owner
array(1) {
[0] =>
string(9 'test2.aer')
}
array(2) {
[0] =>
array(5) {
[function] =>
    string(4 'test')
[args] =>
    array(0) {
    }
[line] =>
    int(4)
[file] =>
    string(51 '/home/likoski/Aer/test2.aer')
[class] =>
    string(1 'X')
}
[1] =>
array(5) {
[function] =>
    string(5 'Xmain')
[args] =>
    array(0) { 
    }
[line] =>
    int(5)
[file] =>
    string(51 '/home/likoski/Aer/test2.aer')
[class] => 
    string(1 'X')
}
}

Above code i produced by 2 files.

class X {
function test() {
    var_dump(debug_backtrace());
}}





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

Files are test1.aer and test2.aer but there is no information about test2.aer. None from debug_backtrace() and get_included_files() mention this file. Additionally, there is some trash returned by debug_backtrace(). I dont see a function named Xmain! Especially not in X class!

array(1) { [0] => string(9 'test2.aer') } array(2) { [0] => array(5) { [function] => string(4 'test') [args] => array(0) { } [line] => int(4) [file] => string(51 '/home/likoski/Aer/test2.aer') [class] => string(1 'X') } [1] => array(5) { [function] => string(5 'Xmain') [args] => array(0) { } [line] => int(5) [file] => string(51 '/home/likoski/Aer/test2.aer') [class] => string(1 'X') } } Above code i produced by 2 files. class X { function test() { var_dump(debug_backtrace()); }} class Program { function main() { include('test1.aer'); var_dump(get_included_files()); X::test(); }} Files are test1.aer and test2.aer but there is no information about test2.aer. None from debug_backtrace() and get_included_files() mention this file. Additionally, there is some trash returned by debug_backtrace(). I dont see a function named Xmain! Especially not in X class!
likoski commented 3 years ago
Poster
Owner

Fixing get_included_files() seems relatively easy. It intentionally cuts the full path and returns just filename.

Fixing get_included_files() seems relatively easy. It intentionally cuts the full path and returns just filename.
likoski commented 3 years ago
Poster
Owner

get_included_files() returns just one file since aa2d762b29

debug_backtrace() returns trash since e7b78be8e5

@devnexen, Could you kindly check this?

get_included_files() returns just one file since aa2d762b2937f8c04c051dcadeadf788ce7b1ac0 debug_backtrace() returns trash since e7b78be8e566e50f512b88bd402b1d9ef080f533 @devnexen, Could you kindly check this?
belliash commented 3 years ago
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.
likoski commented 3 years ago
Poster
Owner

Yes, you are right. But since get_included_files() looks at pVm->aFiles instead of pVm->aIncluded it should return both files, not just the entry point, especially if PH7_VmPushFilePath() alway pushes a file to pVm->aFiles. Tests show something else.

Yes, you are right. But since get_included_files() looks at pVm->aFiles instead of pVm->aIncluded it should return both files, not just the entry point, especially if PH7_VmPushFilePath() alway pushes a file to pVm->aFiles. Tests show something else.
belliash commented 3 years ago
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 3 years ago
Collaborator

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 3 years ago
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 3 years ago
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,
likoski commented 3 years ago
Poster
Owner

You are probably right, @belliash. Great investigation. I had no time so far to trace this, but I wonder how it knows about the line number. If we know that, we could pass information about file in very same way.

Another option is to check if pVm->aFiles is used anywhere else, and if it wont affect other functionalities, we could stop popping files out from it and for example store a pointer to proper file in class. Pointer should have less overhead.

You are probably right, @belliash. Great investigation. I had no time so far to trace this, but I wonder how it knows about the line number. If we know that, we could pass information about file in very same way. Another option is to check if pVm->aFiles is used anywhere else, and if it wont affect other functionalities, we could stop popping files out from it and for example store a pointer to proper file in class. Pointer should have less overhead.
belliash added the
bug
label 3 years ago
belliash changed title from include() and require() do not register included files to debug_backtrace() and __FILE__ OOP problem 3 years ago
belliash commented 3 years ago
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 3 years ago
belliash commented 3 years ago
Owner

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.

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.
likoski commented 3 years ago
Poster
Owner

We should deal with all magic constants at compilation level. This is exactly how it works in PHP.

We should deal with all magic constants at compilation level. This is exactly how it works in PHP.
belliash commented 3 years ago
Owner

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

I have also checked this and I agree this is the way to go with magic constants.
belliash referenced this issue from a commit 3 years ago
belliash commented 3 years ago
Owner

All magic constants should be working correctly now.

All magic constants should be working correctly now.
belliash commented 3 years ago
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.

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 closed this issue 3 years ago
belliash reopened this issue 3 years ago
belliash commented 3 years ago
Owner

67ce98d924 fixes another issue with debug_backtrace().

67ce98d924 fixes another issue with debug_backtrace().
belliash closed this issue 3 years ago
Sign in to join this conversation.
No Milestone
No Assignees
3 Participants
Notifications
Due Date

No due date set.

Dependencies

This issue currently doesn't have any dependencies.

Loading…
There is no content yet.