Bug #5913
closedTail recursion overwrites value of variable
80%
Description
Arising out of a question on Slack on March 7:
Attached is a zip with test.xsl and test.xspec. The XSLT contains two functions, one of which calls the other, and the second is recursive. The XSpec tests the functions with various inputs. The process runs fine when an xsl:message is present either in the first or second function, even if the xsl:message does not read or output the content of any variables, but if all xsl:messages are commented out, the process fails with this error:
XTTE0570 An empty sequence is not allowed as the value of variable $nextLetter
which is a result of accessing a position in the variable $candidateSequence; it seems that the $prevPos variable ends up being assigned a value of 90 at some point, which is a value it should never get. This may be the result of some confusion between the position in the letter sequence ('A' through 'Z', length 26) and the codepoint of the last letter 'Z', which would be 90.
Files
Updated by Martin Honnen almost 2 years ago
XSpec 2.2.4 fails for me with both Saxon 10.9 and 11.5 (used HE) if the comment is not in the code, here is the stack of 11.5:
Type error at char 0 in expression in xsl:variable/@select on line 102 column 107 of test.xsl:
XTTE0570 An empty sequence is not allowed as the value of variable $nextLetter
at function hcmc:getNextBreakpoint on line 68 column 65 of test.xsl:
invoked by unknown caller (class net.sf.saxon.value.MemoClosure)
at function hcmc:getLocationAlphabeticBreakPoints on line 36 column 80 of test.xsl:
invoked by function call at file:/C:/Users/marti/OneDrive/Documents/xslt/blog-xslt-3-by-example/message-prevents-error/xspec/test-compiled.xsl#1632
at template scenario1-scenario1 on line 1571 column 83 of test-compiled.xsl:
invoked by xsl:call-template at file:/C:/Users/marti/OneDrive/Documents/xslt/blog-xslt-3-by-example/message-prevents-error/xspec/test-compiled.xsl#1549
at template scenario1 on line 61 column 83 of test-compiled.xsl:
invoked by xsl:for-each at file:/C:/Users/marti/OneDrive/Documents/xslt/blog-xslt-3-by-example/message-prevents-error/xspec/test-compiled.xsl#1546
at template scenario1 on line 61 column 83 of test-compiled.xsl:
invoked by xsl:call-template at file:/C:/Users/marti/OneDrive/Documents/xslt/blog-xslt-3-by-example/message-prevents-error/xspec/test-compiled.xsl#50
at template main on line 31 column 41 of test-compiled.xsl:
invoked by xsl:for-each at file:/C:/Users/marti/OneDrive/Documents/xslt/blog-xslt-3-by-example/message-prevents-error/xspec/test-compiled.xsl#47
at template main on line 31 column 41 of test-compiled.xsl:
An empty sequence is not allowed as the value of variable $nextLetter
Updated by Martin Honnen almost 2 years ago
Saxon 12 HE gives
Testing with SAXON HE 12.0
Scenarios for testing hcmc:getLocationAlphabeticBreakPoints
..Threshold in the middle of a letter
Type error in expression in xsl:variable/@select on line 102 column 107 of test.xsl:
XTTE0570 An empty sequence is not allowed as the value of variable $nextLetter
at function hcmc:getNextBreakpoint on line 68 column 65 of test.xsl:
An empty sequence is not allowed as the value of variable $nextLetter
All tests pass with the comment being present in 10.9, 11.5, 12.0.
Updated by Norm Tovey-Walsh almost 2 years ago
It can be reduced to this test which can be run with -it -xsl:...
<?xml version="1.0" encoding="UTF-8"?>
<xsl:stylesheet xmlns:xsl="http://www.w3.org/1999/XSL/Transform"
xmlns:hcmc="http://hcmc.uvic.ca/ns"
xmlns:xh="http://www.w3.org/1999/xhtml"
xmlns:xs="http://www.w3.org/2001/XMLSchema"
exclude-result-prefixes="#all"
version="3.0">
<xsl:template name="xsl:initial-template">
<xsl:variable name="testPlaceDiv1" as="element(xh:div)">
<div xmlns="http://www.w3.org/1999/xhtml">
<ul class="family">
<li><span class="name">Abe, Yoshikazu</span></li>
</ul>
</div>
</xsl:variable>
<xsl:sequence select="hcmc:getLocationAlphabeticBreakPoints($testPlaceDiv1, 10)"/>
</xsl:template>
<xsl:function name="hcmc:getLocationAlphabeticBreakPoints" as="xs:string+">
<xsl:param name="placeDiv" as="element(xh:div)"/>
<xsl:param name="threshold" as="xs:integer"/>
<xsl:variable name="candidateSequence" as="xs:string+" select="(for $i in string-to-codepoints('A')[1] to string-to-codepoints('Z')[1] return codepoints-to-string($i))"/>
<xsl:sequence
select="hcmc:getNextBreakpoint($placeDiv, $threshold, 0, $candidateSequence, 0, ())"/>
</xsl:function>
<xsl:function name="hcmc:getNextBreakpoint" as="xs:string+">
<xsl:param name="placeDiv" as="element(xh:div)"/>
<xsl:param name="threshold" as="xs:integer"/>
<xsl:param name="prevPos" as="xs:integer"/>
<xsl:param name="candidateSequence" as="xs:string+"/>
<xsl:param name="currTotal" as="xs:integer"/>
<xsl:param name="sequenceSoFar" as="xs:string*"/>
<xsl:choose>
<xsl:when test="$prevPos ge (count($candidateSequence) - 1)">
<xsl:choose>
<xsl:when test="($currTotal + count($placeDiv//xh:li[xh:span[@class='name'][starts-with(., $candidateSequence[last()])]])) gt $threshold">
<xsl:sequence select="($sequenceSoFar, $candidateSequence[last()])"/>
</xsl:when>
<xsl:otherwise>
<xsl:sequence select="$sequenceSoFar"/>
</xsl:otherwise>
</xsl:choose>
</xsl:when>
<xsl:when test="empty($sequenceSoFar) and $prevPos lt 1">
<xsl:sequence select="hcmc:getNextBreakpoint($placeDiv, $threshold, 1, $candidateSequence, count($placeDiv//xh:li[xh:span[@class='name'][starts-with(., $candidateSequence[1])]]), $candidateSequence[1])"/>
</xsl:when>
<xsl:otherwise>
<xsl:variable name="nextLetter" as="xs:string" select="$candidateSequence[$prevPos + 1]"/>
<xsl:variable name="totalForNextLetter" as="xs:integer" select="count($placeDiv//xh:li[xh:span[@class='name'][starts-with(., $nextLetter)]])"/>
<xsl:variable name="newTotal" as="xs:integer" select="$currTotal + $totalForNextLetter"/>
<xsl:choose>
<xsl:when test="($newTotal gt $threshold)">
<xsl:sequence select="hcmc:getNextBreakpoint($placeDiv, $threshold, $prevPos + 1, $candidateSequence, $totalForNextLetter, (($sequenceSoFar, if ($totalForNextLetter gt 0) then $candidateSequence[($prevPos + 1)] else ())))"/>
</xsl:when>
<xsl:otherwise>
<!-- Otherwise, we just move up one. -->
<xsl:sequence select="hcmc:getNextBreakpoint($placeDiv, $threshold, ($prevPos + 1), $candidateSequence, $newTotal, $sequenceSoFar)"/>
</xsl:otherwise>
</xsl:choose>
</xsl:otherwise>
</xsl:choose>
</xsl:function>
</xsl:stylesheet>
The test conditions are remarkably sensitive at this point. Removing apparently unnecessary types (such as xs:string
from $nextLetter
) or even replacing the construction of the candidate sequence with a literal sequence avoids the error.
Updated by Martin Honnen almost 2 years ago
When I run Norm's reduced test case with Saxon 12.0 HE and tail call optimization disabled (i.e. -opt:-t
) it does not give the error. The same happens with HE 11.5 and HE 10.9.
Updated by Michael Kay almost 2 years ago
We've worked through this together and have established what's going wrong; it remains to decide how to fix it.
We're basically getting an interference between two optimisations:
-
We're doing a lazy evaluation of the expression
for $i in 65 to 90 return codepoints-to-string($i)
(the fact that we could be smarter and evaluate this as a constant expression is immaterial to this bug) -
The call on line 27 is a (different-function) tail call, so the stack frame of the caller is reused and overwritten (this gives benefits when a group of functions call each other in mutual recursion).
Normally, if an expression contains local variable references, then when we defer it for lazy evaluation (in a MemoClosure
) we save a copy of anything in the context that it depends on. But this expression is identified as having no context dependencies, so we avoid the cost of making this copy. However, although it doesn't depend on the context, it does make use of the local stack-frame for evaluating and temporarily keeping the value of $i
. After evaluating the expression, the stack frame slot holding $i
will have the value 90, and because the stack frame has been shared with the tail-called function, this has the effect of clobbering the value of $prevPos
.
Updated by Michael Kay almost 2 years ago
A sufficient solution is to change Closure.saveContext() so it creates a new (all-null) stack frame in the saved context if the expression contains a subexpression that declares local range variables (specifically, an Assignation
or a FLWORExpression
).
However, this is a rather expensive search to do at run-time. The answer to this seems to be to allocate an extra dependency flag, StaticProperty.DEPENDS_ON_OWN_RANGE_VARIABLES
which is maintained in the static properties word of every expression.
Updated by Michael Kay almost 2 years ago
- Subject changed from Saxon 10: error when running without xsl:message to Tail recursion overwrites value of variable
- Category set to Internals
- Status changed from New to In Progress
- Assignee set to Michael Kay
- Applies to branch 11, 12, trunk added
- Fix Committed on Branch 11, 12, trunk added
- Platforms .NET, Java added
Updated by Michael Kay almost 2 years ago
I've added this as test case function-2109 - but unfortunately it doesn't fail when run from the test suite driver, it only fails when run from the command line. As we've seen, it's very sensitive to the input conditions.
Updated by Michael Kay almost 2 years ago
- Status changed from In Progress to Resolved
- Fix Committed on Branch 10 added
Updated by O'Neil Delpratt almost 2 years ago
- Status changed from Resolved to Closed
- % Done changed from 0 to 100
- Fixed in Maintenance Release 12.1 added
Bug fix applied in the Saxon 12.1 maintenance release.
Updated by Debbie Lockett about 1 year ago
- Status changed from Closed to Resolved
- % Done changed from 100 to 80
- Fixed in Maintenance Release 11.6 added
The bug fix was also applied in the Saxon 11.6 maintenance release. Bug leaving bug "resolved" rather than "closed" as the fix has not gone out in a Saxon 10 maintenance release.
Please register to edit this issue