Skip to content

Commit 8993af9

Browse files
committed
make pregexec() handle zero-length strings again
GH #23903 In embed.fnc, commit v5.43.3-167-g45ea12db26 added SPTR, EPTR parameter modifiers to (amongst other API functions), Perl_pregexec(). These cause assert constraints to be added to the effect that SPTR < EPTR (since the latter is supposed to be a pointer to the byte after the last character in the string). This falls down for an empty string since in this case pregexec() is called with strbeg == strend. This was causing an assert failure in the test suite for Package-Stash-XS. The reason it wasn't noticed before is because: 1) pregexec() is a thin wrapper over regexec_flags(); 2) The perl core (e.g. pp_match()) calls regexec_flags() rather than pregexec(); 3) Package::Stash::XS has XS code which calls pregexec() directly rather than using CALLREGEXEC() (which would call regexec_flags()); 4) In embed.fnc, regexec_flags()'s strend parameter is declared as NN rather than EPTR, so it doesn't get the assert added. So very little code was actually using pregexec(). This commit, for now, changes pregexec()'s strend parameter from EPTR to EPTRQ, which has the net effect of allowing zero-length strings to be passed, and thus fixes the CPAN issue. But longer term, we need to decide: is the general logic for EPTR wrong? Should the assert be SPTR <= EPTR? And should EPTR be applied to regexec_flags()'s strend parameter too?
1 parent c8c2426 commit 8993af9

File tree

5 files changed

+31
-5
lines changed

5 files changed

+31
-5
lines changed

embed.fnc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2713,7 +2713,7 @@ Adhp |REGEXP *|pregcomp |NN SV * const pattern \
27132713
|const U32 flags
27142714
Adhp |I32 |pregexec |NN REGEXP * const prog \
27152715
|MPTR char *stringarg \
2716-
|EPTR char *strend \
2716+
|EPTRQ char *strend \
27172717
|SPTR char *strbeg \
27182718
|SSize_t minend \
27192719
|NN SV *screamer \

ext/XS-APItest/APItest.pm

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use strict;
44
use warnings;
55
use Carp;
66

7-
our $VERSION = '1.47';
7+
our $VERSION = '1.48';
88

99
require XSLoader;
1010

ext/XS-APItest/APItest.xs

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4780,6 +4780,29 @@ CODE:
47804780
OUTPUT:
47814781
RETVAL
47824782

4783+
# provide access to pregexec, except replace pointers within the
4784+
# string with offsets from the start of the string
4785+
4786+
I32
4787+
callpregexec(SV *prog, STRLEN stringarg, STRLEN strend, I32 minend, SV *sv, U32 nosave)
4788+
CODE:
4789+
{
4790+
STRLEN len;
4791+
char *strbeg;
4792+
if (SvROK(prog))
4793+
prog = SvRV(prog);
4794+
strbeg = SvPV_force(sv, len);
4795+
RETVAL = pregexec((REGEXP *)prog,
4796+
strbeg + stringarg,
4797+
strbeg + strend,
4798+
strbeg,
4799+
minend,
4800+
sv,
4801+
nosave);
4802+
}
4803+
OUTPUT:
4804+
RETVAL
4805+
47834806
void
47844807
lexical_import(SV *name, CV *cv)
47854808
CODE:

ext/XS-APItest/t/callregexec.t

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
#!perl
22

3-
# test CALLREGEXEC()
3+
# test CALLREGEXEC() and pregexec()
44
# (currently it just checks that it handles non-\0 terminated strings;
55
# full tests haven't been added yet)
66

@@ -10,7 +10,7 @@ use strict;
1010
use XS::APItest;
1111
*callregexec = *XS::APItest::callregexec;
1212

13-
use Test::More tests => 48;
13+
use Test::More tests => 75;
1414

1515
# Test that the regex engine can handle strings without terminating \0
1616
# XXX This is by no means comprehensive; it doesn't test all ops, nor all
@@ -34,6 +34,8 @@ sub try {
3434
my $bytes = do { use bytes; length $str1 };
3535
ok !!$exp == !!callregexec($re, 0, $bytes, 0, $str, 0),
3636
"$desc callregexec";
37+
ok !!$exp == !!callpregexec($re, 0, $bytes, 0, $str, 0),
38+
"$desc callpregexec";
3739
}
3840

3941

@@ -62,4 +64,5 @@ sub try {
6264
try "ab\t", qr/^.+\h/, 0, 'HORIZWS';
6365
try "abx", qr/^.+\H/, 1, 'NHORIZWS';
6466
try "abx", qr/a.*x/, 0, 'CURLY';
67+
try "", qr/x?/, 1, 'empty';
6568
}

proto.h

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)