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

Re: [perl #130666] toke.c:9188: S_parse_ident: Assertion `(((constU8*) (PL_parser->bufend)) > ((const U8*) t) || (((const U8*)(PL_parser->bufend)) == ((const U8*) t) && *((const U8*) t) == '\0'))' failed

Thread Previous | Thread Next
From:
Karl Williamson
Date:
February 2, 2017 03:55
Subject:
Re: [perl #130666] toke.c:9188: S_parse_ident: Assertion `(((constU8*) (PL_parser->bufend)) > ((const U8*) t) || (((const U8*)(PL_parser->bufend)) == ((const U8*) t) && *((const U8*) t) == '\0'))' failed
Message ID:
7775f2dd-5d9b-f3d7-6388-5d61689fb1e3@khwilliamson.com
On 02/01/2017 09:25 AM, Dave Mitchell wrote:
> On Sun, Jan 29, 2017 at 03:14:25PM -0800, Karl Williamson via RT wrote:
>> Thanks for finding this; fixed by
>> deb0ebcf5f07709222a2073bee27df845a1f14a1
>
> I can't see that commit, but I do see this one:
>
>     commit d2067945159644d284f8064efbd41024f9e8448a
>     Author:     Karl Williamson <khw@cpan.org>
>     AuthorDate: Sun Jan 29 15:56:20 2017 -0700
>     Commit:     Karl Williamson <khw@cpan.org>
>     CommitDate: Sun Jan 29 21:05:54 2017 -0700
>
>     PATCH: [perl #130666]: Revert "toke.c, S_scan_ident(): Don't take a "end of buffer" argument, use PL_bufend"
>
>     This reverts commit b5248d1e210c2a723adae8e9b7f5d17076647431.
>
> This appears to be cause of the following compiler warning:
>
>     toke.c:9273:41: warning: unused parameter 'send' [-Wunused-parameter]
>     S_scan_ident(pTHX_ char *s, const char *send, char *dest, STRLEN destlen, I32 ck_uni)
>
> because that commit seems have added back a 'send' parameter to
> S_scan_ident(), but that function still uses PL_bufend for its actual
> end-of-string delimiting.
>
> So I'm not sure what the intent was, or how it should be fixed.
>

Now fixed by this:

  commit c9470cf1abc4cc21f63ba5222f7860ec1175adfc
  Author: Karl Williamson <khw@cpan.org>
  Date:   Wed Feb 1 13:15:00 2017 -0700

toke.c: Remove unused param from static function

Commit d2067945159644d284f8064efbd41024f9e8448a reverted commit
b5248d1e210c2a723adae8e9b7f5d17076647431.  b5248 removed a parameter
from S_scan_ident, and changed its interior to use PL_bufend instead of
that parameter.  The parameter had been used to limit how far into the
string being parsed scan_ident could look.  In all calls to scan_ident
but one, the parameter was already PL_bufend.  In the one call where it
wasn't, b5248 compensated by temporarily changing PL_bufend around the
call, running afoul, eventually, of the expectation that PL_bufend
points to a NUL.

I would have expected the reversion to add back both the parameter and
the uses of it, but apparently the function interior has changed enough
since the original commit, that it didn't even think there were
conflicts.  As a result the parameter got added back, but not the uses
of it.

I tried both approaches to fix this:
     1) to change the function to use the parameter;
     2) to simply delete the parameter.
Only the latter passed the test suite without error.

I then tried to understand why the parameter in the first place, and why
the kludge introduced by b5248 to work around removing it.  It appears
to me that this is for the benefit of the intuit_more function to enable
it to discern $] from a $ ending a bracketed character class, by ending
the scan before the ']' when in a pattern.

The trouble is that modern scan_ident versions do not view themselves as
constrained by PL_bufend.  If that is reached at a point where white
space is allowed, it will try appending the next input line and
continuing, thus changing PL_bufend.  Thus the kludge in b5248 wouldn't
necessarily do the expected limiting anyway.  The reason the approach
"1)" I tried didn't work was that the function continued to use the
original value, even after it had read in new things, instead of
accounting for those.

Hence approach "2)" is used.  I'm a little nervous about this, as it may
lead to intuit_more() (which uses heuristics) having more cases where it
makes the wrong choice about $] vs [...$].  But I don't see a way around
this, and the pre-existing code could fail anyway.

Spotted by Dave Mitchell.

Thread Previous | Thread Next


nntp.perl.org: Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at ask@perl.org | Group listing | About