Repository navigation
Fixes GH-24582. Supersedes #24591, reworked along the lines suggested by @devnexen. - #24630
Open
matyapiro31 wants to merge 1 commit into
Open
matyapiro31 wants to merge 1 commit into
matyapiro31 wants to merge 1 commit into
Conversation
…bails out zend_ffi_finalize_type() reported unsupported type specifier combinations (e.g. _Complex) through zend_ffi_parser_error(), which longjmp()s out of the parser. Several callbacks (zend_ffi_make_pointer_type(), zend_ffi_make_func_type(), zend_ffi_add_arg(), zend_ffi_nested_declaration()) call it while already holding memory that is not owned by the declaration yet, so that memory was leaked on every failed FFI::cdef()/FFI::new()/FFI::type(). Do not bail out from zend_ffi_finalize_type(). Report the error, set FFI_G(parse_failed) and continue with a placeholder type. Later parser errors are not reported again, and zend_ffi_parse_decl()/zend_ffi_parse_type() return FAILURE once parsing is done, so every callback can finish and release what it owns through its normal path. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
zend_ffi_finalize_type()reported unsupported type specifier combinations(e.g. _Complex) with
zend_ffi_parser_error(). That function ends inLONGJMP(FFI_G(bailout), ...), which jumps straight back to theSETJMPinzend_ffi_parse_decl()/zend_ffi_parse_type()and skips every cleanup in betweenzend_ffi_make_pointer_type()zend_ffi_make_func_type()zend_ffi_add_arg()andzend_ffi_nested_declaration()call
zend_ffi_finalize_type()while holding memorythat the declaration (dcl) does not own yet, so that memory was never freed:
every failed
FFI::cdef()/new()/type()leaked it.This no longer bails out from
zend_ffi_finalize_type(){: it reports the error, setsFFI_G(parse_failed)and continues with a placeholder type. Further parser errors are not reported again (only the first one is), andzend_ffi_parse_decl()andzend_ffi_parse_type()` return FAILURE once parsing is done. The callbacksare unchanged and release their memory through their normal paths.
ffi_parser.c is hand-synced with ffi.g (not regenerated).
Tested on a debug ZTS build (8.4.28-dev): ext/ffi/tests 87 pass / 0 fail. The new
test fails without the fix. Leak per failed
cdef()is 0 B for all of the casesbelow (before: 96-312 B), including declarations with more declarations after
the error, struct members and nested declarators.