Fix GH-10755: Memory leak in phar_rename_archive()

[stkeke]

In phar_renmae_archive() context, added one reference but immediately destroyed another, so do not need to increase refcount. With removal of refcount++ line, PHP/Zend no longer reports memory leak. Updated bug69958.phpt test file accordingly.

Testing (PASS on both Debug and Release build)
Debug: ./configure --enable-debug
Release: ./configure

1. Running selected tests.
   PASS Phar: bug #69958: Segfault in Phar::convertToData on invalid file [bug69958.phpt]

2. All tests under ext/phar/tests PASSED except skipped. =====================================================================
   Number of tests : 530 515
   Tests skipped : 15 ( 2.8%) --------
   Tests warned : 0 ( 0.0%) ( 0.0%)
   Tests failed : 0 ( 0.0%) ( 0.0%)
   Tests passed : 515 ( 97.2%) (100.0%)

---

Time taken : 26 seconds

[nielsdos]

Thank you for your interest in fixing this issue!
Since this is a bugfix, you need to target the lowest supported bugfix branch. In this case this is PHP-8.1.

The failures you're seeing in CI on Windows platforms is because the test exception message uses a forward slash (/) in the pathname, while on Windows a backwards slash () is used. You can fix that by replacing all occurrences under --EXPECTF-- of "%s/bug69958.tar" with "%sbug69958.tar" in the test file.

Finally, the failure you're seeing on ASAN can probably be reproduced locally if you configure with ./configure --enable-debug --enable-address-sanitizer. The error message likely means that the code allocates something using malloc, but frees using efree instead of free. So that allocation should probably changed to emalloc instead of malloc. i.e. malloc & free must be paired, and emalloc and efree must be paired, they must not be mixed.
You can probably try to figure out the origin of the problem by using GDB.

[stkeke]

@nielsdos Thanks for the detailed explanation and comments, which is essentially helpful for me. I will follow them and continue digging ...

[stkeke]

@nielsdos A question for you to clarify
When you said 'target the lowest supported bugfix branch. In this case this is PHP-8.1.', do you mean below?
I should create my patch based on 'PHP-8.1' branch and then make the PR against that branch rather than 'master' branch?
someone else will cherry-pick it to 'master' branch? or should I do that?

[nielsdos]

@nielsdos A question for you to clarify
When you said 'target the lowest supported bugfix branch. In this case this is PHP-8.1.', do you mean below?
I should create my patch based on 'PHP-8.1' branch and then make the PR against that branch rather than 'master' branch?

Yes.

someone else will cherry-pick it to 'master' branch? or should I do that?

We will merge your work into PHP-8.1 and then merge the commits upwards into 8.2 and master. So we will always make sure it gets to all branches that need the fix.

In short, all you have to do is target the 8.1 version and we will make sure it gets to all versions.

[stkeke]

[Update push.yml](/php/php-src/pull/10848/commits/d78f7672920946edeee497ec6418b93680046927)

Thanks for the quick response and clarification!
I will make a new PR again PHP-8.1.