perl's times() support is messy, esp on Win32

[p5pRT]

Migrated from rt.perl.org#124152 (status was 'open')

Searchable as RT124152$

[p5pRT]

From rosiejjjj@outlook.com

Created by @bulk88

I looked at Win32 Perl's implementation of times() today, which is below.

DllExport int
win32_times(struct tms *timebuf)
{
FILETIME user;
FILETIME kernel;
FILETIME dummy;
if (GetProcessTimes(GetCurrentProcess(), &dummy, &dummy,
&kernel,&user)) {
timebuf->tms_utime = filetime_to_clock(&user);
timebuf->tms_stime = filetime_to_clock(&kernel);
timebuf->tms_cutime = 0;
timebuf->tms_cstime = 0;
} else {
/* That failed - e.g. Win95 fallback to clock() */
timebuf->tms_utime = process_time_so_far;
timebuf->tms_stime = 0;
timebuf->tms_cutime = 0;
timebuf->tms_cstime = 0;
}
return process_time_so_far;
}

Problems, 1. it looks like there is Win95 code which was forgotten to be
removed. Look at WinME kernel32.dll, I see GetProcessTimes always
returns false and GLR ERROR_CALL_NOT_IMPLEMENTED. times is supposed to
return -1 on failure according to posix, but looking at pp_tms,

PP(pp_tms)
{
#ifdef HAS_TIMES
dSP;
struct tms timesbuf;

EXTEND(SP, 4);
(void)PerlProc_times(&timesbuf);

mPUSHn(((NV)timesbuf.tms_utime)/(NV)PL_clocktick);
if (GIMME_V == G_ARRAY) {
mPUSHn(((NV)timesbuf.tms_stime)/(NV)PL_clocktick);
mPUSHn(((NV)timesbuf.tms_cutime)/(NV)PL_clocktick);
mPUSHn(((NV)timesbuf.tms_cstime)/(NV)PL_clocktick);
}
RETURN;
#else
# ifdef PERL_MICRO
dSP;
mPUSHn(0.0);
EXTEND(SP, 4);
if (GIMME_V == G_ARRAY) {
mPUSHn(0.0);
mPUSHn(0.0);
mPUSHn(0.0);
}
RETURN;
# else
DIE(aTHX_ "times not implemented");
# endif
#endif /* HAS_TIMES */
}

the retval is unused, and potentially, strictly according to the book,
timesbuf will be uninitialized data if times()s returns -1. So why is
win32_times calling clock() or the ret type isn't void? I'd like to
clean up this code but there is no straight way to do this. Is pp_tms
correct? should win32_times be made just for pp_tms and posix
compatibility is irrelevant? Should pp_tms be fixed to be posix
compatible? is times() failing on posix too remote (hardware memory
corruption/water/soon a BSOD or kernel panic) to worth checking for?
Larry Wall removed checking the retval in 2.000 378cc40
and nobody changed it since (yes I git blamed that through a dozen
files). IDK why. My poor attempt at searching Perl RT doesn't reveal
any tickets complaining about times() failing.

I think MS CRT clock() is too complicated in implementation for the
retval of win32_times, since GetTickCount() is faster, since it is
literally a read of a global var in kernel32.dll vs all the math and
conversions in the MS implementation of clock(). win32_times should
probably croak("panic: times() failed") if GetProcessTimes returns false
on NT OSes since Win95/98 and its ERROR_CALL_NOT_IMPLEMENTED are gone.

What about making PL_clocktick a constant (if possible on the platform)
or process (not interp) global? It has no CPAN usage
http://grep.cpan.me/?q=clocktick . On Win32, there is no sysconf(), so
clocktick is a constant (it is CLK_TCK in MSCRT which is a constant in
time.h set at 1000). Division by a constant is usually turned into CPU
multiplication or CPU shift ops by the CC which are faster than CPU
division.

related links

http://code.metager.de/source/xref/gnu/octave/gnulib-hg/lib/times.c
http://code.metager.de/source/xref/gnu/glibc/sysdeps/unix/sysv/linux/times.c
#13086

Perl Info

Flags:
                 category=core
                 severity=low

Site configuration information for perl 5.21.4:

Configured by Owner at Thu Sep 18 12:08:58 2014.

