Skip to content
Permalink
Browse files

Fix self recursion in compact

Summary: There were no checks at all.

Reviewed By: alexmalyshev

Differential Revision: D3623763

fbshipit-source-id: 9d708deca05bbd121503e8f323b4f295fde8e835
  • Loading branch information...
markw65 authored and Hhvm Bot committed Aug 1, 2016
1 parent 05e706d commit e264f04ae825a5d97758130cf8eec99862517e7e
@@ -379,10 +379,10 @@ static void php_array_merge(Array &arr1, const Array& arr2) {
arr1.merge(arr2);
}

static bool couldRecur(const Variant& v, const Array& arr) {
static bool couldRecur(const Variant& v, const ArrayData* arr) {
return v.isReferenced() ||
arr.get()->kind() == ArrayData::kGlobalsKind ||
arr.get()->kind() == ArrayData::kProxyKind;
arr->kind() == ArrayData::kGlobalsKind ||
arr->kind() == ArrayData::kProxyKind;
}

static void php_array_merge_recursive(PointerSet &seen, bool check,
@@ -403,7 +403,7 @@ static void php_array_merge_recursive(PointerSet &seen, bool check,
Variant &v = arr1.lvalAt(key, AccessFlags::Key);
auto subarr1 = v.toArray().copy();
php_array_merge_recursive(seen,
couldRecur(v, subarr1),
couldRecur(v, subarr1.get()),
subarr1,
value.toArray());
v.unset(); // avoid contamination of the value that was strongly bound
@@ -593,7 +593,7 @@ static void php_array_replace_recursive(PointerSet &seen, bool check,
if (v.isArray()) {
Array subarr1 = v.toArray();
const ArrNR& arr_value = value.toArrNR();
php_array_replace_recursive(seen, couldRecur(v, subarr1),
php_array_replace_recursive(seen, couldRecur(v, subarr1.get()),
subarr1, arr_value);
v = subarr1;
} else {
@@ -1217,11 +1217,19 @@ bool HHVM_FUNCTION(array_walk,
return true;
}

static void compact(VarEnv* v, Array &ret, const Variant& var) {
static void compact(PointerSet& seen,
VarEnv* v, Array &ret, const Variant& var) {
if (var.isArray()) {
for (ArrayIter iter(var.getArrayData()); iter; ++iter) {
compact(v, ret, iter.second());
auto adata = var.getArrayData();
auto check = couldRecur(var, adata);
if (check && !seen.insert(adata).second) {
raise_warning("compact(): recursion detected");
return;
}
for (ArrayIter iter(adata); iter; ++iter) {
compact(seen, v, ret, iter.secondRef());
}
if (check) seen.erase(adata);
} else {
String varname = var.toString();
if (!varname.empty() && v->lookup(varname.get()) != NULL) {
@@ -1237,8 +1245,9 @@ Array HHVM_FUNCTION(compact,
Array ret = Array::attach(PackedArray::MakeReserve(args.size() + 1));
VarEnv* v = g_context->getOrCreateVarEnv();
if (v) {
compact(v, ret, varname);
compact(v, ret, args);
PointerSet seen;
compact(seen, v, ret, varname);
if (!args.empty()) compact(seen, v, ret, args);
}
return ret;
}
@@ -1250,8 +1259,9 @@ Array HHVM_FUNCTION(__SystemLib_compact_sl,
Array ret = Array::attach(PackedArray::MakeReserve(args.size() + 1));
VarEnv* v = g_context->getOrCreateVarEnv();
if (v) {
compact(v, ret, varname);
compact(v, ret, args);
PointerSet seen;
compact(seen, v, ret, varname);
if (!args.empty()) compact(seen, v, ret, args);
}
return ret;
}
@@ -15,6 +15,7 @@ function main() {
test($a);
test($GLOBALS);
var_dump(compact($a));
}
main();
@@ -1,4 +1,3 @@

Warning: array_replace_recursive(): recursion detected in %s/test/slow/array_functions/self_recursive.php on line 6

Warning: array_merge_recursive(): recursion detected in %s/test/slow/array_functions/self_recursive.php on line 9
@@ -10,3 +9,7 @@ Warning: array_merge_recursive(): recursion detected in %s/test/slow/array_funct
Warning: array_merge_recursive(): recursion detected in %s/test/slow/array_functions/self_recursive.php on line 9

Warning: array_merge_recursive(): recursion detected in %s/test/slow/array_functions/self_recursive.php on line 9

Warning: compact(): recursion detected in %s/test/slow/array_functions/self_recursive.php on line 18
array(0) {
}

0 comments on commit e264f04

Please sign in to comment.
You can’t perform that action at this time.