Front page | perl.perl5.porters |
Postings from January 2017
On 28 January 2017 at 15:51, demerphq <demerphq@gmail.com> wrote:
> On 27 January 2017 at 20:08, Hugo van der Sanden via RT
> <perlbug-followup@perl.org> wrote:
>> On Fri, 27 Jan 2017 10:32:31 -0800, demerphq wrote:
> The problem is that we cannot reliably know the position and distance
> between two regops until compilation is finished. reginsert() may be
> called at any time to move the ops.
Actually we could do the same thing that we do for OPEN/CLOSE regops
in reginsert(). But since the optimiser still needs to follow all
recursion calls to determine if they are cyclic, it would mean a
higher cost for no advantage.
>> So the first thing I want to do is remove that assumption with a runtime check and panic.
>
> I see no reason to burden every production regexp compilation with
> validating that someone has hacked regexp.c to do something that they
> should never do.
On further reflection I retract. It is not unreasonable to optimise
away a GOSUB op.
However I still see no reason to burden production regexp compilation
with these checks, after all they can only be triggered by changes to
the regex engine.
> It is not ever ever ok to optimise away an OPEN without updating the
> code so that the same logic that populates RExC_recursion. (See for
> instance CURLYM).
>
> If someone does, then you will break recursion. And that is not acceptable.
This is correct, but actually irrelevant to the exception we threw,
which came from having optimising away a GOSUB, which is a reasonable
thing to do (although AFAIK right now we don't do it.) But if someone
changes the code to do so then they will trigger the assert during
dev, and will see the comment explaining what is going on.
> Anyway, I will push a patch with some improvements, and hopefully we
> can then have something to discuss further.
I pushed the following patch:
commit bc18b9df70cc60cdfa7dd6e64d25e960792a0bf3
Author: Yves Orton <demerphq@gmail.com>
Date: Sat Jan 28 16:20:35 2017 +0100
assert that the RExC_recurse data structure points at a valid GOSUB
This assert will fail if someone adds code that optimises away a GOSUB
call. At which point they will see the comment and know what to do.
diff --git a/regcomp.c b/regcomp.c
index d5ce63f..19ed866 100644
--- a/regcomp.c
+++ b/regcomp.c
@@ -7789,6 +7789,18 @@ Perl_re_op_compile(pTHX_ SV ** const patternp,
int pat_count,
while ( RExC_recurse_count > 0 ) {
const regnode *scan = RExC_recurse[ --RExC_recurse_count ];
+ /*
+ * This data structure is set up in study_chunk() and is used
+ * to calculate the distance between a GOSUB regopcode and
+ * the OPEN/CURLYM (CURLYM's are special and can act like OPEN's)
+ * it refers to.
+ *
+ * If for some reason someone writes code that optimises
+ * away a GOSUB opcode then the assert should be changed to
+ * an if(scan) to guard the ARG2L_SET() - Yves
+ *
+ */
+ assert(scan && OP(scan) == GOSUB);
ARG2L_SET( scan, RExC_open_parens[ARG(scan)] - scan );
}
Sorry my original overhasty and misleading reply.
cheers,
Yves
--
perl -Mre=debug -e "/just|another|perl|hacker/"
Thread Previous
|
Thread Next