Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for PHP 7.3 #810

Closed
wants to merge 3 commits into from
Closed

Conversation

remicollet
Copy link

@remicollet remicollet commented Jul 4, 2018

About 1st commit, from UPGRADING.INTERNALS

  a. array_init() and array_init_size() are not functions anymore.
     They don't return any values.

About 2nd commit, -Wformat warning are strangely not raised in PHP < 7.3, but point to real issue. The commit only fix the [-Werror=format-security] with break builds with hardened build options.

About 3rd commit, from UPGRADING.INTERNALS

  k. zend_fcall_info_cache.initialized is removed. zend_fcall_info_cache is
     initialized if zend_fcall_info_cache.function_handler is set.


This change is Reviewable

@msftclas
Copy link

msftclas commented Jul 4, 2018

CLA assistant check
All CLA requirements met.

@remicollet remicollet changed the title Fix for PHP 7.3 WIP - Fix for PHP 7.3 Jul 4, 2018
@remicollet remicollet changed the title WIP - Fix for PHP 7.3 Fix for PHP 7.3 Jul 4, 2018
@remicollet
Copy link
Author

OK, work done, build is ok (extension not yet tested)

@coveralls
Copy link

coveralls commented Jul 4, 2018

Coverage Status

Coverage increased (+0.01%) to 74.711% when pulling 97f31ab on remicollet:issue-php73 into 8f9ecde on Microsoft:master.

@remicollet
Copy link
Author

sqlsrv extension seems OK (minimal connection + query test)

pdo_sqlsrv segfaults with 7.3.0alpha2 (ok with 7.0, 7.1, 7.2)

