[winfu] 'free to wrong pool' handling improvement
Stas Bekman
when the interpreter context is wrong and a memory from the wrong pool is
allocated. This is all dandy, but it doesn't make it any easy to find the
place where the problem is, as it complains at the perl_destruct stage. What
would it take to make windows implementation complain about the wrong pool
when the variable is asking to allocate the memory, or immediately after it,
so the trace will pinpoint the place with a problem.
I'm not familiar with the implementation but may be something like: allocate
the memory, then free it and then allocate again? will that work?
The bottom line is that the same application behaves differently under winFU
and non-winFU perl versions. So if I'm a non-winFU developer and have a user
reporting a problem with my application on winFU, I should be able to
reproduce this problem on non-winFU platform. Please notice that this has
nothing to do with OS-specifics, like file system, signals or what not, but
just memory allocation/freeing implemented purely by Perl itself. How do we
make perl behave identically, so that if my program works on unix it will work
on winFU just the same (minus the perlport.pod issues).
If you think that there is no way to make winFU perl memory allocation behave
like non-winFU perl, give us an option to make non-winFU perl behave like
winFU perl, so we can debug our perl programs w/o needing to buy heaps of sw,
install and learn winFU, just to be able to resolve problems for the
unfortunate users of winFU. How hard would it be to emulate memory pools per
thread for non-winFU perl? I'm talking about a compile time option of course.
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:st...@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
Elizabeth Mattijsen
>The bottom line is that the same application behaves differently
>under winFU and non-winFU perl versions. So if I'm a non-winFU
>developer and have a user reporting a problem with my application on
>winFU, I should be able to reproduce this problem on non-winFU
>platform. Please notice that this has nothing to do with
>OS-specifics, like file system, signals or what not, but just memory
>allocation/freeing implemented purely by Perl itself. How do we make
>perl behave identically, so that if my program works on unix it will
>work on winFU just the same (minus the perlport.pod issues).
Well, that's just it. Perl tries to provide the same interface on
OS's that are _radically_ different underneath. And that involves
different approaches to memory allocation because the underlying OS
"invites" you to do things differently. And Perl fails in this
respect. What was found is no more and no less than a bug. That
should be fixed. Hopefully for 5.8.4 (if I'm to believe the
ActiveState people).
The same application _should_ behave the same. If there is a problem
on one platform and not on another, then there is a problem in the
interface with the OS on that platform. Please don't ask for Win32
bugs to happen on non Win32 systems as well. ;-)
>If you think that there is no way to make winFU perl memory
>allocation behave like non-winFU perl, give us an option to make
>non-winFU perl behave like winFU perl, so we can debug our perl
>programs w/o needing to buy heaps of sw, install and learn winFU,
>just to be able to resolve problems for the unfortunate users of
>winFU. How hard would it be to emulate memory pools per thread for
>non-winFU perl? I'm talking about a compile time option of course.
Im thinking added complexity and an open invitation to more bugs here.
It's like testing with Class::MockObject: in the end you only know
for sure that it works with Class::MockObject.
Liz
Elizabeth Mattijsen
>>The same application _should_ behave the same. If there is a
>>problem on one platform and not on another, then there is a problem
>>in the interface with the OS on that platform. Please don't ask
>>for Win32 bugs to happen on non Win32 systems as well. ;-)
>contexts/alloc/free can get tricky. It's possible that the
>application doesn't fail immediately on unix but it may fail in the
>middle of some important task, because the freed memory just
>happened not to be overrun by some other allocation. I actually find
>it a goodness that winFU perl implementation screams when it finds
>inconsistencies, I wish it was screaming much earlier and not during
>perl_destruct, when it's not very helpful. What I'm asking for is to
>have an optional feature on non-winFU perl which will scream the
>same way, plus doing it when things get allocated and not freed.
If it's screaming you're interested in, run it under valgrind ;-)
With --leak-check=yes, you should get a fairly comprehensice overview
of what was allocated and "lost".
>>Im thinking added complexity and an open invitation to more bugs here.
>>It's like testing with Class::MockObject: in the end you only know
>>for sure that it works with Class::MockObject.
>Any solution that will let me solve the problem for winFU users is welcome.
True. But I think that only a real winFU user will be able to fix
this in the "real" environment in the long run.
Liz
Stas Bekman
> At 18:57 -0800 1/20/04, Stas Bekman wrote:
>
>> The bottom line is that the same application behaves differently under
>> winFU and non-winFU perl versions. So if I'm a non-winFU developer and
>> have a user reporting a problem with my application on winFU, I should
>> be able to reproduce this problem on non-winFU platform. Please notice
>> that this has nothing to do with OS-specifics, like file system,
>> signals or what not, but just memory allocation/freeing implemented
>> purely by Perl itself. How do we make perl behave identically, so that
>> if my program works on unix it will work on winFU just the same (minus
>> the perlport.pod issues).
>
>
> Well, that's just it. Perl tries to provide the same interface on OS's
> that are _radically_ different underneath. And that involves different
> approaches to memory allocation because the underlying OS "invites" you
> to do things differently. And Perl fails in this respect. What was
> found is no more and no less than a bug. That should be fixed.
> Hopefully for 5.8.4 (if I'm to believe the ActiveState people).
A bug? I don't think so. As I move around the context definitions as I flip
threads, things get fixed. But I'm really guessing and ask Steve to test it
for me, which is not very productive. And even if this gets fixed in 5.8.4,
what are we going to do with users running on previous perl versions?
> The same application _should_ behave the same. If there is a problem on
> one platform and not on another, then there is a problem in the
> interface with the OS on that platform. Please don't ask for Win32 bugs
> to happen on non Win32 systems as well. ;-)
this is not necessarily correct, Liz. The issue with contexts/alloc/free can
get tricky. It's possible that the application doesn't fail immediately on
unix but it may fail in the middle of some important task, because the freed
memory just happened not to be overrun by some other allocation. I actually
find it a goodness that winFU perl implementation screams when it finds
inconsistencies, I wish it was screaming much earlier and not during
perl_destruct, when it's not very helpful. What I'm asking for is to have an
optional feature on non-winFU perl which will scream the same way, plus doing
it when things get allocated and not freed.
>> If you think that there is no way to make winFU perl memory allocation
>> behave like non-winFU perl, give us an option to make non-winFU perl
>> behave like winFU perl, so we can debug our perl programs w/o needing
>> to buy heaps of sw, install and learn winFU, just to be able to
>> resolve problems for the unfortunate users of winFU. How hard would it
>> be to emulate memory pools per thread for non-winFU perl? I'm talking
>> about a compile time option of course.
>
>
> Im thinking added complexity and an open invitation to more bugs here.
>
> It's like testing with Class::MockObject: in the end you only know for
> sure that it works with Class::MockObject.
Any solution that will let me solve the problem for winFU users is welcome.
Stas Bekman
> At 02:08 -0800 1/21/04, Stas Bekman wrote:
>
>>> The same application _should_ behave the same. If there is a problem
>>> on one platform and not on another, then there is a problem in the
>>> interface with the OS on that platform. Please don't ask for Win32
>>> bugs to happen on non Win32 systems as well. ;-)
>>
>> this is not necessarily correct, Liz. The issue with
>> contexts/alloc/free can get tricky. It's possible that the application
>> doesn't fail immediately on unix but it may fail in the middle of some
>> important task, because the freed memory just happened not to be
>> overrun by some other allocation. I actually find it a goodness that
>> winFU perl implementation screams when it finds inconsistencies, I
>> wish it was screaming much earlier and not during perl_destruct, when
>> it's not very helpful. What I'm asking for is to have an optional
>> feature on non-winFU perl which will scream the same way, plus doing
>> it when things get allocated and not freed.
>
>
> If it's screaming you're interested in, run it under valgrind ;-) With
> --leak-check=yes, you should get a fairly comprehensice overview of what
> was allocated and "lost".
It's not quite doable when perl is embedded inside apache. I've tried several
times and it almost killed my machine.
>>> Im thinking added complexity and an open invitation to more bugs here.
>>> It's like testing with Class::MockObject: in the end you only know
>>> for sure that it works with Class::MockObject.
>>
>> Any solution that will let me solve the problem for winFU users is
>> welcome.
>
> True. But I think that only a real winFU user will be able to fix this
> in the "real" environment in the long run.
I think winFU perl can handle this issue better. I hope Jan will be able to
comment on my suggestion, on doing this pool testing during the memory
allocation and not during perl_destruct.
Dave Mitchell
> So we get these 'free to wrong pool' errors on winfu, usually under
What's winfu?
Also, does anyone know exactly what is meant when windows generates these
errors? Is it that threads under windows OSes have a separate malloc pool
for each thread, so that if one thread does a malloc and then another
thread tries to free that address, you get the error?
> ithreads when the interpreter context is wrong and a memory from the wrong
> pool is allocated. This is all dandy, but it doesn't make it any easy to
> find the place where the problem is, as it complains at the perl_destruct
> stage. What would it take to make windows implementation complain about the
> wrong pool when the variable is asking to allocate the memory, or
> immediately after it, so the trace will pinpoint the place with a problem.
>
> I'm not familiar with the implementation but may be something like:
> allocate the memory, then free it and then allocate again? will that work?
I don't see how a scheme like that could possibly work. The only approach
that I can think of is that under certain Perl builds (say ithreads +
DEBUGGING), malloc/free are wrapped with functions that record the
thread id (or something) at the start of each allocated block, and then
throw a wobby if free is called and the tids don't match. This is
roughly what I hacked together the last time I fixed such a bug (not
being a windows hacker). Of course this doesn't provide for earlier
detection, it just makes detection available across all platforms.
The downside is that each malloc will use an extra 4+ bytes.
--
Monto Blanco... scorchio!
Nicholas Clark
> I don't see how a scheme like that could possibly work. The only approach
> that I can think of is that under certain Perl builds (say ithreads +
> DEBUGGING), malloc/free are wrapped with functions that record the
> thread id (or something) at the start of each allocated block, and then
> throw a wobby if free is called and the tids don't match. This is
> roughly what I hacked together the last time I fixed such a bug (not
> being a windows hacker). Of course this doesn't provide for earlier
> detection, it just makes detection available across all platforms.
>
> The downside is that each malloc will use an extra 4+ bytes.
So can't be on by default; should probably be an independent #define option
I think I can see two "extra" things that would be useful - the thread ID
(as you observe) and a pointer to create a linked list of memory blocks
allocated under this thread. This way the implementation can free all the
ithread's memory on ithread exit, which (as I understand it) is equivalent
to what happens on Windows, but with the benefit that on x86/Linux while
running under valgrind one will get access errors as soon as another
ithread attempts to use the now-freed memory.
I'm not sure of the best way to tweak the layers of obfuscation, er,
indirection on perl's malloc() use to achieve this. Had someone kind
at ActiveState volunteer to suggest an interface, or am I misremembering?
I'm happy to actually implement the guts, if spoon fed the interface
interception stuff. (Foolish me for volunteering)
Nicholas Clark
Jan Dubois
<da...@fdisolutions.com> wrote:
>On Tue, Jan 20, 2004 at 06:57:14PM -0800, Stas Bekman wrote:
>> So we get these 'free to wrong pool' errors on winfu, usually under
>
>What's winfu?
I'm assuming Stas means something like "windows knowledge", just like
patch-fu or kung-fu.
>Also, does anyone know exactly what is meant when windows generates these
>errors? Is it that threads under windows OSes have a separate malloc pool
>for each thread, so that if one thread does a malloc and then another
>thread tries to free that address, you get the error?
Yes, that is exactly what happens. Each thread maintains its own heap
that can easily be discarded once the interpreter terminates. This
helps with memory leaks in persistent interpreters: you just discard and
reallocate the Perl interpreter. With a global heap you would have to
create a new process to clean up after any memory leaks.
>> ithreads when the interpreter context is wrong and a memory from the wrong
>> pool is allocated. This is all dandy, but it doesn't make it any easy to
>> find the place where the problem is, as it complains at the perl_destruct
>> stage. What would it take to make windows implementation complain about the
>> wrong pool when the variable is asking to allocate the memory, or
>> immediately after it, so the trace will pinpoint the place with a problem.
>>
>> I'm not familiar with the implementation but may be something like:
>> allocate the memory, then free it and then allocate again? will that work?
>
>I don't see how a scheme like that could possibly work. The only approach
>that I can think of is that under certain Perl builds (say ithreads +
>DEBUGGING), malloc/free are wrapped with functions that record the
>thread id (or something) at the start of each allocated block, and then
>throw a wobby if free is called and the tids don't match. This is
>roughly what I hacked together the last time I fixed such a bug (not
>being a windows hacker). Of course this doesn't provide for earlier
>detection, it just makes detection available across all platforms.
This is pretty much what happens on Windows. I'll now (today) hack up a
patch for util.c that will do the same on Unix.
Unfortunately you can't do early detection because you don't know which
THX is active during the call to free().
>The downside is that each malloc will use an extra 4+ bytes.
It will only be enabled by an additional preprocessor symbol, e.g.
-DTRACK_MEMPOOL
Cheers,
-Jan
Jan Dubois
>I think winFU perl can handle this issue better. I hope Jan will be able to
>comment on my suggestion, on doing this pool testing during the memory
>allocation and not during perl_destruct.
I have attached a patch (relative to Perl 5.8.3) that should reproduce
the "free to wrong pool" errors on Unix too if you build Perl with both
-DPERL_IMPLICIT_CONTEXT and -DPERL_TRACK_MEMPOOL.
Unfortunately there is no THX macro that expands to "PerlInterpreter*",
so I had to violate the encapsulation a little bit. If this gets
applied to core Perl, then we should probably add a macro for that.
Note that the error checking happens during freeing (or reallocating)
of the memory. Most often this happens at perl_destruct() time, but
not necessarily.
You may want to change the croak("panic: ..."); parts to something that
creates an access violation at that point. The Windows code in
win32/vmem.h does this:
int *nowhere = NULL;
*nowhere = 0;
to make sure Perl stops right there. :)
Here is a test program I used to check that the attached patch catches
the wrong free:
/* --------------------------------------------------------------- */
#include <EXTERN.h>
#include <perl.h>
int main()
{
char *argv[] = {"perl", "-e", "warn", NULL};
int argc = 3;
PerlInterpreter *my_perl1 = perl_alloc();
PerlInterpreter *my_perl2;
SV *sv;
perl_construct(my_perl1);
perl_parse(my_perl1, NULL, argc, argv, 0);
perl_run(my_perl1);
sv = Perl_newSVpv(my_perl1, "foo", 3);
warn(SvPVX(sv));
my_perl2 = perl_alloc();
perl_construct(my_perl2);
perl_parse(my_perl2, NULL, argc, argv, 0);
perl_run(my_perl2);
Perl_sv_free(my_perl2, sv);
perl_destruct(my_perl2);
perl_free(my_perl2);
Perl_set_context(my_perl1);
perl_destruct(my_perl1);
perl_free(my_perl1);
return 0;
}
/* --------------------------------------------------------------- */
Running it should produce this output:
Warning: something's wrong at -e line 1.
foo.
Warning: something's wrong at -e line 1.
panic: free from wrong pool.
Cheers,
-Jan
diff -u util.c.orig util.c
--- util.c.orig Mon Jan 19 19:45:59 2004
+++ util.c Wed Jan 21 15:49:31 2004
@@ -60,6 +60,9 @@
my_exit(1);
}
#endif /* HAS_64K_LIMIT */
+#if defined(PERL_IMPLICIT_CONTEXT) && defined(PERL_TRACK_MEMPOOL)
+ size += sizeof(aTHX);
+#endif
#ifdef DEBUGGING
if ((long)size < 0)
Perl_croak_nocontext("panic: malloc");
@@ -67,8 +70,13 @@
ptr = (Malloc_t)PerlMem_malloc(size?size:1); /* malloc(0) is NASTY on our system */
PERL_ALLOC_CHECK(ptr);
DEBUG_m(PerlIO_printf(Perl_debug_log, "0x%"UVxf": (%05ld) malloc %ld bytes\n",PTR2UV(ptr),(long)PL_an++,(long)size));
- if (ptr != Nullch)
+ if (ptr != Nullch) {
+#if defined(PERL_IMPLICIT_CONTEXT) && defined(PERL_TRACK_MEMPOOL)
+ *(PerlInterpreter**)ptr = aTHX;
+ ptr = (Malloc_t)((char*)ptr+sizeof(aTHX));
+#endif
return ptr;
+ }
else if (PL_nomemok)
return Nullch;
else {
@@ -104,6 +112,12 @@
if (!where)
return safesysmalloc(size);
+#if defined(PERL_IMPLICIT_CONTEXT) && defined(PERL_TRACK_MEMPOOL)
+ where = (Malloc_t)((char*)where-sizeof(aTHX));
+ size += sizeof(aTHX);
+ if (*(PerlInterpreter**)where != aTHX)
+ Perl_croak_nocontext("panic: realloc from wrong pool");
+#endif
#ifdef DEBUGGING
if ((long)size < 0)
Perl_croak_nocontext("panic: realloc");
@@ -114,8 +128,12 @@
DEBUG_m(PerlIO_printf(Perl_debug_log, "0x%"UVxf": (%05ld) rfree\n",PTR2UV(where),(long)PL_an++));
DEBUG_m(PerlIO_printf(Perl_debug_log, "0x%"UVxf": (%05ld) realloc %ld bytes\n",PTR2UV(ptr),(long)PL_an++,(long)size));
- if (ptr != Nullch)
+ if (ptr != Nullch) {
+#if defined(PERL_IMPLICIT_CONTEXT) && defined(PERL_TRACK_MEMPOOL)
+ ptr = (Malloc_t)((char*)ptr+sizeof(aTHX));
+#endif
return ptr;
+ }
else if (PL_nomemok)
return Nullch;
else {
@@ -131,11 +149,16 @@
Free_t
Perl_safesysfree(Malloc_t where)
{
-#ifdef PERL_IMPLICIT_SYS
+#if defined(PERL_IMPLICIT_CONTEXT) || defined(PERL_TRACK_MEMPOOL)
dTHX;
#endif
DEBUG_m( PerlIO_printf(Perl_debug_log, "0x%"UVxf": (%05ld) free\n",PTR2UV(where),(long)PL_an++));
if (where) {
+#if defined(PERL_IMPLICIT_CONTEXT) && defined(PERL_TRACK_MEMPOOL)
+ where = (Malloc_t)((char*)where-sizeof(aTHX));
+ if (*(PerlInterpreter**)where != aTHX)
+ Perl_croak_nocontext("panic: free from wrong pool");
+#endif
/*SUPPRESS 701*/
PerlMem_free(where);
}
@@ -161,11 +184,18 @@
Perl_croak_nocontext("panic: calloc");
#endif
size *= count;
+#if defined(PERL_IMPLICIT_CONTEXT) && defined(PERL_TRACK_MEMPOOL)
+ size += sizeof(aTHX);
+#endif
ptr = (Malloc_t)PerlMem_malloc(size?size:1); /* malloc(0) is NASTY on our system */
PERL_ALLOC_CHECK(ptr);
DEBUG_m(PerlIO_printf(Perl_debug_log, "0x%"UVxf": (%05ld) calloc %ld x %ld bytes\n",PTR2UV(ptr),(long)PL_an++,(long)count,(long)size));
if (ptr != Nullch) {
memset((void*)ptr, 0, size);
+#if defined(PERL_IMPLICIT_CONTEXT) && defined(PERL_TRACK_MEMPOOL)
+ *(PerlInterpreter**)ptr = aTHX;
+ ptr = (Malloc_t)((char*)ptr+sizeof(aTHX));
+#endif
return ptr;
}
else if (PL_nomemok)
Jan Dubois
wrote:
>I think I can see two "extra" things that would be useful - the thread ID
You need to store the Perl "context", not the thread ID because multiple
interpreters can live on the same thread. If you interleave access to
them, you can end up with this problem too.
>(as you observe) and a pointer to create a linked list of memory blocks
>allocated under this thread. This way the implementation can free all the
>ithread's memory on ithread exit, which (as I understand it) is equivalent
>to what happens on Windows, but with the benefit that on x86/Linux while
>running under valgrind one will get access errors as soon as another
>ithread attempts to use the now-freed memory.
The freeing of memory at perl_destruct() time is not specific to
Windows, it happens on all platforms, depending on the value of
PL_destruct_level.
The difference on Windows is that all interpreter memory is allocated
from a "local" heap, which is discarded once perl_destruct finishes. So
if another thread allocates memory on it, it may get released to the OS
prematurely. Each interpreter must only hang on to memory allocated
from its own heap.
>I'm not sure of the best way to tweak the layers of obfuscation, er,
>indirection on perl's malloc() use to achieve this. Had someone kind
>at ActiveState volunteer to suggest an interface, or am I misremembering?
I volunteered (and still do) to fix the integer overflow problem for
Perl memory allocation. I still haven't done this, and it is a separate
problem.
>I'm happy to actually implement the guts, if spoon fed the interface
>interception stuff. (Foolish me for volunteering)
I think the patch I just submitted to Stas should allow him to reproduce
the "free to wrong pool" on Linux as well. I'll leave it to him to add
whatever additional diagnostics he needs to figure out where the
corresponding allocation happened. :)
Cheers,
-Jan
Stas Bekman
> On Wed, 21 Jan 2004 20:58:18 +0000, Dave Mitchell
> <da...@fdisolutions.com> wrote:
>
>
>>On Tue, Jan 20, 2004 at 06:57:14PM -0800, Stas Bekman wrote:
>>
>>>So we get these 'free to wrong pool' errors on winfu, usually under
>>
>>What's winfu?
>
>
> I'm assuming Stas means something like "windows knowledge", just like
> patch-fu or kung-fu.
By winFU, I meant win[XP|95|NT|etc]. I'm used to people saying winFU when they
want to refer to 'any MS Windows OS'. I think it was Doug MacEachern from whom
I've heard it first.
> It will only be enabled by an additional preprocessor symbol, e.g.
>
> -DTRACK_MEMPOOL
It's -DPERL_TRACK_MEMPOOL according to your patch, sent in later. I'm going to
try it now. Thanks Jan.
Stas Bekman
> On Wed, 21 Jan 2004 12:43:16 -0800, Stas Bekman <st...@stason.org> wrote:
>
>
>>I think winFU perl can handle this issue better. I hope Jan will be able to
>>comment on my suggestion, on doing this pool testing during the memory
>>allocation and not during perl_destruct.
>
>
> I have attached a patch (relative to Perl 5.8.3) that should reproduce
> the "free to wrong pool" errors on Unix too if you build Perl with both
> -DPERL_IMPLICIT_CONTEXT and -DPERL_TRACK_MEMPOOL.
All I can say: Jan Rocks!
Once I had Jan's patch inside perl, I've immediately reproduced the problems
that windows users had with modperl2 and shortly after that, we had them
fixed. This is after more than 4 months of mostly unsuccessful guesswork and
many wasted hours.
Thanks to this patch I was also able to reproduce the problem with Storable
2.09 to which I've posted a patch a few days ago.
So what does it take to integrate Jan's patch into the core?
I think it should not be optional with -DPERL_TRACK_MEMPOOL, but be there with
-DDEBUGGING. Since it shows real problems which just happen not to show
themselves right away on the current implementation on unix. As more people
start using ithreads (and with modperl 2.0 many will do that soon) we will see
more problems reported on unix, and they will be very hard to reproduce w/o
this alerting feature provided by perl on windows and now ported to unix by Jan.
My only remaining wish is for someone to figure out how to alert on these
wrong context usage problems when they happen and not during the destruct, to
make the debugging so much easier. I understand that this is probably not so
doable. :( At the moment when this kind of problem happens you really have to
guess where the cause is. The best scenario is when you code and test
incrementally and then you know that when you added something new and it
started to fail you hopefully know where the problem is. or may be not, as the
new code could trigger a problem elsewhere.
p.s. to build with Jan's patch, configure perl with something like:
./Configure -des -Dprefix=/whereever/perl-with-jan-patch \
-Dusethreads -Doptimize='-g' -Duseshrplib -Dusedevel \
-Accflags="-DPERL_IMPLICIT_CONTEXT -DPERL_TRACK_MEMPOOL"
to build Jan's test script (assuming that you save it in pool.c) do:
% gcc -o pool pool.c `perl-with-jan-patch -MExtUtils::Embed -e ccopts -e
ldopts` -Wall -g
then run:
% ./pool
Elizabeth Mattijsen
>Jan Dubois wrote:
>>I'm assuming Stas means something like "windows knowledge", just like
>>patch-fu or kung-fu.
>
>By winFU, I meant win[XP|95|NT|etc]. I'm used to people saying winFU
>when they want to refer to 'any MS Windows OS'. I think it was Doug
>MacEachern from whom I've heard it first.
Hmmm.... I always thought the generic term was Win32? As opposed to
Win16 (Windows 3.X and lower) who nobody van remember anymore
(fortunately)... ;-)
Liz
Stas Bekman
Aren't we going to see win64 soon?
Elizabeth Mattijsen
>Elizabeth Mattijsen wrote:
>>At 17:26 -0800 1/21/04, Stas Bekman wrote:
>>
>>>Jan Dubois wrote:
>>>
>>>>I'm assuming Stas means something like "windows knowledge", just like
>>>>patch-fu or kung-fu.
>>>
>>>
>>>By winFU, I meant win[XP|95|NT|etc]. I'm used to people saying
>>>winFU when they want to refer to 'any MS Windows OS'. I think it
>>>was Doug MacEachern from whom I've heard it first.
>>Hmmm.... I always thought the generic term was Win32? As opposed
>>to Win16 (Windows 3.X and lower) who nobody van remember anymore
>>(fortunately)... ;-)
>Aren't we going to see win64 soon?
Exactly. And that's going to have its own set of problems. And
including WinFU in that, would be confusing... ;-)
Liz
Jan Dubois
>My only remaining wish is for someone to figure out how to alert on these
>wrong context usage problems when they happen and not during the destruct, to
>make the debugging so much easier. I understand that this is probably not so
>doable. :(
It is probably impossible to add comprehensive checks for this, but we
_could_ incrementally add checks for all usage patterns that have at
least once triggered the problem.
I've tried to create the support necessary to do this. It is more work
than I thought it would be and I'm running out of time now. I've attached
a patch that will catch all cases where you reallocate the PVX via sv_grow()
from the wrong pool.
One problem is that Perl always uses an arena to allocate SVs, even when
you compile with -DPURIFY. To be able to check the owner of an SV we need
to allocate them individually from the heap. There are also a couple of SVs
that are not SV*s, but structs directly embedded in a bigger struct
(PL_sv_yes etc, PERL_DEBUG_PAD). If we add ownership checks, these SVs
must never go through that code. A better solution would be to allocate
*all* SVs from the heap.
The attached patch still fails various core regressions tests and is not
very clean, but it catches this bug:
/* ---------------------------------------------------- */
#include <EXTERN.h>
#include <perl.h>
void foo(pTHX_ SV *sv) {
/* stringification will allocate a PVX from the wrong pool */
warn(SvPV_nolen(sv));
}
int main()
{
char *argv[] = {"perl", "-e", "warn", NULL};
int argc = 3;
PerlInterpreter *my_perl1 = perl_alloc();
PerlInterpreter *my_perl2;
SV *sv;
perl_construct(my_perl1);
perl_parse(my_perl1, NULL, argc, argv, 0);
perl_run(my_perl1);
sv = Perl_newSViv(my_perl1, 42);
/* make sure sv_2pv() won't upgrade the SV itself. Otherwise we
* won't even reach the part where the PVX is updated.
*/
Perl_sv_upgrade(my_perl1, sv, SVt_PVNV);
my_perl2 = perl_alloc();
perl_construct(my_perl2);
perl_parse(my_perl2, NULL, argc, argv, 0);
perl_run(my_perl2);
foo(my_perl2, sv);
Perl_set_context(my_perl1);
perl_destruct(my_perl1);
perl_free(my_perl1);
Perl_set_context(my_perl2);
perl_destruct(my_perl2);
perl_free(my_perl2);
return 0;
}
/* ---------------------------------------------------- */
I won't have time to work on this anytime soon, so if you think this will
help finding mod_perl problems, please run with it and turn it into a
proper patch. :) Also add checks that would catch all the mod_perl
problems related to this that you have already found and fixed. :)
Note that you have to compile with -DPURIFY if you want to insert
CHECK_MEMPOOL() calls on SV bodies (XIV, XNV etc) and not just on
the SVs or PVXs.
Cheers,
-Jan
The patch requires installation of my earlier "free to wrong pool" patch.
I would also recommend to change the Perl_croak_nocontext() calls in that
previous patch to the method used in CHECK_MEMPOOL() below.
diff -u perl.h.orig perl.h
--- perl.h.orig Mon Jan 19 19:45:50 2004
+++ perl.h Thu Jan 22 10:43:52 2004
@@ -122,6 +122,18 @@
# define pTHX_2 3
# define pTHX_3 4
# define pTHX_4 5
+# if defined(PERL_TRACK_MEMPOOL)
+# define CHECK_MEMPOOL(p) \
+ if (*(PerlInterpreter**)((char*)p-sizeof(aTHX)) != aTHX) { \
+ int *nowhere = NULL; \
+ Perl_warn(aTHX_ "panic: modifying memory from wrong pool"); \
+ *nowhere = 0; \
+ }
+# endif
+#endif
+
+#ifndef CHECK_MEMPOOL
+# define CHECK_MEMPOOL(p)
#endif
#define STATIC static
diff -u perl.c.orig perl.c
--- perl.c.orig Mon Jan 19 19:45:50 2004
+++ perl.c Thu Jan 22 11:14:52 2004
@@ -155,6 +155,23 @@
=cut
*/
+#if defined(PERL_IMPLICIT_SYS) && defined(PERL_TRACK_MEMPOOL)
+STATIC void
+S_sv_initpv(pTHX_ SV *sv, const char *ptr)
+{
+ STRLEN len = strlen(ptr);
+ sv_upgrade(sv, SVt_PV);
+ New(703, SvPVX(sv), len+1, char);
+ Move(ptr, SvPVX(sv), len+1, char);
+ SvCUR(sv) = len;
+ SvLEN(sv) = len+1;
+ SvPOK_only_UTF8(sv);
+}
+# define sv_initpv(sv, ptr) S_sv_initpv(aTHX_ sv, ptr)
+#else
+# define sv_initpv(sv, ptr) sv_setpv(sv, ptr)
+#endif
+
void
perl_construct(pTHXx)
{
@@ -210,12 +227,12 @@
SvREADONLY_on(&PL_sv_undef);
SvREFCNT(&PL_sv_undef) = (~(U32)0)/2;
- sv_setpv(&PL_sv_no,PL_No);
+ sv_initpv(&PL_sv_no,PL_No);
SvNV(&PL_sv_no);
SvREADONLY_on(&PL_sv_no);
SvREFCNT(&PL_sv_no) = (~(U32)0)/2;
- sv_setpv(&PL_sv_yes,PL_Yes);
+ sv_initpv(&PL_sv_yes,PL_Yes);
SvNV(&PL_sv_yes);
SvREADONLY_on(&PL_sv_yes);
SvREFCNT(&PL_sv_yes) = (~(U32)0)/2;
@@ -276,9 +293,9 @@
PL_fdpid = newAV(); /* for remembering popen pids by fd */
PL_modglobal = newHV(); /* pointers to per-interpreter module globals */
PL_errors = newSVpvn("",0);
- sv_setpvn(PERL_DEBUG_PAD(0), "", 0); /* For regex debugging. */
- sv_setpvn(PERL_DEBUG_PAD(1), "", 0); /* ext/re needs these */
- sv_setpvn(PERL_DEBUG_PAD(2), "", 0); /* even without DEBUGGING. */
+ sv_initpv(PERL_DEBUG_PAD(0), ""); /* For regex debugging. */
+ sv_initpv(PERL_DEBUG_PAD(1), ""); /* ext/re needs these */
+ sv_initpv(PERL_DEBUG_PAD(2), ""); /* even without DEBUGGING. */
#ifdef USE_ITHREADS
PL_regex_padav = newAV();
av_push(PL_regex_padav,(SV*)newAV()); /* First entry is an array of empty elements */
diff -u sv.c.orig sv.c
--- sv.c.orig Mon Jan 19 19:45:53 2004
+++ sv.c Thu Jan 22 11:18:24 2004
@@ -170,6 +170,21 @@
} STMT_END
+#if defined(PERL_IMPLICIT_CONTEXT) && defined(PERL_TRACK_MEMPOOL)
+STATIC SV*
+S_new_SV()
+{
+ SV* sv = safemalloc(sizeof(SV));
+ SvANY(sv) = 0;
+ SvREFCNT(sv) = 1;
+ SvFLAGS(sv) = 0;
+ return sv;
+}
+
+# define new_SV(p) (p)=S_new_SV()
+# define del_SV(p) safefree((char*)p)
+#else
+
/* new_SV(): return a new, empty SV head */
#ifdef DEBUG_LEAKING_SCALARS
@@ -253,6 +268,7 @@
#endif /* DEBUGGING */
+#endif
/*
=head1 SV Manipulation Functions
@@ -1633,6 +1649,7 @@
SvFAKE_off(sv);
SvREADONLY_off(sv);
}
+ CHECK_MEMPOOL(sv);
New(703, s, newlen, char);
if (SvPVX(sv) && SvCUR(sv)) {
Move(SvPVX(sv), s, (newlen < SvCUR(sv)) ? newlen : SvCUR(sv), char);
@@ -10603,6 +10620,23 @@
proto_perl->IProc);
}
+#if defined(PERL_IMPLICIT_SYS) && defined(PERL_TRACK_MEMPOOL)
+STATIC void
+S_sv_initpv(pTHX_ SV *sv, const char *ptr)
+{
+ STRLEN len = strlen(ptr);
+ sv_upgrade(sv, SVt_PV);
+ New(703, SvPVX(sv), len+1, char);
+ Move(ptr, SvPVX(sv), len+1, char);
+ SvCUR(sv) = len;
+ SvLEN(sv) = len+1;
+ SvPOK_only_UTF8(sv);
+}
+# define sv_initpv(sv, ptr) S_sv_initpv(aTHX_ sv, ptr)
+#else
+# define sv_initpv(sv, ptr) sv_setpv(sv, ptr)
+#endif
+
PerlInterpreter *
perl_clone_using(PerlInterpreter *proto_perl, UV flags,
struct IPerlMem* ipM, struct IPerlMem* ipMS,
@@ -10818,9 +10852,9 @@
#endif
PL_encoding = sv_dup(proto_perl->Iencoding, param);
- sv_setpvn(PERL_DEBUG_PAD(0), "", 0); /* For regex debugging. */
- sv_setpvn(PERL_DEBUG_PAD(1), "", 0); /* ext/re needs these */
- sv_setpvn(PERL_DEBUG_PAD(2), "", 0); /* even without DEBUGGING. */
+ sv_initpv(PERL_DEBUG_PAD(0), ""); /* For regex debugging. */
+ sv_initpv(PERL_DEBUG_PAD(1), ""); /* ext/re needs these */
+ sv_initpv(PERL_DEBUG_PAD(2), ""); /* even without DEBUGGING. */
/* Clone the regex array */
PL_regex_padav = newAV();
Stas Bekman
is stock 5.8.3 w/debug, B is the same build plus Jan's patch.
% bench /home/stas/perl/5.8.3-ithread/bin/perl5.8.3
/home/stas/perl/5.8.3-ithread-pool/bin/perl5.8.3
A) perl-5.008003
path = /home/stas/perl/5.8.3-ithread/bin/perl5.8.3
cc = cc
optimize = -g
ccflags = -D_REENTRANT -D_GNU_SOURCE -DTHREADS_HAVE_PIDS
-DDEBUGGING -fno-strict-aliasing -I/usr/local/include -D_LARGEFILE_SOURCE
-D_FILE_OFFSET_BITS=64 -I/usr/include/gdbm
usemymalloc = n
B) perl-5.008003
path = /home/stas/perl/5.8.3-ithread-pool/bin/perl5.8.3
cc = cc
optimize = -g
ccflags = -D_REENTRANT -D_GNU_SOURCE -DTHREADS_HAVE_PIDS
-DPERL_IMPLICIT_CONTEXT -DPERL_TRACK_MEMPOOL -DDEBUGGING -fno-strict-aliasing
-I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64
-I/usr/include/gdbm
usemymalloc = n
A B
---- ----
arith/mixed 100 114
arith/trig 100 102
array/copy 100 124
array/foreach 100 110
array/index 100 147
array/pop 100 104
array/shift 100 107
array/sort-num 100 94
array/sort 100 98
call/0arg 100 104
call/1arg 100 98
call/2arg 100 103
call/9arg 100 95
call/empty 100 100
call/fib 100 98
call/method 100 121
call/wantarray 100 108
hash/copy 100 102
hash/each 100 106
hash/foreach-sort 100 112
hash/foreach 100 113
hash/get 100 106
hash/set 100 106
loop/for-c 100 114
loop/for-range-const 100 130
loop/for-range 100 123
loop/getline 100 105
loop/while-my 100 152
loop/while 100 97
re/const 100 103
re/w 100 103
startup/fewmod 100 99
startup/lotsofsub 100 87
startup/noprog 100 101
string/base64 100 105
string/htmlparser 100 111
string/index-const 100 153
string/index-var 100 113
string/ipol 100 125
string/tr 100 101
AVERAGE 100 110
Dave Mitchell
> On Wed, 21 Jan 2004 12:43:16 -0800, Stas Bekman <st...@stason.org> wrote:
> I have attached a patch (relative to Perl 5.8.3) that should reproduce
> the "free to wrong pool" errors on Unix too if you build Perl with both
> -DPERL_IMPLICIT_CONTEXT and -DPERL_TRACK_MEMPOOL.
For this patch to go "mainstream", one of things it would need to handle
is alignment constraints. For example on SPARC in 32-bit mode, pointers
are 4 bytes, doubles are 8 bytes, and doubles must be be aligned on an
8-byte boundary. The Solaris malloc() will always return addresses aligned
on an 8-byte boundary; adding sizeof(aTHX) and returning it would hand the
application a pointer which can't hold arbitrary data. Eg the following
gives a bus fault on SPARC:
char *p = (char *)malloc(sizeof(void*) + sizeof(double));
if (!p) exit(0);
p+=sizeof(void *);
*((double *)p) = 1.1;
--
To collect all the latest movies, simply place an unprotected ftp server
on the Internet, and wait for the disk to fill....
Stas Bekman
> I won't have time to work on this anytime soon, so if you think this will
> help finding mod_perl problems, please run with it and turn it into a
> proper patch. :) Also add checks that would catch all the mod_perl
> problems related to this that you have already found and fixed. :)
>
> Note that you have to compile with -DPURIFY if you want to insert
> CHECK_MEMPOOL() calls on SV bodies (XIV, XNV etc) and not just on
> the SVs or PVXs.
>
> Cheers,
> -Jan
>
> The patch requires installation of my earlier "free to wrong pool" patch.
> I would also recommend to change the Perl_croak_nocontext() calls in that
> previous patch to the method used in CHECK_MEMPOOL() below.
unfortunately I can't even build perl with this patch (I didn't use -DPURIFY),
but:
./Configure -des -Dprefix=/home/stas/perl/5.8.3-ithread-pool \
-Dusethreads -Doptimize='-g' -Duseshrplib -Dusedevel \
-Accflags="-DPERL_IMPLICIT_CONTEXT -DPERL_TRACK_MEMPOOL"
It fails to build miniperl:
% make
....
LD_LIBRARY_PATH=/home/stas/perl.org/perl-5.8.3
/home/stas/perl.org/perl-5.8.3/preload
/home/stas/perl.org/perl-5.8.3/libperl.so cc -L/usr/local/lib -o miniperl \
miniperlmain.o opmini.o libperl.so -lnsl -ldl -lm -lcrypt -lutil
-lpthread -lc
panic: modifying memory from wrong pool.
make: *** [miniperl] Segmentation fault
the preloaded libperl.so causes that, e.g I get the same with just calling cc:
env LD_LIBRARY_PATH=/home/stas/perl.org/perl-5.8.3
/home/stas/perl.org/perl-5.8.3/preload
/home/stas/perl.org/perl-5.8.3/libperl.so cc
panic: modifying memory from wrong pool.
Segmentation fault (core dumped)
Jan Dubois
>Jan Dubois wrote:
>
>> I won't have time to work on this anytime soon, so if you think this will
>> help finding mod_perl problems, please run with it and turn it into a
>> proper patch. :) Also add checks that would catch all the mod_perl
>> problems related to this that you have already found and fixed. :)
>>
>> Note that you have to compile with -DPURIFY if you want to insert
>> CHECK_MEMPOOL() calls on SV bodies (XIV, XNV etc) and not just on
>> the SVs or PVXs.
>>
>> Cheers,
>> -Jan
>>
>> The patch requires installation of my earlier "free to wrong pool" patch.
>> I would also recommend to change the Perl_croak_nocontext() calls in that
>> previous patch to the method used in CHECK_MEMPOOL() below.
>
>unfortunately I can't even build perl with this patch (I didn't use -DPURIFY),
>but:
Weird, it worked for me on Windows...
I looked at the patch again and noticed at least 2 problems:
1) add a check for PERL_IMPLICIT_CONTEXT here:
| diff -u perl.h.orig perl.h
| --- perl.h.orig Mon Jan 19 19:45:50 2004
| +++ perl.h Thu Jan 22 10:43:52 2004
| @@ -122,6 +122,18 @@
| # define pTHX_2 3
| # define pTHX_3 4
| # define pTHX_4 5
| +# if defined(PERL_TRACK_MEMPOOL)
should be:
# if defined(PERL_TRACK_MEMPOOL) && defined(PERL_IMPLICIT_CONTEXT)
| +# define CHECK_MEMPOOL(p) \
| +
2) the patch contained a few "defined(PERL_IMPLICIT_SYS)" checks. They
should have been testing PERL_IMPLICIT_CONTEXT instead.
Cheers,
-Jan
Stas Bekman
Thanks, Jan. Plus one more:
3) this part:
diff -u sv.c.orig sv.c
--- sv.c.orig Mon Jan 19 19:45:53 2004
+++ sv.c Thu Jan 22 11:18:24 2004
@@ -170,6 +170,21 @@
} STMT_END
[...]
+#if defined(PERL_IMPLICIT_CONTEXT) && defined(PERL_TRACK_MEMPOOL)
+STATIC void
+S_sv_initpv(pTHX_ SV *sv, const char *ptr)
+{
+ STRLEN len = strlen(ptr);
+ sv_upgrade(sv, SVt_PV);
+ New(703, SvPVX(sv), len+1, char);
+ Move(ptr, SvPVX(sv), len+1, char);
+ SvCUR(sv) = len;
+ SvLEN(sv) = len+1;
+ SvPOK_only_UTF8(sv);
+}
+# define sv_initpv(sv, ptr) S_sv_initpv(aTHX_ sv, ptr)
+#else
+# define sv_initpv(sv, ptr) sv_setpv(sv, ptr)
+#endif
+
needs to be moved one function up, before perl_clone definition. before I did
that I was getting undefined symbol sv_initpv.
But 'make test' hangs in the closure.t test. Did all tests pass for you, Jan?
Jan Dubois
problems.]
On Sat, 24 Jan 2004 17:31:57 -0800, Stas Bekman <st...@stason.org> wrote:
>Thanks, Jan. Plus one more:
>
>3) this part:
[...]
>needs to be moved one function up, before perl_clone definition. before I did
>that I was getting undefined symbol sv_initpv.
Yes, I see, it needs to be outside the #ifdef PERL_IMPLICIT_SYS, which
is only defined on Windows. Sorry about that.
>But 'make test' hangs in the closure.t test. Did all tests pass for you, Jan?
No, they don't:
| The attached patch still fails various core regressions tests and is not
| very clean, but it catches this bug:
That's what I meant by "I'm running out of time" as I can tell how much
effort may be needed to get this working somewhat cleanly.
I'm not sure if this is really worth the effort. If you only add the
part that allocates and frees SVs from the heap instead of using the
arena, then you should already catch all cases where an SV is upgraded
because the Safefree() of the old any pointer will already trigger an
error. That may be enough to catch most problems immediately.
Cheers,
-Jan
Stas Bekman
[...]
>>But 'make test' hangs in the closure.t test. Did all tests pass for you, Jan?
>
>
> No, they don't:
>
> | The attached patch still fails various core regressions tests and is not
> | very clean, but it catches this bug:
Oops, sorry about that.
> That's what I meant by "I'm running out of time" as I can tell how much
> effort may be needed to get this working somewhat cleanly.
I understand.
> I'm not sure if this is really worth the effort. If you only add the
> part that allocates and frees SVs from the heap instead of using the
> arena, then you should already catch all cases where an SV is upgraded
> because the Safefree() of the old any pointer will already trigger an
> error. That may be enough to catch most problems immediately.
I haven't really had chance to look at what your patches do yet. I have just
used your first patch and it worked great. I need to take time to grok the
change. I guess we could try and see first how can we get that part it in,
taking into account Dave Mitchell's comments about the alignment constraints.
Nicholas Clark
> in, taking into account Dave Mitchell's comments about the alignment
> constraints.
IIRC There's no official guarantee in C about which type is largest, or
which type has what alignment constraints.
I spot
/* MEM_ALIGNBYTES:
* This symbol contains the number of bytes required to align a
* double, or a long double when applicable. Usual values are 2,
* 4 and 8. The default is eight, for safety.
*/
in config.h, but as it's
#if defined(USE_CROSS_COMPILE) || defined(MULTIARCH)
# define MEM_ALIGNBYTES 8
#else
#define MEM_ALIGNBYTES 4
#endif
I don't think that it's correct. (What's the long double alignment constraint
on Solaris?)
Would something like sizeof() a union of the thread data, a long double
and a pointer be good enough to get the alignment constraint on practically
every system?
Nicholas Clark
Dave Mitchell
I think you'd want more something like:
struct {
aTHX base,
union { long double d, void *p, .... } first
}
and make the increment the difference in address between base and first.
--
Please note that ash-trays are provided for the use of smokers,
whereas the floor is provided for the use of all patrons.
-- Bill Royston