develooper Front page | perl.perl5.porters | Postings from January 2017

Re: [perl #130561] Coredump in Perl_re_op_compile

Thread Previous | Thread Next
January 28, 2017 14:51
Re: [perl #130561] Coredump in Perl_re_op_compile
Message ID:
On 27 January 2017 at 20:08, Hugo van der Sanden via RT
<> wrote:
> On Fri, 27 Jan 2017 10:32:31 -0800, demerphq wrote:
>> On 27 January 2017 at 19:04, Hugo van der Sanden via RT
>> <> wrote:
>> > On Fri, 27 Jan 2017 08:02:14 -0800, demerphq wrote:
>> >> On 27 January 2017 at 16:15, Hugo van der Sanden wrote:
>> >> > I'm not entirely sure that I have a use case, but what I had in
>> >> > mind
>> >> > was to modify the fixup loop as in your "this GOSUB may have been
>> >> > optimized away" change, but combine it with a regexec-time check &
>> >> > panic should we see a GOSUB that hasn't had its target set (which
>> >> > requires inventing a way to flag that). Failing that, we should at
>> >> > least have an assert at the point of fixup, for a clea[nr]er
>> >> > failure
>> >> > mode.
>> >>
>> >> If we do this in debug mode only then I see no harm.
>> >
>> > I had planned to make it a mandatory panic (not only under
>> > DEBUGGING), instead of the SEGV or worse you would otherwise get.
>> >
>> >> Can I leave that part to you?
>> >
>> > For now yes - I had already had a go at it (assuming I could set
>> > arg2=0 and test that for "not set") but got a bunch of test failures,
>> > which I haven't tried to dig into yet. If I can't use arg2, I might
>> > want to use flags - but as far as I can see we don't ever actually
>> > use those as flags, so some caution will be needed.
>> Wait wait. Are we talking about the same thing? I thought you were
>> talking about guarding the code that produced the SEGV with logic like
>> i posted in my GOSUB patch which I did not apply.
> I'm talking about: carefully hiding mandatory fixups in an apparently optimization-only function such as study_chunk makes me nervous. Currently downstream code just assumes it has happened.

I don't really get your concern here.

Lots of stuff  like this happens in study_chunk().

And if this code doesnt happen in study chunk then i would have to
write a "walk the optree" sub that is basically a slimmed down version
of study chunk.

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.

So once compilation is finished we need something to walk the data
structure, find the regops we care about and update them so they know
the offset they need to jump to etc.

So if this doesnt happen in study chunk we would have to write a copy
of study chunk that knows how to walk the regop tree, and we would
have to walk it twice for any pattern with recursion.

> 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.

I am sympathetic to the idea that an assert would have prevented some
of this, so I do agree that in debugging mode we should do checks, but
I strongly object to the idea it should happen in non debugging

> The second thing I want to do is allow for the possibility we do optimizing things in the apparently optimization-only function, and take advantage of the runtime check to make it ok to skip unset RExC_recurse nodes (or ones that no longer point to GOSUBs).

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.

Anyway, I will push a patch with some improvements, and hopefully we
can then have something to discuss further.


Thread Previous | Thread Next Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at | Group listing | About