[perl #122445] Segmentation fault while debugging programs using bignum in 5.18.2

Thanks for the feedback.

On Fri Aug 15 04:47:22 2014, bulk88 wrote:
> On Wed Aug 13 18:25:26 2014, tonyc wrote:
> >
> > Attached is a test and a patch that does just that.
> >
> > This is my first time implementing magic.
> >
> > Tony
> >
> 
> diff --git a/intrpvar.h b/intrpvar.h
> index 9dd4e16..a8fa1ef 100644
> --- a/intrpvar.h
> +++ b/intrpvar.h
> @@ -390,6 +390,10 @@ PERLVAR(I, DBtrace,        SV *)           /*
> $DB::trace  */
>  PERLVAR(I, DBsignal,   SV *)           /*  $DB::signal */
>  PERLVAR(I, dbargs,     AV *)           /* args to call listed by
> caller function */
> 
> +PERLVAR(I, DBsingle_iv, IV)
> +PERLVAR(I, DBtrace_iv,  IV)
> +PERLVAR(I, DBsignal_iv, IV)
> +
>  /* symbol tables */
>  PERLVAR(I, debstash,   HV *)           /* symbol table for perldb
> package */
>  PERLVAR(I, globalstash,        HV *)           /* global keyword
> overrides imported here */
> 
> 
> Why are these IVs and not chars or bits in a char?

I considered this, but perl5db.pl stores flags in $DB::single and $DB::trace at least.  Another debugger might use more flags.

> 
> diff --git a/pp_ctl.c b/pp_ctl.c
> index 5e671ee..aca051e 100644
> --- a/pp_ctl.c
> +++ b/pp_ctl.c
> @@ -1943,7 +1943,7 @@ PP(pp_dbstate)
>      PERL_ASYNC_CHECK();
> 
> if (PL_op->op_flags & OPf_SPECIAL /* breakpoint */
> -           || SvIV(PL_DBsingle) || SvIV(PL_DBsignal) ||
> SvIV(PL_DBtrace))
> +           || PL_DBsingle_iv || PL_DBsignal_iv || PL_DBtrace_iv)
> {
>    dSP;
>    PERL_CONTEXT *cx;
> 
> This code is begging for a bitfield or 4 char array that can be
> cast/union to I32.

Since I don't want to assumed limited sizes for these variables, I can't make this optimization.

> +int
> +Perl_magic_setdebugvar(pTHX_ SV *sv, MAGIC *mg) {
> +    PERL_ARGS_ASSERT_MAGIC_SETDEBUGVAR;
> +
> +    switch (mg->mg_private) {
> +    case DBVARMG_SINGLE:
> +        PL_DBsingle_iv = SvIV_nomg(sv);
> +        break;
> +
> +    case DBVARMG_TRACE:
> +        PL_DBtrace_iv = SvIV_nomg(sv);
> +        break;
> +
> +    case DBVARMG_SIGNAL:
> +        PL_DBsignal_iv = SvIV_nomg(sv);
> +        break;
> +
> +    default:
> +        NOT_REACHED;
> +    }
> +
> +    return 1;
> +}
> 
> Factor out the SvIV_nomg calls. Also instead of using the constants
> DBVARMG_* and mg_private, why not put the pointer to the IV (or
> whatever) into mg_ptr? then no more switch and the code is branchless.
> Interps can't be realloced.

I made the PL_*_iv variables into an array, which removed the switch and factored out the SvIV_nomg() calls.

> diff --git a/mg_raw.h b/mg_raw.h
> index f508ad0..984f1d7 100644
> --- a/mg_raw.h
> +++ b/mg_raw.h
> @@ -14,6 +14,8 @@
>        "/* rhash '%' extra data for restricted hashes */" },
>      { '&', "magic_vtable_max",
>        "/* proto '&' my sub prototype CV */" },
> +    { '*', "want_vtbl_debugvar",
> +      "/* debugvar '*' $DB::single, signal, trace vars */" },
>      { '.', "want_vtbl_pos | PERL_MAGIC_VALUE_MAGIC",
>        "/* pos '.' pos() lvalue */" },
>      { ':', "magic_vtable_max | PERL_MAGIC_VALUE_MAGIC",
> 
> Why are we using up a precious letter? per letter magic limitation
> became obsolete when mg_findext was added.
> Why not have it be ~/PERL_MAGIC_ext like regular 3rd party XS magic?
> Or this is for Dump() reasons?

When I wrote it I checked the core for use of PERL_MAGIC_ext and didn't find any, so I assumed it was best practice in core to add new magic.

As you say, it makes the sv_dump() output more intelligible.

Using the indexes as I've done also helps to make it readable.

Tony

---
via perlbug:  queue: perl5 status: open
https://rt.perl.org/Ticket/Display.html?id=122445

• [perl #122445] Segmentation fault while debugging programs using bignum in 5.18.2 by Tony Cook via RT