version->new("version") SEGVs #11723
Comments
From @nwc10Created by @nwc10Perl_new_version() is being caught out by the fact that sv_derived_from() $ ./perl -Ilib -e 'version->new("version")'Assertion failed: (SvTYPE(hv) == SVt_PVHV), function Perl_hv_common, file hv.c, line 352. Perl_xs_version_bootcheck() has a similar problem: $ ./perl -Ilib -e '$::{"1.11::"} = $::{"version::"}; require Fcntl' This patch seems to fix the problems above. I'm not sure if it's either Inline Patchdiff --git a/util.c b/util.c
index 221dee5..882086b 100644
--- a/util.c
+++ b/util.c
@@ -4857,7 +4857,7 @@ Perl_new_version(pTHX_ SV *ver)
dVAR;
SV * const rv = newSV(0);
PERL_ARGS_ASSERT_NEW_VERSION;
- if ( sv_derived_from(ver,"version") ) /* can just copy directly */
+ if ( SvROK(ver) && sv_derived_from(ver,"version") ) /* can just copy directly */
{
I32 key;
AV * const av = newAV();
@@ -6429,7 +6429,7 @@ Perl_xs_version_bootcheck(pTHX_ U32 items, U32 ax, const char *xs_p,
}
if (sv) {
SV *xssv = Perl_newSVpvn_flags(aTHX_ xs_p, xs_len, SVs_TEMP);
- SV *pmsv = sv_derived_from(sv, "version")
+ SV *pmsv = SvROK(sv) && sv_derived_from(sv, "version")
? sv : sv_2mortal(new_version(sv));
xssv = upg_version(xssv, 0);
if ( vcmp(pmsv,xssv) ) {
Nicholas ClarkPerl Info |
From @HugmeirOn Mon, Oct 31, 2011 at 12:08 PM, Nicholas Clark
There's a couple other uses of sv_derived_from that check for version |
|
The RT System itself - Status changed from 'new' to 'open' |
From @cpansproutOn Mon Oct 31 08:08:31 2011, nicholas wrote:
You probably suspected I would come up with this: sub TIESCALAR { bless[] } See also Brian Fraser’s response.
PATH=/opt/local/bin:/opt/local/sbin:/Users/nick/bin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/bin:/usr/X11/bin:/usr/local/sbin:/sbin:/usr/sbin
-- Father Chrysostomos |
From @nwc10On Mon, Oct 31, 2011 at 12:17:08PM -0300, Brian Fraser wrote:
I don't know. I felt I'd already been distracted too much by this bug by At least 2 other people are far more familiar with the internals of this
No, I didn't suspect anything. I decided that I couldn't actually fix the Nicholas Clark |
From @cpansproutOn Thu Nov 03 07:48:29 2011, nicholas wrote:
That sounds familiar, except I usually put a few notes in a text file -- Father Chrysostomos |
From @nwc10On Thu, Nov 03, 2011 at 08:17:41AM -0700, Father Chrysostomos via RT wrote:
You'd wipe the floor on the "open bug" stats if you do report all of those. Nicholas Clark |
From @xdgOn Mon, Oct 31, 2011 at 11:17 AM, Brian Fraser <fraserbn@gmail.com> wrote:
Yes. Or maybe we need a "sv_obj_isa" function/macro to encapsulate the concept. Nicholas -- I believe the patch is correct enough, though a segfault -- David |
From @JohnPeacockOn 11/03/2011 12:08 PM, David Golden wrote:
Sorry I took so long to respond to this. Yes, I think sv_obj_isa would #define ISA_VERSION_OBJ(v,c) (SvROK(v) && sv_derived_from(v,c)) though there is nothing about that define that is specific to version
There is already a vverify() routine, which should protect the vast John |
From @cpansproutOn Sat Nov 12 05:16:34 2011, john.peacock@havurah-software.org wrote:
That will still crash on a tied variable returning the string "version", sv_derived_from in core is now a wrapper around Then sv_obj_isa can be made a wrapper around sv_derived_from_pvn that
-- Father Chrysostomos |
From @cpansproutOn Sat Nov 19 11:30:11 2011, sprout wrote:
Actually, sv_derived_from does *three* things. isa([], 'ARRAY'), Just to get this bug patch up, I’ll implement a flags & SVf_ROK check in -- Father Chrysostomos |
From @cpansproutOn Sat Nov 19 12:02:51 2011, sprout wrote:
lib/version.t is dual-lived, so where should I put tests? -- Father Chrysostomos |
From @cpansproutOn Sat Nov 19 12:09:10 2011, sprout wrote:
I just went ahead and added tests to lib/version.t. I used the simplest -- Father Chrysostomos |
|
@cpansprout - Status changed from 'open' to 'resolved' |
From @JohnPeacockOn 11/23/2011 12:36 AM, Father Chrysostomos via RT wrote:
Is there some reason you add the SvROK test _after_ sv_derived_from that John |
From @JohnPeacockOn 11/23/2011 09:36 PM, John Peacock wrote:
Ah, never mind. I've read the log message now. ;-) John |
From @JohnPeacockOn 11/23/2011 12:36 AM, Father Chrysostomos via RT wrote:
I'd like to suggest that this is a better solution: #define ISA_CLASS_OBJ(v,c) (SvGMAGICAL(v)\ This will be faster for the majority of the cases and still handle the I've also pulled the test case you've added into the version.pm Thoughts? John |
From @cpansproutOn Sun Nov 27 16:43:49 2011, john.peacock@havurah-software.org wrote:
Sounds good to me. -- Father Chrysostomos |
From @cpansproutOn Thu Nov 03 08:20:43 2011, nicholas wrote:
Well, I fixed most of them, but more accumulated. So I have just clean -- Father Chrysostomos |
From @cpansproutOn Sun Dec 11 14:03:25 2011, sprout wrote:
s/clean/cleaned/, otherwise it’s completely illiterate. Someone once -- Father Chrysostomos |
Migrated from rt.perl.org#102586 (status was 'resolved')
Searchable as RT102586$
The text was updated successfully, but these errors were encountered: