Fixes #64936 : doc comments picked up from previous scanner run #350
Conversation
Under some circumstances it was possible for doc comments to be associated with the wrong thing, as the doc_comment compiler global was carried over between scanner runs. This patch adds the doc_comment (and doc_comment_len) to zend_lex_state, so they're saved/restored when changing contexts, and then resets them whenever we start scanning a new file/string.
|
Ah, obvious bug, RESET_DOC_COMMENT() is going to free doc_comment, so the stored lex state isn't actually valid. This is less than ideal. I'm not actually sure that I need to store the doc_comments in the lex state, is there any situation under which it's useful? I'm not sure how the scanner might be reinvoked in a situation where resetting doc_comment isn't the right thing to do anyway. I'll add a fix for the obvious bug to this pull request shortly. |
doc_comment needs to be duplicated into the lex_state, otherwise it gets freed next time someone calls RESET_DOC_COMMENT().
| @@ -226,6 +226,12 @@ ZEND_API void zend_save_lexical_state(zend_lex_state *lex_state TSRMLS_DC) | |||
| lex_state->input_filter = SCNG(input_filter); | |||
| lex_state->output_filter = SCNG(output_filter); | |||
| lex_state->script_encoding = SCNG(script_encoding); | |||
|
|
|||
| lex_state->doc_comment = NULL; | |||
There was a problem hiding this comment.
why put it as null if you're going to override it 2 lines below?
|
Do we really need to preserve the doc_comment state? Wouldn't resetting it in restore_lexical_state and at the beginning of the scan solve it? I can't imagine situation where you need to preserve it. And not adding new struct would also preserve BC. |
|
I think commit 2208447 fixes it. Please reopen the bug if you see a case where it is not enough. |
|
Comment on behalf of stas at php.net: fixed |
|
Cool. I didn't want to make the decision not to store/restore it, as I wasn't sure that I wouldn't be missing something (see my second commit comment.) If you're happy that resetting it is not going to have unexpected side effects, then I'm also happy with that as the solution. Cheers! |
Under some circumstances it was possible for doc comments to be
associated with the wrong thing, as the doc_comment compiler global
was carried over between scanner runs.
This patch adds the doc_comment (and doc_comment_len) to
zend_lex_state, so they're saved/restored when changing contexts,
and then resets them whenever we start scanning a new file/string.