*** Error in `/opt/remi/php73/root/usr/bin/php': double free or corruption (out): 0x00007ffff385f640 ***
(gdb) bt
#0  0x00007ffff4b71207 in raise () from /lib64/libc.so.6
#1  0x00007ffff4b728f8 in abort () from /lib64/libc.so.6
#2  0x00007ffff4bb3cc7 in __libc_message () from /lib64/libc.so.6
#3  0x00007ffff4bbc429 in _int_free () from /lib64/libc.so.6
#4  0x0000555555856b24 in zend_hash_destroy (ht=0x7ffff3860230) at /usr/src/debug/php-7.3.0alpha3/Zend/zend_hash.c:1403
#5  0x00007fffe7926720 in reset (ptr=0x0, this=0x7fffffffa3e8) at /usr/src/debug/php73-php-sqlsrv-5.2.1~preview/NTS/pdo_sqlsrv/shared/core_sqlsrv.h:697
#6  ~sqlsrv_auto_ptr (this=0x7fffffffa3e8, __in_chrg=<optimized out>) at /usr/src/debug/php73-php-sqlsrv-5.2.1~preview/NTS/pdo_sqlsrv/shared/core_sqlsrv.h:513
#7  ~hash_auto_ptr (this=0x7fffffffa3e8, __in_chrg=<optimized out>) at /usr/src/debug/php73-php-sqlsrv-5.2.1~preview/NTS/pdo_sqlsrv/shared/core_sqlsrv.h:684
#8  pdo_sqlsrv_db_handle_factory (dbh=<optimized out>, driver_options=<optimized out>) at /usr/src/debug/php73-php-sqlsrv-5.2.1~preview/NTS/pdo_sqlsrv/pdo_dbh.cpp:521
#9  0x00007fffe959d716 in zim_PDO_dbh_constructor (execute_data=<optimized out>, return_value=<optimized out>) at /usr/src/debug/php-7.3.0alpha3/ext/pdo/pdo_dbh.c:358
#10 0x00005555558d7155 in ZEND_DO_FCALL_SPEC_RETVAL_UNUSED_HANDLER () at /usr/src/debug/php-7.3.0alpha3/Zend/zend_vm_execute.h:980
#11 execute_ex (ex=0x942) at /usr/src/debug/php-7.3.0alpha3/Zend/zend_vm_execute.h:54561
#12 0x00005555558d79b3 in zend_execute (op_array=op_array@entry=0x7ffff38842a0, return_value=0x0, return_value@entry=0x7ffff388f1a0)
    at /usr/src/debug/php-7.3.0alpha3/Zend/zend_vm_execute.h:59965
#13 0x0000555555847082 in zend_execute_scripts (type=type@entry=8, retval=0x7ffff388f1a0, retval@entry=0x0, file_count=-209596368, file_count@entry=3)
    at /usr/src/debug/php-7.3.0alpha3/Zend/zend.c:1564
#14 0x00005555557e6c70 in php_execute_script (primary_file=primary_file@entry=0x7fffffffcd40) at /usr/src/debug/php-7.3.0alpha3/main/main.c:2608
#15 0x00005555558d9e22 in do_cli (argc=2, argv=0x555555c6a0a0) at /usr/src/debug/php-7.3.0alpha3/sapi/cli/php_cli.c:1002
#16 0x0000555555641c3b in main (argc=2, argv=0x555555c6a0a0) at /usr/src/debug/php-7.3.0alpha3/sapi/cli/php_cli.c:1395

@yitam
Copy link
Contributor

yitam commented Jul 4, 2018

Thank you very much, @remicollet , for your contributions, which are greatly appreciated. Yet, at this point we are close to production release, thus already code frozen.

That being said, we will merge your changes in the next preview. Thanks for your patience and understanding.

@Jan-E
Copy link

Jan-E commented Jul 13, 2018

@remicollet I had a heap corruption as well on Windows
Unhandled exception at 0x0000000077A9F6B2 (ntdll.dll) in php.exe: 0xC0000374: A heap has been corrupted (parameters: 0x0000000077B07C70). occurred
Backtrace:

 	ntdll.dll!RtlReportCriticalFailure�()	Unknown
 	ntdll.dll!RtlpReportHeapFailure�()	Unknown
 	ntdll.dll!RtlpHeapHandleError�()	Unknown
 	ntdll.dll!RtlpLogHeapFailure�()	Unknown
 	ntdll.dll!RtlFreeHeap�()	Unknown
 	ucrtbase.dll!_free_base�()	Unknown
>	php7ts.dll!zend_hash_destroy(_zend_array * ht) Line 1405	C
 	[Inline Frame] php_pdo_sqlsrv.dll!hash_auto_ptr::reset(_zend_array *) Line 634	C++
 	[Inline Frame] php_pdo_sqlsrv.dll!sqlsrv_auto_ptr<_zend_array,hash_auto_ptr>::{dtor}() Line 450	C++
 	php_pdo_sqlsrv.dll!pdo_sqlsrv_db_handle_factory(_pdo_dbh_t * dbh, _zval_struct * driver_options) Line 480	C++
 	php7ts.dll!zim_PDO_dbh_constructor(_zend_execute_data * execute_data, _zval_struct * return_value) Line 358	C
 	php7ts.dll!ZEND_DO_FCALL_SPEC_RETVAL_UNUSED_HANDLER(_zend_execute_data * execute_data) Line 994	C
 	php7ts.dll!execute_ex(_zend_execute_data * ex) Line 54410	C
 	php7ts.dll!zend_execute(_zend_op_array * op_array, _zval_struct * return_value) Line 59966	C
 	php7ts.dll!zend_execute_scripts(int type, _zval_struct * retval, int file_count, ...) Line 1565	C
 	php7ts.dll!php_execute_script(_zend_file_handle * primary_file) Line 2610	C
 	php.exe!do_cli(int argc, char * * argv) Line 1003	C
 	php.exe!main(int argc, char * * argv) Line 1395	C
 	[External Code]	

@Jan-E
Copy link

Jan-E commented Jul 13, 2018

The heap corruption disappeared after commenting out this line:
https://github.com/Microsoft/msphpsql/blob/dev/source/shared/core_sqlsrv.h#L697

    // free the original pointer and assign a new pointer. Use NULL to simply free the pointer.
    void reset( _In_opt_ HashTable* ptr = NULL )
    {
        if( _ptr ) {
            // zend_hash_destroy( _ptr );
            FREE_HASHTABLE( _ptr );
        }
        _ptr = ptr;
    }

@yitam
Copy link
Contributor

yitam commented Aug 1, 2018

Closing this pull request as the suggested changes have been included in PR #822

@yitam yitam closed this Aug 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants