Skip to content

Sync with master#3

Open
odoucet wants to merge 15 commits into
odoucet:masterfrom
phacility:master
Open

Sync with master#3
odoucet wants to merge 15 commits into
odoucet:masterfrom
phacility:master

Conversation

@odoucet
Copy link
Copy Markdown
Owner

@odoucet odoucet commented Feb 5, 2016

No description provided.

epriestley and others added 15 commits August 27, 2014 11:31
Summary:
See <#40>. This is currently broken and extremely insecure.

We may want to restore it eventually, but understanding composer is very complex (no one who touched this realized that the package was owned by someone unrelated to the project who can apparently redirect it at will with no accountability). No one on the ticket seems to have any reason why this isn't totally wide open, and I haven't gotten in touch with anyone in `#composer-dev`.

Test Plan: N/A

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D10365
…HProf

Summary: Modernize this stuff a bit.

Test Plan: Ran `arc diff` to produce this diff. Read `CREDITS`.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D10366
Summary: See <#42>.

Reviewed by: epriestley
Summary: Add a .arclint file and fix some stray tabs and whitespace issues.

Test Plan: Built xhprof, ran tests (some failures, but preexisting).

Reviewers: chad, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D10372
Summary:
This allows "arc unit" to more-or-less run the PHP extension tests in an approximately correct way.

Note that there are two test failures at HEAD on recent PHP, and they've been failing for some time.

Test Plan: Used `arc unit` to run some tests.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D10373
Summary:
This adds a failing test case for the bug discussed in <#33>.

It also adds a `bin/xhprofile` script which makes it easier to test stuff like this, by invoking the profiler on some other script. This isn't hugely useful in production but is valuable diagnostically, and helped me reduce this test case.

Test Plan:
  - Ran `arc unit` and got a failure.
  - Verified that this test passes if `class_exists()` is commented out (which makes the test not segfault).

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D10374
Summary:
Fixes <#33>. Since the issue with that patch was CLA, I developed this independently.

This works because it just inlines the body of `execute_internal()`, which is nearly identical:

```
ZEND_API void execute_internal(zend_execute_data *execute_data_ptr, zend_fcall_info *fci, int return_value_used TSRMLS_DC)
{
	if(fci != NULL) {
		((zend_internal_function *) execute_data_ptr->function_state.function)->handler(fci->param_count,
				*fci->retval_ptr_ptr, fci->retval_ptr_ptr, fci->object_ptr, 1 TSRMLS_CC);

	} else {
		zval **return_value_ptr = &EX_TMP_VAR(execute_data_ptr, execute_data_ptr->opline->result.var)->var.ptr;
		((zend_internal_function *) execute_data_ptr->function_state.function)->handler(execute_data_ptr->opline->extended_value, *return_value_ptr,
					(execute_data_ptr->function_state.function->common.fn_flags & ZEND_ACC_RETURN_REFERENCE)?return_value_ptr:NULL,
					execute_data_ptr->object, return_value_used TSRMLS_CC);
	}
}
```

Test Plan: Failing test now passes. No more segfaults on PHP 5.5.8.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D10375
Summary:
There are two broken tests. One (007) is easy to fix. It was failing because of array-to-string conversion warnings.

Since I didn't want to add function calls because that would change the test output, I just silenced the warnings with "@".

The other test (004) was also easy to fix but I can't really explain what's going on there. Seems like it works though? It shouldn't do anything bad, since we would have generated a meaningless "???_op" otherwise.

Test Plan: Ran tests, tests passed.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D10376
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.

6 participants