Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
14 changes: 9 additions & 5 deletions ext/dom/JitDomNodeListItem.php
Original file line number Diff line number Diff line change
Expand Up @@ -98,11 +98,14 @@ private static function rememberCompileTimeChildIndex(Context $context, JITVaria
return;
}

// getElementsByTagName NodeList: item($N) is the Nth tag match in document
// order, not parent->childNodes[$N]. Using $N as a direct-child index made
// replaceChild InnerXml replace the wrong sibling (#34780).
// getElementsByTagName / simple DOMXPath //tag NodeLists: item($N) is the Nth
// tag match in document order, not parent->childNodes[$N]. Using $N as a
// direct-child index made replaceChild InnerXml replace the wrong sibling
// (#34780) and stamped XPath item() with sibling-0 attrs so setIdAttribute
// read id=x for //b and SIGSEGV'd (#35447 leftover #35433).
$tagQuery = JitDomGetElementsByTagNameUserScript::lastTagQuery()
?? JitDomGetElementsByTagNameUserScript::liveItemTagQuery();
?? JitDomGetElementsByTagNameUserScript::liveItemTagQuery()
?? JitDomXPathQueryUserScript::lastQueryTag();
if (null !== $tagQuery) {
self::rememberTagListItemChildIndex($xml, $tagQuery, $index);

Expand All @@ -124,7 +127,8 @@ private static function rememberCompileTimeChildIndex(Context $context, JITVaria
}

/**
* Map getElementsByTagName()->item($N) to a direct-child index for InnerXml (#34780).
* Map getElementsByTagName() / simple XPath //tag item($N) to a direct-child
* index for InnerXml and setIdAttribute attr stamps (#34780 / #35447).
*/
private static function rememberTagListItemChildIndex(string $xml, string $tagQuery, int $index): void
{
Expand Down
71 changes: 69 additions & 2 deletions ext/dom/JitDomNodeListItemUserScript.php
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,47 @@ public static function tryInvoke(Context $context, JITVariable ...$args): ?Value
}
}
if (null !== $xml && null !== $queryTag && '' !== $queryTag) {
if (JITVariable::TYPE_NATIVE_LONG === $arg->type) {
$indexVal = $context->helper->loadValue($arg);
} elseif (JITVariable::TYPE_VALUE === $arg->type) {
$indexVal = $context->builder->call(
$context->lookupFunction('__value__readLong'),
JitValueBox::valuePtrFromVariable($context, $arg)
);
} else {
$indexVal = null;
}
if (null !== $indexVal) {
$pinned = DomUserScriptPinnedRootLlvm::load($context);
$live = JitDomLiveElementsByTagWalk::itemAt(
$context,
$pinned,
$queryTag,
$indexVal,
false
);
$compileTime = self::materializeDynamicIndexQueryMatch(
$context,
$xml,
$queryTag,
$arg
);
$objPtrTy = $context->getTypeFromString('__object__*');
$pinNull = $context->builder->icmp(
Builder::INT_EQ,
$pinned,
$objPtrTy->constNull()
);
$liveObj = $context->builder->call(
$context->lookupFunction('__value__readObject'),
$live
);
$liveNull = $context->builder->icmp(Builder::INT_EQ, $liveObj, $objPtrTy->constNull());
$preferRemat = $context->builder->or($pinNull, $liveNull);

return $context->builder->select($preferRemat, $compileTime, $live);
}

return self::materializeDynamicIndexQueryMatch($context, $xml, $queryTag, $arg);
}
if (null !== $tagQuery && null !== $itemMarkup) {
Expand All @@ -137,9 +178,35 @@ public static function tryInvoke(Context $context, JITVariable ...$args): ?Value
return null;
}

// XPath //tag (and predicate) lists: materialize Nth match with attrs (#27275).
// XPath //tag lists: prefer live pinned-root walk so item() keeps
// ownerDocument for setIdAttribute (#35447). Always rematerializing
// (#27275) returned a detached clone — getAttribute worked via Attr
// presence, but NestedJIT setIdAttribute SIGSEGV'd.
if (null !== $xml && null !== $queryTag && '' !== $queryTag) {
return self::materializeNthQueryMatch($context, $xml, $queryTag, $index);
$pinned = DomUserScriptPinnedRootLlvm::load($context);
$i64 = $context->getTypeFromString('int64');
$live = JitDomLiveElementsByTagWalk::itemAt(
$context,
$pinned,
$queryTag,
$i64->constInt($index, false),
false
);
$compileTime = self::materializeNthQueryMatch($context, $xml, $queryTag, $index);
$objPtrTy = $context->getTypeFromString('__object__*');
$pinNull = $context->builder->icmp(
Builder::INT_EQ,
$pinned,
$objPtrTy->constNull()
);
$liveObj = $context->builder->call(
$context->lookupFunction('__value__readObject'),
$live
);
$liveNull = $context->builder->icmp(Builder::INT_EQ, $liveObj, $objPtrTy->constNull());
$preferRemat = $context->builder->or($pinNull, $liveNull);

return $context->builder->select($preferRemat, $compileTime, $live);
}

