PERL_MEM_LOG does not work with PERL_IMPLICIT_SYS

[khwilliamson]

Description

It is possible to compile with PERL_MEM_LOG on Windows, but if you try to use it by setting the environment variable, it will crash with no output, as it recurses infinitely until it runs out of memory.

03694582f8c247d4a1cc8a7bb8348af0173944d7 fixed this in 5.34, but it did not address PERL_IMPLICIT_SYS builds

Steps to Reproduce

On Windows, change win32/Makefile to include something like EXTRACFLAGS = -nologo -GF -W3 -DPERL_MEM_LOG

then run.

set the environment variable PERL_MEM_LOG=m

and run perl -v

Expected behavior

Logging of perl memory allocations.

Perl configuration

`This is perl 5, version 37, subversion 12 (v5.37.12 (v5.37.11-299-g0cafffb0c6*)) built for MSWin32-x64-multi-thread
``

[tonycoz]

This isn't limited to PERL_IMPLICIT_SYS builds. I built with:

gmake CCTYPE=MSVC143 BUILDOPTEXTRA=-DPERL_MEM_LOG USE_MULTI=undef USE_IMP_SYS=undef USE_ITHREADS=undef CFG=Debug test-prep

and still had deep recursion and a stack overflow:

>   perl537.dll!win32_getenv(const char * name) Line 2309   C
    perl537.dll!Perl_mortal_getenv(const char * str) Line 3332  C
    perl537.dll!S_mem_log_common(mem_log_type mlt, const unsigned __int64 n, const unsigned __int64 typesize, const char * type_name, const sv * sv, void * oldalloc, void * newalloc, const char * filename, const int linenumber, const char * funcname) Line 4898    C
    perl537.dll!Perl_mem_log_alloc(const unsigned __int64 n, const unsigned __int64 typesize, const char * type_name, void * newalloc, const char * filename, const int linenumber, const char * funcname) Line 5031    C
    perl537.dll!Perl_more_sv() Line 277 C
    perl537.dll!Perl_newSV_type(const svtype type) Line 376 C
    perl537.dll!Perl_newSVpvn(const char * const buffer, const unsigned __int64 len) Line 9660  C
    perl537.dll!win32_getenv(const char * name) Line 2311   C
    perl537.dll!Perl_mortal_getenv(const char * str) Line 3332  C
    perl537.dll!S_mem_log_common(mem_log_type mlt, const unsigned __int64 n, const unsigned __int64 typesize, const char * type_name, const sv * sv, void * oldalloc, void * newalloc, const char * filename, const int linenumber, const char * funcname) Line 4898    C
    perl537.dll!Perl_mem_log_alloc(const unsigned __int64 n, const unsigned __int64 typesize, const char * type_name, void * newalloc, const char * filename, const int linenumber, const char * funcname) Line 5031    C
    perl537.dll!Perl_more_sv() Line 277 C
    perl537.dll!Perl_newSV_type(const svtype type) Line 376 C
    perl537.dll!Perl_newSVpvn(const char * const buffer, const unsigned __int64 len) Line 9660  C
    perl537.dll!win32_getenv(const char * name) Line 2311   C
    perl537.dll!Perl_mortal_getenv(const char * str) Line 3332  C
    perl537.dll!S_mem_log_common(mem_log_type mlt, const unsigned __int64 n, const unsigned __int64 typesize, const char * type_name, const sv * sv, void * oldalloc, void * newalloc, const char * filename, const int linenumber, const char * funcname) Line 4898    C
    perl537.dll!Perl_mem_log_alloc(const unsigned __int64 n, const unsigned __int64 typesize, const char * type_name, void * newalloc, const char * filename, const int linenumber, const char * funcname) Line 5031    C
    perl537.dll!Perl_more_sv() Line 277 C
    perl537.dll!Perl_newSV_type(const svtype type) Line 376 C
    perl537.dll!Perl_newSVpvn(const char * const buffer, const unsigned __int64 len) Line 9660  C
    perl537.dll!win32_getenv(const char * name) Line 2311   C
...

[khwilliamson]

I suspect that this patch to util.c will cause things to work

>  4898a4899,4906
 > #ifdef WIN32
 > 
 >     if (PL_phase == PERL_PHASE_CONSTRUCT) {
 >         return;
 >     }
 > 
 > #endif

Also, with this patch to win32/perlhost.h I can get the whole thing to work, but I don't know if this is the right patch

> 456c456,457
 <     return IPERL2HOST(piPerl)->Getenv(varname);
 ---
 > 
 >     return Perl_mortal_getenv(varname);

Edit: TonyC added some ```

[tonycoz]

I've added ``` around the inline patches, github used the > as quoting markers.

I don't think the second is correct, this code is what is indirectly called by PerlEnv_getenv(), so win32_getenv() would be skipped entirely, as well as the per-thread/pseudo-process environment emulation done in CPerlHost::Getenv().

I suspect win32_getenv() will need to do the same type of pre-check for PERL_MEM_LOG that Perl_mortal_getenv() does, but there's enough of a mess of configurations that I don't think there's a simple fix :(

[hvds]

I haven't looked at the code involved, but is it possible to have a flag to say "mem log initialized", set it only after the environment variable is fetched, and avoid malloc logging until that flag is set?

[khwilliamson]

Actually win32_getenv() does get called, as Perl_mortal_getenv() calls plain getenv() and a windows header redefines getenv() to instead be win32_getenv(). I know nothing about the CPerlHost::Getenv(). Is there documentation about how the pseudo fork works?

The logging design deliberately allows the environment variable to change at any time. It is fetched anew at each potential logging event. I presume that this so so a script can do a putenv just before a section of problematic code, and turn if off just afterwards. And I presume this is so one can limit the size of the output that needs to be pored through. Thus a flag wouldn't work

[tonycoz]

Actually win32_getenv() does get called, as Perl_mortal_getenv() calls plain getenv() and a windows header redefines getenv() to instead be win32_getenv().

There's two defines that I see for getenv():

• XSUB.h - That definition only applies when building XS outside of the core.
• win32/win32iop.h - only defined when WIN32IO_IS_STDIO not defined, which it is for PERL_IMPLICIT_SYS (win32/win32.h)

If I gmake -j2 CCTYPE=MSVC143 ..\mg.i the resulting calls to getenv() in mortal_getenv() are calls to getenv() itself, not to win32_getenv().

Without PERL_IMPLICIT_SYS the win32iop.h definition does apply, as you can see from my backtrace.

I know nothing about the CPerlHost::Getenv(). Is there documentation about how the pseudo fork works?

There pod/perlfork.pod, but that's mostly the user view.

From what I understand each pseudo process has a CPerlHost object associated with it, and each thread in that process uses that pseudo-process's CPerlHost object.

On a pseudo-fork (see win32/perlhost.h PerlProcFork()) a new CPerlHost object is created, copying some data from the parent CPerlHost object, and creating new objects in some cases (like the memory tracking object).

From looking at the implementation of CPerlHost::Getenv(), for child pseudo-processes it fetches from the host object's environment table - not from the process environment (win32_getenv() is not called). Similarly CPerlHost::Putenv() doesn't write to the Win32 process environment, just to the table in the host object. Which could be a problem for multi-threading, since we can have more than one thread in a pseudo-process.