Child mode: report the child's exit status, and say why it failed - #164
Merged
Merged
Conversation
Three things make `phpspy -- cmd` awkward to use, all in the same area. The child's exit status is discarded. `waitpid` is called with NULL, so `phpspy -- php -r 'exit(3);'` exits 0, and so does a script that fails to parse. That makes phpspy hard to drop into a pipeline or CI job. It now reports the child's status, the way strace(1) and time(1) do: a profiling failure is still reported on stderr, but it does not mask the exit code of the command being profiled -- not least because a short-lived child routinely exits before phpspy has finished attaching, so preferring phpspy's own error would mean the status is lost precisely for the quick commands most likely to be scripted. The normal end of a run looks like an error. copy_proc_mem prints "process_vm_readv: No such process" when the target exits, which is what happens at the end of every successful child-mode run. ESRCH is now silent there and the loop says "phpspy: pid N exited" once instead. A failure to redirect child stdio does not say what it could not open. The default `-O phpspy.%d.out` / `-E phpspy.%d.err` are relative to the working directory, so running from somewhere unwritable produces a bare "fopen: Permission denied" with no path -- and then phpspy carries on, so the next thing on screen is an awk/objdump cascade from probing a child that never execed. The message now names the path and the stream, and main_fork distinguishes a child that died before exec from the expected SIGTRAP and stops there. Also fixes an unchecked short read while in copy_proc_mem: only -1 was treated as failure, so a partial read left the tail of the destination struct holding whatever the caller had memset it to. zend_execute_data and zend_function are largely pointers, which are then dereferenced as addresses in the target. Note the exit-status change is user-visible: `phpspy -- cmd` goes from always exiting 0 to exiting what cmd exited. No existing test depended on it (every child-mode target in the suite exits 0), but it is a CLI contract change. Co-authored-by: Adam Saponara <as@php.net>
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
2 participants
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.
#158 with merge conflicts fixed and the test changed to fit new test pattern.