// getElementsByTagNameNS live list — prefer pinned-root walk (#34995 / re-#34983).
Expand Down
8 changes: 5 additions & 3 deletions lib/JIT.php
Original file line number Diff line number Diff line change
Expand Up @@ -16963,9 +16963,11 @@ private function propagateDomCreateDocumentTypeCompileTimeTag(Operand $result):
* Without this index, {@see \PHPCompiler\ext\dom\JitDomReplaceChild} leaves seeded InnerXml
* unchanged so serialization keeps the replaced sibling.
*
* getElementsByTagName()->item($N) is the Nth **tag match**, not childNodes[$N]. Using the
* raw NodeList index as {@see JIT\Variable::$compileTimeDomChildIndex} stamped tag `a` for
* `getElementsByTagName('b')->item(0)` and setIdAttribute registered id `x` on `<b>` (#35433
* getElementsByTagName()/XPath //tag ->item($N) is the Nth **tag match**, not
* childNodes[$N]. Using the raw NodeList index as
* {@see JIT\Variable::$compileTimeDomChildIndex} stamped tag `a` for
* `getElementsByTagName('b')->item(0)` / `query('//b')->item(0)` and
* setIdAttribute registered id `x` on `<b>` (or SIGSEGV; #35433 / #35447
* re-#33957). Prefer {@see \PHPCompiler\ext\dom\JitDomNodeListItem::$lastFetchedChildIndex}
* (mapped in {@see \PHPCompiler\ext\dom\JitDomNodeListItem::rememberTagListItemChildIndex},
* #34780).
Expand Down
26 changes: 26 additions & 0 deletions test/repro/aot_dom_xpath_item_setidattribute.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<?php
// #35447 — AOT: DOMXPath query()->item()->setIdAttribute must use live node + tag attrs
// php-src: ext/dom/xpath.c query; ext/dom/element.c setIdAttribute → xmlAddID
$d = new DOMDocument();
$d->loadXML('<r><a id="x">1</a><b id="y">2</b></r>');
$xp = new DOMXPath($d);
$e = $xp->query('//b')->item(0);
if ($e === null) {
echo "tag=null\n";
} else {
echo 'tag=', $e->nodeName, "\n";
echo 'attr=', $e->getAttribute('id'), "\n";
$e->setIdAttribute('id', true);
}
$hit = $d->getElementById('y');
if ($hit === null) {
echo "y=null\n";
} else {
echo 'y=', $hit->nodeName, "\n";
}
$miss = $d->getElementById('x');
if ($miss === null) {
echo "x=null\n";
} else {
echo 'x=', $miss->nodeName, "\n";
}
64 changes: 64 additions & 0 deletions test/unit/DomXPathItemSetIdAttribute35447AotTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
<?php

declare(strict_types=1);

use PHPUnit\Framework\TestCase;

/**
* AOT: DOMXPath query()->item()->setIdAttribute uses tag-match attrs (#35447).
*
* php-src: ext/dom/xpath.c — query node-set; ext/dom/element.c — setIdAttribute
*
* @group llvm
* @group aot
*/
final class DomXPathItemSetIdAttribute35447AotTest extends TestCase
{
public function testXPathItemSetIdAttributeMatchesZend(): void
{
$src = __DIR__.'/../repro/aot_dom_xpath_item_setidattribute.php';
$zend = $this->runPhp($src);
$aot = $this->runAot($src);
$this->assertSame($zend, $aot);
$this->assertStringContainsString('tag=b', $aot);
$this->assertStringContainsString('attr=y', $aot);
$this->assertStringContainsString('y=b', $aot);
$this->assertStringContainsString('x=null', $aot);
}

/** getElementsByTagName path from #35433 must stay green. */
public function testGetElementsLaterSiblingStillMatchesZend(): void
{
$src = __DIR__.'/../repro/dom_setidattribute_getelementbyid_aot.php';
$zend = $this->runPhp($src);
$aot = $this->runAot($src);
$this->assertSame($zend, $aot);
$this->assertStringContainsString('y=b', $aot);
$this->assertStringContainsString('x=null', $aot);
}

private function runPhp(string $src): string
{
$cmd = escapeshellarg(PHP_BINARY).' '.escapeshellarg($src);
exec($cmd.' 2>&1', $out, $rc);
$this->assertSame(0, $rc, implode("\n", $out));

return implode("\n", $out);
}

private function runAot(string $src): string
{
$root = dirname(__DIR__, 2);
$bin = sys_get_temp_dir().'/dom_xpath_setid_35447_'.getmypid().'_'.mt_rand();
$cmd = escapeshellarg(PHP_BINARY).' '.escapeshellarg($root.'/bin/compile.php')
.' -o '.escapeshellarg($bin).' '.escapeshellarg($src);
exec($cmd.' 2>&1', $compOut, $compRc);
$this->assertSame(0, $compRc, implode("\n", $compOut));
$this->assertFileExists($bin);
exec(escapeshellarg($bin).' 2>&1', $out, $rc);
@unlink($bin);
$this->assertSame(0, $rc, implode("\n", $out));

return implode("\n", $out);
}
}
Loading