New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bleadperl v5.25.5-100-g2b5e7bc2e6 breaks JDDPAUSE/re-engine-GNU-0.021.tar.gz #15911
Comments
From @andkrt-cpan https://rt.cpan.org/Ticket/Display.html?id=120498 bisect commit 2b5e7bc utf8n_to_uvchr(): Note multiple malformations perl -V % /home/sand/src/perl/repoperls/installed-perls/host/k93msid/v5.25.5-100-g2b5e7bc2e6/f11c/bin/perl -V Characteristics of this binary (from libperl): -- |
From @iabynOn Sat, Mar 04, 2017 at 09:49:12PM -0800, Andreas J. Koenig via RT wrote:
Hi Karl, Newx(adjusted_s0, OFFUNISKIP(min_uv) + 1, U8); The problem is that re-engine-GNU is calling utf8n_to_uvchr() during AFAIKT, the Newx is just just being used to create a temporary buffer big Perl_utf8n_to_uvchr_error(...) -- |
The RT System itself - Status changed from 'new' to 'open' |
From @khwilliamsonOn 03/06/2017 05:39 AM, Dave Mitchell wrote:
I agree, and I probably should have done it your way to begin with. |
From @khwilliamson0002-PATCH-perl-130921-Don-t-use-Newx-in-decoding-UTF-8.patchFrom bfb3432aec08361b344a6cdf1b80160bf53e981f Mon Sep 17 00:00:00 2001
From: Karl Williamson <khw@cpan.org>
Date: Mon, 6 Mar 2017 12:25:21 -0700
Subject: [PATCH 2/2] PATCH: [perl #130921]: Don't use Newx in decoding UTF-8
The bottom level UTF-8 decoding routine can be used during periods when
using Newx is prohibited, as diagnosed by Dave Mitchell for this ticket.
The particular use of Newx was unnecessary, as it is just large enough
to hold a single character, and that can be done by an automatic
variable on the C stack, and probably more efficiently.
---
utf8.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/utf8.c b/utf8.c
index 89c8413..4949bf6 100644
--- a/utf8.c
+++ b/utf8.c
@@ -1079,6 +1079,8 @@ Perl_utf8n_to_uvchr_error(pTHX_ const U8 *s,
U8 * adjusted_s0 = (U8 *) s0;
U8 * adjusted_send = NULL; /* (Initialized to silence compilers' wrong
warning) */
+ U8 temp_char_buf[UTF8_MAXBYTES + 1]; /* Used to avoid a Newx in this
+ routine; see [perl #130921] */
UV uv_so_far = 0; /* (Initialized to silence compilers' wrong warning) */
PERL_ARGS_ASSERT_UTF8N_TO_UVCHR_ERROR;
@@ -1245,10 +1247,7 @@ Perl_utf8n_to_uvchr_error(pTHX_ const U8 *s,
I8_TO_NATIVE_UTF8(UTF_CONTINUATION_MARK));
}
- Newx(adjusted_s0, OFFUNISKIP(min_uv) + 1, U8);
- SAVEFREEPV((U8 *) adjusted_s0); /* Needed because we may not get
- to free it ourselves if
- warnings are made fatal */
+ adjusted_s0 = temp_char_buf;
adjusted_send = uvoffuni_to_utf8_flags(adjusted_s0, min_uv, 0);
}
}
--
2.7.4
|
From @iabynOn Mon, Mar 06, 2017 at 12:31:09PM -0700, Karl Williamson wrote:
It fails in a different way now - the test script no longer crashes while I strongly suspect its unrelated to to the utf8n_to_uvch() issue/regression. re-engine-GNU appears to be using the non-documented, non-API function SV = PV(0x18b03b8) at 0x18acc20 I don't know yet whether re-engine-GNU or sv_setsv_cow() has unrealistic -- |
From @iabynOn Mon, Mar 06, 2017 at 08:33:49PM +0000, Dave Mitchell wrote:
So I think your fix is good to apply to blead.
It fails as far back as at least 5.24.0 on threaded builds with debugging It can be reduced to use threads; sub f { when the match is run within a child thread, the const SV holding "x" gets FLAGS = (PADSTALE,POK,READONLY,PROTECT,pPOK,UTF8) This is not COWable, so sv_setsv_cow() triggers an assertion. -- |
From @khwilliamsonOn 03/07/2017 04:15 AM, Dave Mitchell wrote:
Now done, as e9f2c44 with the Aaron Crane had an interesting observation. Since this was a regression |
From @khwilliamsonOn Tue, 07 Mar 2017 09:59:16 -0800, public@khwilliamson.com wrote:
I was careless with my commit message for e9f2c44. The problem is not the Newx, but the SAVEFREEPV of the string allocated by the Newx -- |
From @jkeenanOn Tue, 07 Mar 2017 23:08:35 GMT, khw wrote:
This ticket is marked as a blocker for 5.26.0. Can we get an update on its status? Thank you very much. -- |
From @xsawyerxOn 03/18/2017 01:09 AM, James E Keenan via RT wrote:
Karl, will it be possible to merge please? |
From @iabynOn Sat, Mar 18, 2017 at 06:40:35PM +0100, Sawyer X wrote:
The fix for blead was merged by Karl back on 7 March. The secondary issue So I'll close this ticket now. -- |
@iabyn - Status changed from 'open' to 'resolved' |
Migrated from rt.perl.org#130921 (status was 'resolved')
Searchable as RT130921$
The text was updated successfully, but these errors were encountered: