develooper Front page | perl.perl5.porters | Postings from December 2016

Re: [perl #130318] segfault in Perl_mg_magical (mg.c:144)

Thread Previous | Thread Next
From:
demerphq
Date:
December 26, 2016 21:18
Subject:
Re: [perl #130318] segfault in Perl_mg_magical (mg.c:144)
Message ID:
CANgJU+XZfV9C=sHTQGu_gYDJgB2WQdG0M3mmo3WVu1DnREp_PA@mail.gmail.com
On 26 December 2016 at 17:47, Sawyer X <xsawyerx@gmail.com> wrote:
>
>
> On 12/25/2016 11:19 AM, Dave Mitchell wrote:
>> On Thu, Dec 15, 2016 at 01:15:59PM +0100, Sawyer X wrote:
>>> On 12/13/2016 10:02 PM, Leon Timmermans wrote:
>>>> My number one regret of the hackathon must be that we didn't sit down
>>>> and brainstorm how to actually fix the stack-not-refcounted issue.
>>>> I've been increasingly feeling like this is an unavoidable, but
>>>> painful, path forwards.
>>> I wonder if there is value is having a meeting over IRC (or some other
>>> live communication) to discuss this.
>>
>> I think FC started some work on this (origin/sprout/rstack), but I don't
>> know how it was intended to work.
>>
>> I don't think there's much point in a discussion until someone has some
>> initial ideas about how this could be achieved (I don't).
>
> Makes sense to me. Without a plan, little discussion can be had, unless
> someone can add insight on why these happen and what kind of approach
> can be taken.

My understanding is that we have a good idea why these happen, but
less of an idea what to do about it and stay performant.

Here is my very superficial understanding of the matter:

When we place something on the stack we do not increment its refcount
and when we remove something from the stack we do not decrement its
refcount. This means that under some situations we can end up freeing
a value that is still on the stack.

An obvious solution to this would be to refcount the stack. However
even if we assume this is possible (I am doubtful), it would probably
be slow. I seem to recall that MS did some studies with VB and
refcount manipulation of stack arguments can account for 25% of run
time, which is why dotNet is garbage collected.

You can see this in a reduced/modified version of the bug report in this thread:

$ cat t_refcount.pl
use Devel::Peek;
map(
    {
        Dump($_);
        #((%^H) = ('D', 'E',)),
    }
    (%_) = (
        'D', 'E',
        'F', 'G'
    ),
    (%_) = (),

);

# this hash seed is for One-At-A-time, my tests are from Perl v5.18.4
$ PERL_HASH_SEED=0xc717eb9fe97a9788 perl t_refcount.pl
SV = PV(0x1305d80) at 0x1304fc8
  REFCNT = 2
  FLAGS = (POK,pPOK)
  PV = 0x1314d20 "D"\0
  CUR = 1
  LEN = 16
SV = UNKNOWN(0xff) (0x131b2f8) at 0x1305178
  REFCNT = 1
  FLAGS = ()
Attempt to free unreferenced scalar: SV 0x1305178 at t_refcount.pl line 4.
SV = PV(0x1305e80) at 0x131b2e0
  REFCNT = 2
  FLAGS = (POK,pPOK)
  PV = 0x132b0a0 "F"\0
  CUR = 1
  LEN = 16
SV = UNKNOWN(0xff) (0x131b328) at 0x131b2f8
  REFCNT = 1
  FLAGS = ()
Attempt to free unreferenced scalar: SV 0x131b2f8 at t_refcount.pl line 4.

If you uncomment the modification of $^H you can see a different
issue. And if you then comment out the call to Dump() you get the
segfault.

At least part of this bug is due to the following rather insane piece of code:

map { whatever() }
(
    (%_) = (
        'D', 'E',
        'F', 'G'
    ),
    (%_) = ()
);

that is, we populate %_ with 'D', 'E','F','G', which then puts the
keys and values on the stack, with the keys being copies, and the
values being actually on the stack. We then empty the hash, which
frees the values, we then enter the map with the values pointing to
freed scalars.

It is hard to say what we can do about insane stuff like this without
making all sane code pay a penalty. Perhaps it is something we just
want to address via education, and leave it as "won't fix". My
understanding is it is rare to unheard of for real code to tickle
stack-not-refcounted bugs, it only happens when people do crazy stuff
that nobody would do in real code anyway.

As for this particular bug I think it warrants further attention if
only because I think there is more weirdness afoot here than can be
accounted for simply via stack-not-refcounted, in particular the
assignment to the hints hash %^H is required for the segfault, and I
do not see why. I suspect there is some other deeper issue with our
stack manipulations. Compare

use Devel::Peek;
map { Dump($_) }
(
    (%_) = (
        'D', 'E',
        'F', 'G'
    ),
    (%_) = ()
);

SV = PV(0x187dd80) at 0x187cfc8
  REFCNT = 2
  FLAGS = (POK,pPOK)
  PV = 0x188cd20 "D"\0
  CUR = 1
  LEN = 16
SV = UNKNOWN(0xff) (0x18932f8) at 0x187d178
  REFCNT = 1
  FLAGS = ()
Attempt to free unreferenced scalar: SV 0x187d178 at t_refcount.pl line 4.
SV = PV(0x187de80) at 0x18932e0
  REFCNT = 2
  FLAGS = (POK,pPOK)
  PV = 0x18a3130 "F"\0
  CUR = 1
  LEN = 16
SV = UNKNOWN(0xff) (0x1893328) at 0x18932f8
  REFCNT = 1
  FLAGS = ()
Attempt to free unreferenced scalar: SV 0x18932f8 at t_refcount.pl line 4.

to

use Devel::Peek;
map { Dump($_) }
(
    (%_) = (
        'D', 'E',
        'F', 'G'
    ),
    (%_) = ('P','Q')
);

SV = PV(0xb13d80) at 0xb12fc8
  REFCNT = 2
  FLAGS = (POK,pPOK)
  PV = 0xb22d20 "D"\0
  CUR = 1
  LEN = 16
SV = PV(0xb13e10) at 0xb13178
  REFCNT = 2
  FLAGS = (POK,pPOK)
  PV = 0xb35200 "P"\0
  CUR = 1
  LEN = 16
SV = PV(0xb13e80) at 0xb292e0
  REFCNT = 2
  FLAGS = (POK,pPOK)
  PV = 0xb50620 "F"\0
  CUR = 1
  LEN = 16
SV = PV(0xb13e90) at 0xb292f8
  REFCNT = 2
  FLAGS = (POK,pPOK)
  PV = 0xb40500 "Q"\0
  CUR = 1
  LEN = 16
SV = PV(0xb13e10) at 0xb13178
  REFCNT = 2
  FLAGS = (POK,pPOK)
  PV = 0xb35200 "P"\0
  CUR = 1
  LEN = 16
SV = PV(0xb13e90) at 0xb292f8
  REFCNT = 2
  FLAGS = (POK,pPOK)
  PV = 0xb40500 "Q"\0
  CUR = 1
  LEN = 16

I can understand the

SV = UNKNOWN(0xff) (0x1893328) at 0x18932f8

in the first dump, but why do we see 'D', 'P', 'F', 'Q', 'P', 'Q'. I
would expect 'D', UNKNOWN, 'F', UNKNOWN, 'P', 'Q'.

Also why is it when we change it to

use Devel::Peek;
map {
  Dump($_);
 (%^H=('M','N'));
}
(
    (%_) = (
        'D', 'E',
        'F', 'G'
    ),
    (%_) = ()
);

Why do we get:

SV = PV(0x233dd80) at 0x233cfc8
  REFCNT = 2
  FLAGS = (POK,pPOK)
  PV = 0x234cd20 "D"\0
  CUR = 1
  LEN = 16
SV = PVMG(0x23951e0) at 0x233d178
  REFCNT = 2
  FLAGS = (SMG,RMG,POK,pPOK)
  IV = 0
  NV = 0
  PV = 0x235f200 "N"\0
  CUR = 1
  LEN = 16
  MAGIC = 0x2363900
    MG_VIRTUAL = &PL_vtbl_hintselem
    MG_TYPE = PERL_MAGIC_hintselem(h)
    MG_LEN = -2
    MG_PTR = 0x23532f8 => HEf_SVKEY
    SV = PV(0x233de90) at 0x23532f8
      REFCNT = 1
      FLAGS = (POK,pPOK)
      PV = 0x236a500 "M"\0
      CUR = 1
      LEN = 16
SV = PV(0x233de80) at 0x23532e0
  REFCNT = 2
  FLAGS = (POK,pPOK)
  PV = 0x2357ed0 "F"\0
  CUR = 1
  LEN = 16
SV = UNKNOWN(0xff) (0x23533b8) at 0x23532f8
  REFCNT = 1
  FLAGS = ()
Attempt to free unreferenced scalar: SV 0x23532f8 at t_refcount.pl line 4.

And why is it that when I remove the Dump we end up with a segfault
which we dont see with the Dump call?

use Devel::Peek;
map {
 (%^H=('M','N'));
}
(
    (%_) = (
        'D', 'E',
        'F', 'G'
    ),
    (%_) = ()
);

PERL_HASH_SEED=0xc717eb9fe97a9788 perl t_refcount.pl
Segmentation fault

Some of this is easily explained by stack-not-refcounted, but some of
it looks really strange.

How does the return of the the map end up polluting the stack that we
are processing? Why does it only segfault when we remove the call to
Dump()?

Yves

-- 
perl -Mre=debug -e "/just|another|perl|hacker/"

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