Skip to content

[bug] XSLT::Stylesheet#transform corrupts params after GC compaction — build_xslt_params holds char* across the loop #3670

Description

@jeremy

Unrelated to #3667, which is about the stylesheet's _private. This is the params array in
the same file.

ext/nokogiri/xslt_stylesheet.c carries an explicit note that this is safe:

/*
 * Note: params[j] is a raw pointer into a Ruby string's buffer, and we do not pin the underlying
 * VALUEs against GC compaction. This is safe (despite not pinning the VALUEs) because libxslt fully
 * processes params (interning names, evaluating values) before template execution begins, and Ruby
 * callbacks can only run during template execution. By the time GC compaction is reachable, libxslt
 * no longer reads params[].
 */

That reasoning covers the window after the loop, and as far as I can measure it holds there.
But there is a window inside the loop that it doesn't cover, and a compaction there corrupts
every pointer already stored.

Please describe the bug

static VALUE
build_xslt_params(VALUE args_ptr)
{
  build_xslt_params_args_t *args = (build_xslt_params_args_t *)args_ptr;

  for (long j = 0; j < args->param_len; j++) {
    VALUE entry = rb_ary_entry(args->rb_param, j);
    args->params[j] = StringValueCStr(entry);
  }

  return Qnil;
}

params is a ruby_xcalloc'd array, so the char * stored at iteration j outlives that
iteration. entry is a loop local, reassigned on the next pass, so for every j' < j there
is no VALUE anywhere in the frame for the String that params[j'] points into. The Strings
stay alive — rb_param holds them — but nothing pins them.

And StringValueCStr(entry) is a Ruby-callback site whenever entry isn't already a String:
it goes through rb_str_to_str#to_str. Any allocation there can be a compacting GC when
GC.auto_compact is on. Strings shorter than the embedded boundary (616 on ruby 4.0) keep
their bytes inside the object slot, so they move with it, and every params[j'] already
stored is left pointing at a vacated slot.

XSLT param names are short by nature, so in practice it's the name that goes.

Help us reproduce what you're seeing

require "nokogiri"

class CompactingParam            # any object with #to_str reaches this
  def to_str
    GC.verify_compaction_references(expand_heap: true, toward: :empty)
    "'ok'"
  end
end

stylesheet = Nokogiri::XSLT.parse(<<~XSLT)
  <?xml version="1.0"?>
  <xsl:stylesheet version="1.0" xmlns:xsl="http://www.w3.org/1999/XSL/Transform">
    <xsl:param name="title"/>
    <xsl:param name="trigger"/>
    <xsl:template match="/"><out><xsl:value-of select="$title"/></out></xsl:template>
  </xsl:stylesheet>
XSLT

params = ["title", "'Employee List'", "trigger", CompactingParam.new]

puts stylesheet.transform(Nokogiri::XML("<r/>"), params).to_xml

Actual:

Invalid expression
runtime error
Evaluating user parameter  failed
 (RuntimeError)

Note the empty parameter name in libxslt's own message — params[0] now points at a
zero-filled vacated slot.

It does not always raise. If anything reuses the vacated slot before libxslt reads it —
which is just ordinary allocation — the param silently resolves to nothing and transform
returns a document with a missing value and no error at all:

<?xml version="1.0"?>
<out/>

3/3, from adding 20.times { 2_000.times { +("Z" * 10) } }; GC.start after the compaction
inside #to_str. I mention it because it is the manifestation a regression test is most
likely to meet, and because a silently wrong transform is worse than the exception.

Expected, and what you get with the GC.verify_compaction_references line removed (I ran that
as a control):

<?xml version="1.0"?>
<out>Employee List</out>

3/3 with, 0/3 without. One preceding param pair is enough; it isn't a many-params edge case.

Nothing about how the params are held is contrived, and it's worth saying why, because it is
the part that makes this reach ordinary code. A local variable in a live Ruby frame does pin
its object — I checked, and it holds for method locals, block locals, and locals captured by a
lambda or by binding (pinned 3/3 on ruby 4.0.6 and 3.4.10). But an element of an Array is
not a local
, and it is not pinned even when the Array itself is a live method local: that
relocates 3/3 in the same test. The params argument is always an Array (or a Hash that
transform immediately converts to one), so its entries are never pinned, wherever the caller
keeps it. Wrapping the whole reproduction in a method changes nothing.

The trigger doesn't have to call GC

The reproducer above is explicit for legibility, but nothing needs to touch GC. With
GC.auto_compact = true and ordinary allocation inside #to_str — no GC.compact, no
GC.start, no verify_compaction_references anywhere in the process — it corrupts 3/3 the
same way. So the operator dependency is just GC.auto_compact, plus one params entry that
isn't already a String.

I ran four independent in-loop triggers, three runs each:

what runs the GC inside the loop result
GC.verify_compaction_references in #to_str 3/3 corrupt
GC.compact in #to_str 3/3 corrupt
GC.auto_compact = true + GC.start in #to_str 3/3 corrupt
GC.auto_compact = true + plain allocation in #to_str, no GC call at all 3/3 corrupt

Controls

Three, each isolating one variable, three runs each:

control result what it shows
no compaction 0/3 baseline
move the coercing entry to index 0, so nothing precedes it 0/3 the defect is inside the loop — the comment's window is genuinely fine
names and values ≥ 616 bytes 0/3 mobility, not liveness — and the object slots still relocated 16/16 in these runs; a String at or above the embedded boundary keeps its bytes in a malloc'd buffer that compaction doesn't move

That last row is the one to watch when writing a regression test: a test using long params
will pass on a broken build.
It has to use short, embedded strings.

Which operand corrupts follows the same rule — whichever one is embedded:

libxslt reports
short names (4 B) + long values (700 B) Global parameter already defined — the name is now empty
long names (700 B) + short values (100 B) Evaluating user parameter p000xxxxx… failed — name intact, the value is gone
both long correct output

What I could not settle, and it may matter more than the above

The comment's premise is that "Ruby callbacks can only run during template execution". That's
true of callbacks — but nokogiri installs Ruby's allocator into libxml2 in
nokogiri.c:177:

xmlMemSetup((xmlFreeFunc)ruby_xfree, (xmlMallocFunc)ruby_xmalloc, (xmlReallocFunc)ruby_xrealloc, ruby_strdup);

(VERSION_INFO reports "memory_management": "ruby".) So libxslt's own allocations are
Ruby GC points, including the ones inside xsltEvalUserParams — which run while params[]
is being read, before template execution. That's a second window the comment's reasoning
doesn't reach.

I could not get a corruption there: with every params entry a plain String, under GC.stress

  • GC.auto_compact, I measured 0/3. But I don't think that clears it, and I'd rather say so
    than imply otherwise — arming GC.stress compacts the whole heap before the transform is
    entered, so by the time xsltEvalUserParams runs there is nothing left to relocate (8/16
    subjects did relocate in those runs, so it wasn't a dead run, but the moves landed outside
    the window). I don't have an instrument that can put a compaction inside that window. You
    may be able to rule it in or out from the libxslt side faster than I can from Ruby.

What an operator would have to do

GC.auto_compact = true and nothing else. That is worth stating precisely because it differs
from the two IO issues I've filed alongside this: there the subject is a T_DATA IO object,
which I could not relocate with anything but the debug API. Here the subjects are Strings,
and Strings relocate under ordinary compaction — which is why the KIND=compact,
KIND=gcstart and KIND=alloc rows above are real, and why the last of them needs no GC
call anywhere in the process.

Reproduced here on the precompiled 1.19.4-arm64-darwin gem. Not a linked-library artifact as
far as I can tell — the mechanism is entirely on nokogiri's side of the boundary; libxslt just
reads the pointers it was handed.

Expected behavior

transform returns the same document whether or not a compaction happens while the params
array is being built.

Environment

nokogiri 1.19.4 (arm64-darwin, precompiled) — packaged libxml2 2.13.9, libxslt 1.1.43
ruby 4.0.6 (2026-07-14 revision 03b6d3f889) +PRISM [arm64-darwin23]  (embedded boundary 616)

Present on main as of 04a4c29: ext/nokogiri/xslt_stylesheet.c:157-160, unchanged. The
loop predates b81d745b; that commit moved it under rb_protect for the leak fix and added
the note quoted above.

Possible fix

Two shapes, both of which close the in-loop window:

  1. Coerce first, derive second. One pass calling rb_String/StringValue on every entry
    into a fresh rb_ary_new (or write the coerced values back into a protected array), then a
    second pass taking StringValueCStr with no coercion left to run. No allocation between
    the first StringValueCStr and the last, so nothing can compact mid-array.
  2. Copy the bytes. params[] is already an owned ruby_xcalloc allocation freed
    immediately after xsltApplyStylesheet; ruby_strduping each string into it and freeing
    them alongside removes the aliasing entirely, and would close the xsltEvalUserParams
    window above as well, whatever the answer to that question turns out to be.

I've not built either — I only have the precompiled gem here — so I can't offer a
before/after the way #3667 does. Happy to send a PR with whichever you prefer, plus a
regression test in test/test_compaction.rb; it needs short params, an unpinned array, and a
coercing entry after at least one real pair.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions