BitOne / php-meminfo

PHP extension to get insight about memory usage
MIT License
1.08k stars 78 forks source link

Fix #105 Wrong frame label when there are more than two frames into the stack #106

Closed ahocquard closed 3 years ago

ahocquard commented 4 years ago

See https://github.com/BitOne/php-meminfo/issues/105

PHP meminfo algorithm

The basic algorithm of php-meminfo:

Issue The issue is that the zend_rebuild_symbol_table does not return necessarily the symbol table of the current frame. Actually, it gets the symbol table of the first user function: https://github.com/php/php-src/blob/php-7.4.4/Zend/zend_execute_API.c#L1468

In php-meminfo, the last called function is meminfo_dump, which is not a user function. There is no symbol table associated to it. It means that zend_rebuild_symbol_table returns the symbol table of the next frame.

That's why php-meminfo (I suppose) gets the previous frame to guess the name: because the symbol table was not the correct one. But it works with only one function in the stack.

Why wasn't it detected by the test? Following the previous test, here an explanation why it was green and not detecting the issue:

exec_frame symbol_table prev_frame prev_frame -> prev_execute_data label
meminfo_dump myTestFunction myTestFunction global myTestFunction
myTestFunction myTestFunction global null GLOBAL (but symbol table already memoized)
global global null GLOBAL (from variable state of previous loop)

Why is it detected by the new test?

Let's increase the number of function as in the fix:

exec_frame symbol_table of prev_frame prev_frame -> prev_execute_data label
meminfo_dump functionLevel3 functionLevel3 functionLevel2 functionLevel3
functionLevel3 functionLevel3 functionLevel2 functionLevel1 functionLevel2 (but symbol table already analyzed)
functionLevel2 functionLevel2 functionLevel1 global functionLevel1
functionLevel1 functionLevel1 global null GLOBAL
global global null GLOBAL (from variable state of previous loop)

We can see the shift between the symbol table and the label. If you try to execute the code of the test without the fix, it will be exactly this output.

Solution

Actually, the fix is simple. Instead of relying of the frame returned by zend_rebuild_symbol_table, it gets it from the frame directly. This way, there is no shift. The first frame meminfo_dump will be skipped as there is no symbol table. The label to get is the one from the current frame, not the previous one.