Bug #116685 for B-Hooks-Parser: skipspace may be buggy

Wed Aug 03 02:05:21 2016 $_ = 'spro^^*%*^6ut# [...] &$%*c>#!^!#&!pan.org'; y/a-z.@//cd; print - Ticket created

Subject:

skipspace may be buggy

The patch from https://rt.cpan.org/Ticket/Display.html?id=102918 may need to be applied to stolen_chunk_of_toke.c. B::Hooks::Toke::skipspace/hook_toke_skipspace may well be buggy without it on recent perls, due to its use of PL_sublex_info.sub_inwhat. I don’t really know how to go about testing it. Now, the *real* reason I bring this up is that I am considering eliminating the sublex_info struct, to avoid alignment holes. B::Hooks::Parser is the only CPAN user of PL_sublex_info. BTW, signatures seems to be the only CPAN module using skipspace (or scan_str, which calls skipspace), but it works in such a way as to be unaffected by this potential bug.

Wed Aug 03 09:28:32 2016 $_ = 'spro^^*%*^6ut# [...] &$%*c>#!^!#&!pan.org'; y/a-z.@//cd; print - Correspondence added

RT-Send-CC:

ANDK [...] cpan.org

On Wed Aug 03 02:05:21 2016, SPROUT wrote: Show quoted text

> The patch from https://rt.cpan.org/Ticket/Display.html?id=102918 may > need to be applied to stolen_chunk_of_toke.c. > B::Hooks::Toke::skipspace/hook_toke_skipspace may well be buggy > without it on recent perls, due to its use of > PL_sublex_info.sub_inwhat. I don’t really know how to go about > testing it. > > Now, the *real* reason I bring this up is that I am considering > eliminating the sublex_info struct, to avoid alignment holes. > B::Hooks::Parser is the only CPAN user of PL_sublex_info.

Well, I have just done that, in commit v5.25.3-98-g7ef70b3. This probably breaks B::Hooks::Parser. (I’m CCing Andreas to save him having to bisect.)

Wed Aug 03 09:28:33 2016 The RT System itself - Status changed from 'new' to 'open'

Mon Aug 08 22:04:22 2016 ether [...] cpan.org - Correspondence added

On 2016-08-02 23:05:21, SPROUT wrote: Show quoted text

> The patch from https://rt.cpan.org/Ticket/Display.html?id=102918 may > need to be applied to stolen_chunk_of_toke.c.

Should I just copy (bits of) toke.c from blead, or are modifications necessary for compatibility with other perl versions? This doesn't solve the whole problem though, does it?

Mon Aug 08 23:36:35 2016 $_ = 'spro^^*%*^6ut# [...] &$%*c>#!^!#&!pan.org'; y/a-z.@//cd; print - Correspondence added

On Mon Aug 08 22:04:22 2016, ETHER wrote: Show quoted text

> On 2016-08-02 23:05:21, SPROUT wrote:

> > The patch from https://rt.cpan.org/Ticket/Display.html?id=102918 may > > need to be applied to stolen_chunk_of_toke.c.

> > Should I just copy (bits of) toke.c from blead, or are modifications > necessary for compatibility with other perl versions?

The patch that was applied to Devel::Declare’s stolen_chunk should be fine. As for compatibility, it should not be an issue. When the part of toke.c in question started checking PL_sublex_info.sub_inwhat, it was a mistake to begin with. It just happened that PL_sublex_info.sub_inwhat usually had the same value as PL_lex_inwhat, but not always. Perl itself was corrected not to use sub_inwhat that way, and so it stopped localizing sub_inwhat, as that was no longer necessary. I don’t have time to think this through thoroughly now, but I *think* the potential bug is that after a nested quote-like operator scan_str will misbehave when it comes to a line break. With the patch that was applied to Devel::Declare, that will not happen. Also, copying bits of the current toke.c is infeasible, as much of the code has been reworked, to the point that you will not even find the stolen_chunk’s condition in question in any recognizable form in bleadperl. Show quoted text

> This doesn't > solve the whole problem though, does it?

I think it does. Sorry, I wrote this really fast. I don’t know whether it makes any sense.

Mon Aug 29 20:26:12 2016 ether [...] cpan.org - Correspondence added

On 2016-08-08 20:36:35, SPROUT wrote: Show quoted text

> On Mon Aug 08 22:04:22 2016, ETHER wrote:

> > On 2016-08-02 23:05:21, SPROUT wrote:

> > > The patch from https://rt.cpan.org/Ticket/Display.html?id=102918 > > > may > > > need to be applied to stolen_chunk_of_toke.c.

> > > > Should I just copy (bits of) toke.c from blead, or are modifications > > necessary for compatibility with other perl versions?

> > The patch that was applied to Devel::Declare’s stolen_chunk should be

Ok, I've applied the patch and released it as 0.18-TRIAL - thanks!

Mon Aug 29 20:26:14 2016 ether [...] cpan.org - Status changed from 'open' to 'resolved'

Tue Aug 30 14:20:35 2016 ether [...] cpan.org - Fixed in 0.18-TRIAL added