Skip to content

AOT: string assignment uses addref instead of eager memcpy (#36192) #36279

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
PurHur merged 1 commit into from
Sep 2, 2026
Merged

AOT: string assignment uses addref instead of eager memcpy (#36192) #36279

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 25 additions & 39 deletions lib/JIT/JitValueBox.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -484,15 +484,7 @@ public static function assignToPointer(Context $context, Value $destPtr, Variabl
// Function-static / alloca slots are __string__**; writeString wants * (#31966).
$strPtr = $context->builder->load($strPtr);
}
$owned = $context->builder->call(
$context->lookupFunction('__string__separate'),
$strPtr
);
$context->builder->call(
$context->lookupFunction('__value__writeString'),
$destPtr,
$owned
);
self::writeStringToValuePtrByAddref($context, $destPtr, $strPtr);

return;
case Variable::TYPE_OBJECT:
Expand Down Expand Up @@ -568,14 +560,10 @@ public static function promoteNativeLvalueToValueBox(Context $context, Variable
$context->builder->call($context->lookupFunction('__value__writeNull'), $ptr);
break;
case Variable::TYPE_STRING:
$owned = $context->builder->call(
$context->lookupFunction('__string__separate'),
$context->helper->loadValue($var)
);
$context->builder->call(
$context->lookupFunction('__value__writeString'),
self::writeStringToValuePtrByAddref(
$context,
$ptr,
$owned
$context->helper->loadValue($var)
);
break;
case Variable::TYPE_OBJECT:
Expand Down Expand Up @@ -688,15 +676,7 @@ private static function copyBetweenPointers(Context $context, Value $destPtr, Va
$context->lookupFunction('__value__readString'),
$srcPtr
);
$owned = $context->builder->call(
$context->lookupFunction('__string__separate'),
$str
);
$context->builder->call(
$context->lookupFunction('__value__writeString'),
$destPtr,
$owned
);
self::writeStringToValuePtrByAddref($context, $destPtr, $str);
$context->builder->branch($done);

$context->builder->positionAtEnd($afterString);
Expand Down Expand Up @@ -790,6 +770,20 @@ private static function copyBetweenPointers(Context $context, Value $destPtr, Va
BasicBlockHelper::branchToFreshContinue($context, 'after_value_copy_'.$tag);
}

/**
* Share a refcounted {@see __string__} into a value box (Zend zend_string_copy semantics).
* {@see __string__separate} is for mutation / hashtable-key ownership, not assignment copy.
*/
private static function writeStringToValuePtrByAddref(Context $context, Value $destPtr, Value $strPtr): void
{
$context->refcount->addref($strPtr);
$context->builder->call(
$context->lookupFunction('__value__writeString'),
$destPtr,
$strPtr
);
}

/**
* Read boxed bool payload (writeBool stores int8 at value[0]).
* Do not use {@see __value__readLong} — no NATIVE_BOOL arm (#21892).
Expand Down Expand Up @@ -888,14 +882,10 @@ public static function valuePtrFromNativeVariable(Context $context, Variable $va
);
break;
case Variable::TYPE_STRING:
$owned = $context->builder->call(
$context->lookupFunction('__string__separate'),
$native
);
$context->builder->call(
$context->lookupFunction('__value__writeString'),
self::writeStringToValuePtrByAddref(
$context,
self::pointer($context, $slot),
$owned
$native
);
break;
case Variable::TYPE_OBJECT:
Expand Down Expand Up @@ -976,14 +966,10 @@ public static function coerceToValuePtrForStore(Context $context, Value $raw): V
}
if ('__string__*' === $tyName) {
$slot = self::alloc($context);
$owned = $context->builder->call(
$context->lookupFunction('__string__separate'),
$raw
);
$context->builder->call(
$context->lookupFunction('__value__writeString'),
self::writeStringToValuePtrByAddref(
$context,
self::pointer($context, $slot),
$owned
$raw
);

return self::pointer($context, $slot);
Expand Down
16 changes: 16 additions & 0 deletions test/fixtures/aot/cases/aot_string_copy_addref_36192.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
--TEST--
Language: AOT string assignment shares refcount (copy-on-write) — not eager memcpy (#36192)
--FILE--
<?php
$s = str_repeat('x', 64);
$u = $s;
$v = $s;
echo strlen($u), '|', strlen($v), "\n";

$a = 'hello';
$b = $a;
$a = 'bye';
echo $b, "\n";
--EXPECT--
64|64
hello
9 changes: 9 additions & 0 deletions test/repro/string_copy_addref_36192.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<?php

$s = str_repeat('x', 1 << 20);
$t = 0;
for ($i = 0; $i < 20000; $i++) {
$u = $s;
$t += strlen($u);
}
echo $t, "\n";
23 changes: 23 additions & 0 deletions test/unit/StringCopyOnWriteAotTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?php

declare(strict_types=1);

namespace PHPCompiler\Test\Unit;

use PHPUnit\Framework\TestCase;

/**
* AOT string assignment must addref (Zend zend_string_copy), not memcpy via __string__separate (#36192).
*
* @group aot-lint
*/
final class StringCopyOnWriteAotTest extends TestCase
{
public function testValueBoxCopyUsesAddrefNotSeparate(): void
{
$src = (string) file_get_contents(dirname(__DIR__, 2).'/lib/JIT/JitValueBox.php');
$this->assertStringContainsString('writeStringToValuePtrByAddref', $src);
$this->assertStringContainsString('$context->refcount->addref($strPtr)', $src);
$this->assertStringNotContainsString('lookupFunction(\'__string__separate\')', $src);
}
}
Loading