Summary of my perl5 (revision 5 version 21 subversion 4) configuration:
               Derived from: 7d2b2edb94ab56333b9049a3e26d15ea18445512
               Ancestor: 19be3be6968e2337bcdfe480693fff795ecd1304
               Platform:
                 osname=MSWin32, osvers=5.1,
archname=MSWin32-x86-multi-thread
                 uname=''
                 config_args='undef'
                 hint=recommended, useposix=true, d_sigaction=undef
                 useithreads=define, usemultiplicity=define
                 use64bitint=undef, use64bitall=undef, uselongdouble=undef
                 usemymalloc=n, bincompat5005=undef
               Compiler:
                 cc='cl', ccflags ='-nologo -GF -W3 -O1 -MD -Zi -DNDEBUG
-DWIN32
-D_CONSOLE -DNO_STRICT  -DPERL_TEXTMODE_SCRIPTS
-DPERL_HASH_FUNC_ONE_AT_A_TIME -DPERL_IMPLICIT_CONTEXT
-DPERL_IMPLICIT_SYS -DUSE_PERLIO -D_USE_32BIT_TIME_T',
                 optimize='-O1 -MD -Zi -DNDEBUG',
                 cppflags='-DWIN32'
                 ccversion='12.00.8168', gccversion='', gccosandvers=''
                 intsize=4, longsize=4, ptrsize=4, doublesize=8,
byteorder=1234
                 d_longlong=undef, longlongsize=8, d_longdbl=define,
longdblsize=8,
longdblkind=0
                 ivtype='long', ivsize=4, nvtype='double', nvsize=8,
Off_t='__int64',
lseeksize=8
                 alignbytes=8, prototype=define
               Linker and Libraries:
                 ld='link', ldflags ='-nologo -nodefaultlib -debug
-opt:ref,icf
-libpath:"c:\perl521\lib\CORE"  -machine:x86'
                 libpth=C:\PROGRA~1\MIAF9D~1\VC98\lib
                 libs=oldnames.lib kernel32.lib user32.lib gdi32.lib
winspool.lib
comdlg32.lib advapi32.lib shell32.lib ole32.lib oleaut32.lib
netapi32.lib uuid.lib ws2_32.lib mpr.lib winmm.lib  version.lib
odbc32.lib odbccp32.lib comctl32.lib msvcrt.lib
                 perllibs=oldnames.lib kernel32.lib user32.lib gdi32.lib
winspool.lib  comdlg32.lib advapi32.lib shell32.lib ole32.lib
oleaut32.lib  netapi32.lib uuid.lib ws2_32.lib mpr.lib winmm.lib
version.lib odbc32.lib odbccp32.lib comctl32.lib msvcrt.lib
                 libc=msvcrt.lib, so=dll, useshrplib=true,
libperl=perl521.lib
                 gnulibc_version=''
               Dynamic Linking:
                 dlsrc=dl_win32.xs, dlext=dll, d_dlsymun=undef, 
ccdlflags=' '
                 cccdlflags=' ', lddlflags='-dll -nologo -nodefaultlib 
-debug
-opt:ref,icf  -libpath:"c:\perl521\lib\CORE"  -machine:x86'

Locally applied patches:
                 uncommitted-changes
                 a0fe7a7e75de29e59f1da0d6822dc06e5be658fe
                 a261faffee83d0145642ab5d1d046c9f813bc497
                 6506ab86ad1602a9ca720fcd30446dce1461d23d
                 7d2b2edb94ab56333b9049a3e26d15ea18445512


@INC for perl 5.21.4:
                 lib
                 C:/perl521/srcnew/lib
                 .


Environment for perl 5.21.4:
                 HOME (unset)
                 LANG (unset)
                 LANGUAGE (unset)
                 LD_LIBRARY_PATH (unset)
                 LOGDIR (unset)
                 PATH=
                 PERL_BADLANG (unset)
                 PERL_JSON_BACKEND=Cpanel::JSON::XS
                 PERL_YAML_BACKEND=YAML
                 SHELL (unset)

[p5pRT]

From @jkeenan

Is there any particular aspect of the win32 implementation of times() for which you would like to propose a patch?

Thank you very much.

--
James E Keenan (jkeenan@​cpan.org)

[p5pRT]

The RT System itself - Status changed from 'new' to 'open'

[toddr]

@bulk88, I'm with @jkeenan on this. Barring any patch, I don't see a bug per-se, correct?