mruby-regexp: do not truncate a stored capture name length - #7035

compile_atom() stores a named capture's name length in a uint16_t and
truncates to it with a cast rather than rejecting the name. The truncation is
silent, and every consumer of the stored name then works from the truncated
bytes: the pattern still compiles, the group still captures, but its name is no
longer the name that was written.

n = "A" * 65539
re = Regexp.new("(?<#{n}>x)")
re.named_captures.keys.map(&:length)  # CRuby: [65539], mruby: [3]
re.match("x")[n]                      # CRuby: "x", mruby: IndexError
re.match("x")["AAA"]                  # CRuby: IndexError, mruby: "x"

The group exists and matched, but it cannot be reached by the name it was
given, and it answers to a three-byte name that appears nowhere in the pattern.

The IndexError on the second row is the current answer, not the historical
one: the lookup for a name longer than 65535 bytes used to read past the named
capture arena, and #7002 bounded it. That fix is about the read, and it leaves
the truncation this pull request is about exactly where it was.

Two consumers disagree about the same name

The truncation is not confined to lookup by name. It also lets two groups
collapse onto one stored name, and the two APIs that resolve a name then pick
different groups.

long = "ab" + "A" * 65536             # 65538 bytes, (uint16_t)65538 == 2
re = Regexp.new("(?<ab>x)(?<#{long}>y)")
re.named_captures
# CRuby: {"ab" => [1], "abAAA..." => [2]}
# mruby: {"ab" => [2]}
md = re.match("xy")
md["ab"]                              # mruby: "x", which is group 1
md.named_captures                     # mruby: {"ab" => "y"}, which is group 2

MatchData#[] scans the table and stops at the first entry, so "ab" resolves
to group 1. The @named_captures table and MatchData#named_captures are
Hashes keyed by the stored name and let the later entry overwrite the earlier
one, so the same name resolves to group 2 through Regexp#named_captures,
which derives from that table. One name, two answers, in one pattern.

\k binds to the wrong group

The same collapse reaches the matcher, where it changes what the pattern
accepts.

long = "ab" + "A" * 65536
re = Regexp.new("(?<ab>x)(?<#{long}>y)\\k<#{long}>")
re.match("xyx")                       # CRuby: nil, mruby: matches
re.match("xyy")                       # CRuby: matches, mruby: nil

The backreference was written against the second group and binds to the first.
This is a wrong answer from a pattern that compiled without complaint, not a
diagnostic gap.

A name of exactly 65536 bytes becomes the empty name

(uint16_t)65536 == 0, so the name is truncated away entirely and the group
answers to "".

z = "Z" * 65536
re = Regexp.new("(?<#{z}>x)")
re.named_captures.keys.map(&:length)  # CRuby: [65536], mruby: [0]
re.match("x")[""]                     # CRuby: IndexError, mruby: "x"

Cause

Two casts in compile_atom(), one at capture registration and one at \k
resolution:

cap_name_len = (uint16_t)(c->p - cap_name);

uint16_t name_len = (uint16_t)(c->p - name);

c->p - cap_name is a ptrdiff_t over the pattern source, while
re_named_capture::name_len is a uint16_t. The truncated value is what gets
stored and what sizes the owned name arena, so nothing reads out of bounds: the
arena is built from the same truncated lengths that every reader uses. What is
lost is the rest of the name, and with it the identity of the group.

Every reader works from the stored length and is therefore consistent with the
truncation rather than with the pattern: the \k lookup, the
@named_captures table Regexp#named_captures derives from,
MatchData#named_captures and MatchData#[].

Fix

Widen re_named_capture::name_len to uint32_t, so a stored length is the
name's true length, and let every local, loop counter and comparison that
consumes it follow. All four wrong answers above then match CRuby.

On a 64-bit target the widening is free: the struct is
const char *name; uint16_t name_len; uint16_t group;, which already pads to
16 bytes, and uint32_t name_len still fits in 16 (measured with cc, both
sizes 16). On a 32-bit target it grows the entry from 8 bytes to 12. That is
the whole cost, paid per named capture.

The bound the cast needs does not go away with the wider field. c->p - cap_name is a ptrdiff_t over the pattern source and nothing in
mrb_re_compile() limits the pattern's length, so a check has to sit in front
of the cast whatever the field's width is. It moves to UINT32_MAX, out of
reach of any pattern this gem can compile, and it is what keeps the cast
lossless.

The bound is spelled once, beside the field it describes, so a later change of
width is one line:

#define RE_MAX_NAME_LEN UINT32_MAX
#define RE_NAME_LEN_FITS(n) ((uintmax_t)(n) <= RE_MAX_NAME_LEN)

The widening inside the macro is not cosmetic. On a target whose argument type
is no wider than the field, a ptrdiff_t on ILP32 or an mrb_int on
MRB_INT32, the naive comparison is against a constant the type cannot exceed,
and clang reports it:

warning: comparison of integers of different signs: 'int32_t' (aka 'int')
         and 'unsigned int' [-Wsign-compare]

uintmax_t rather than uint64_t: C99 makes the exact-width 64-bit types
optional (7.18.1.1p3), and every uint64_t in mruby today sits behind
MRB_GC_PROFILE or MRB_INT64, so a gem header should not require one
unconditionally. CONTRIBUTING.md asks for code as close to C99 as possible.

The guard #7002 put in matchdata_aref() stays, reading through the same
macro. It is what makes the cast on the line below it lossless, and dropping it
would put a truncating comparison back next to an untruncated memcmp(), one
edit away from the same overread. The test #7002 added alongside it needs
nothing either way: its pattern is /(?<abc>x)/, so the over-long name it
looks up resolves to no group whatever the field's width is.

Alternative considered

Rejecting a name that does not fit is two lines and no representation change.
It turns four wrong answers into one RegexpError and leaves the
representation alone, but it is still a divergence from CRuby, which has no
such limit and resolves the full name. Widening makes all four cases agree with
CRuby instead, for one byte per entry on 32-bit targets and nothing on 64-bit
ones.

Tests

One assert() in mrbgems/mruby-regexp/test/regexp.rb, immediately after
assert("MatchData#[] - group name longer than a uint16 length"), which is
#7002's test and the read-side counterpart of this one. It carries one
assertion per wrong answer above: the name survives lookup and
Regexp#named_captures, two names that shared a truncation stay distinct and
both APIs agree on which group each names, \k binds to the group its name was
written on, and a 65536-byte name is not the empty name.

Verified on x86_64-linux:

• Every reproduction above now matches CRuby 4.0.6.
• rake test: 1957 total, 1939 OK, 0 KO, 0 crash, and bintest 105 OK.
• An MRB_INT32 build with clang and -Wall -Wextra: 1862 total, 1844 OK,
  0 KO, 0 crash, and no new warning from any mruby-regexp file.
• Struct size measured on x86_64 only, unchanged at 16 bytes. The 32-bit figure
  of 12 bytes is read from the layout, not measured; no -m32 toolchain was
  available.

Summary by CodeRabbit

• Bug Fixes
  • Fixed regular expressions with named capture groups longer than 65,535 bytes.
  • Ensured long capture names remain distinct and work correctly with named-capture lookups, match indexing, and backreferences.
• Tests
  • Added regression coverage for capture names exceeding the previous length limit, including names exactly 65,536 bytes long.