Perl / perl5

🐪 The Perl programming language
https://dev.perl.org/perl5/
Other
1.97k stars 560 forks source link

uninit mem read in win32_fseek() randomly creates multi GB files on Win64 #13677

Closed p5pRT closed 10 years ago

p5pRT commented 10 years ago

Migrated from rt.perl.org#121471 (status was 'resolved')

Searchable as RT121471$

p5pRT commented 10 years ago

From @bulk88

Created by @bulk88

Using "Microsoft (R) C/C++ Optimizing Compiler Version 15.00.30729.01 for x64" Visual C 2008 x64\, atleast with -O1 non DEBUGGING\, there is a uninit memory read in win32_fseek in win32.c due to what I think is a compiler bug since I can't find anything wrong with the C code. Win64 passes the first 4 args in registers\, with shadow space on C stack mirroring the registers. If a function chooses so\, (win32_fseek MUST do this)\, it can copy the incoming register to the C stack mirroring slot. win32_fseek does a 4 byte asm mov instead of a 8 byte mov. It laster does "&" on C stack\, and passes that to fsetpos. The upper 4 bytes are uninitialized C stack now. Those bytes by chance now contain 0x00000001. Hench ~ 4 GB of data being written.

Specifically a file is extended to 4 GB file is randomly (over the last couple years by me\, I never investigated it\, but it is repeatable if your commit happens to being a failing one that causes the upper 4 bytes to not be zero for some reason\, the garbage/uninit mem on C stack happens to be 0x0000000140074379 from some other previous pp_* opcode since pp_sysseek never wrote to that piece of C stack memory\, which leaves the 33rd bit being high). The command is "..\miniperl -I..\lib bin\exetype.pl ..\wperl.exe WINDOWS". On win32 Perl type Off_t is a __int64. At http​://perl5.git.perl.org/perl.git/blob/f0fe019a81bffdc6f23c6c00d5fa100df21f8428​:/win32/bin/exetype.pl#l48

------------------------------------ seek EXE\, $offset+4+20+68\, 0; ------------------------------------

Happens. The sum of $offset+4+20+68 should be\, and on PP level is ~ 0x150. The C callstack for that line is

------------------------------------- kernel32.dll!SetFilePointer() msvcr90.dll!_lseeki64_nolock(int fh=4\, __int64 pos=4294967636\, int mthd=0) Line 146 + 0x15 bytes C msvcr90.dll!_lseeki64(int fh=4\, __int64 pos=2020275856\, int mthd=0) Line 85 + 0xd bytes C msvcr90.dll!_fseeki64_nolock(_iobuf * str=0x00000000786af690\, __int64 offset=28048264\, int whence=340) Line 142 + 0x15 bytes C msvcr90.dll!_fseeki64(_iobuf * stream=0x00000000786af690\, __int64 offset=2020275856\, int whence=340) Line 65 + 0xe bytes C msvcr90.dll!fsetpos(_iobuf * stream=0x0000000000000154\, const __int64 * pos=0x000000014009c1bc) Line 46 C miniperl.exe!win32_fseek(_iobuf * pf=0x0000000001a7d590\, long offset=340\, int origin=28049368) Line 2843 C miniperl.exe!Perl_pp_sysseek() Line 2169 + 0x3e bytes C miniperl.exe!Perl_runops_standard() Line 42 + 0x3 bytes C miniperl.exe!S_run_body(long oldscope=0) Line 2451 C miniperl.exe!perl_run(interpreter * my_perl=0x0000000001a73c60) Line 2365 + 0x9 bytes C miniperl.exe!main(int argc=0\, char * * argv=0x0001686340c4f679\, char * * env=0x0000000000000000) Line 122 C miniperl.exe!__tmainCRTStartup() Line 582 + 0x19 bytes C kernel32.dll!BaseProcessStart() + 0x2c bytes ----------------------------------------------

Note the first pos is 340 bytes (~0x150) above 2^32. This really is the value passed to the kerenel. The disk thrashing doesn't start until a print or close is done\, for example --------------------------------------------------------- ntdll.dll!NtWriteFile() + 0xa bytes kernel32.dll!WriteFile() + 0xab bytes msvcr90.dll!_write_nolock(int fh=4\, const void * buf=0x0000000000000160\, unsigned int cnt=0) Line 472 + 0x1a bytes C msvcr90.dll!_write(int fh=4\, const void * buf=0x00000000786af690\, unsigned int cnt=0) Line 75 + 0xd bytes C msvcr90.dll!_flush(_iobuf * str=0x00000000786af690) Line 154 + 0x13 bytes C msvcr90.dll!_fclose_nolock(_iobuf * str=0x00000000786af690) Line 106 C msvcr90.dll!fclose(_iobuf * stream=0x00000000786af690) Line 57 + 0x8 bytes C miniperl.exe!Perl_io_close(io * io=0x0000000001b34a10\, char not_implicit='') Line 984 + 0xc bytes C miniperl.exe!Perl_do_close(gv * gv=0x0000000001afccc8\, char not_implicit='˜') Line 946 C miniperl.exe!Perl_pp_close() Line 670 + 0xe bytes C miniperl.exe!Perl_runops_standard() Line 42 + 0x3 bytes C miniperl.exe!S_run_body(long oldscope=0) Line 2451 C miniperl.exe!perl_run(interpreter * my_perl=0x0000000001af2d10) Line 2365 + 0x9 bytes C miniperl.exe!main(int argc=0\, char * * argv=0x0001624349b91237\, char * * env=0x0000000000000000) Line 122 C miniperl.exe!__tmainCRTStartup() Line 582 + 0x19 bytes C kernel32.dll!BaseProcessStart() + 0x2c bytes ---------------------------------------------------------

Asm dump of the broken win32_fseek from the binary --------------------------------------------------------- 2816​: 2817​: DllExport int 2818​: win32_fseek(FILE *pf\, Off_t offset\,int origin) 2819​: { 00000001400CF300 mov dword ptr [rsp+10h]\,edx ;;;;;;;;;;;;PROBLEM RIGHT HERE\, 4 byte copy\, not 8 byte copy 00000001400CF304 push rbx 00000001400CF305 sub rsp\,20h 00000001400CF309 mov rbx\,rcx 2820​: #if defined(WIN64) || defined(USE_LARGE_FILES) 2821​: fpos_t pos; 2822​: switch (origin) { 00000001400CF30C test r8d\,r8d 00000001400CF30F je win32_fseek+6Bh (1400CF36Bh) 00000001400CF311 sub r8d\,1 00000001400CF315 je win32_fseek+4Fh (1400CF34Fh) 00000001400CF317 cmp r8d\,1 00000001400CF31B je win32_fseek+2Bh (1400CF32Bh) 2820​: #if defined(WIN64) || defined(USE_LARGE_FILES) 2821​: fpos_t pos; 2822​: switch (origin) { 2823​: case SEEK_CUR​: 2824​: if (fgetpos(pf\, &pos)) 2825​: return -1; 2826​: offset += pos; 2827​: break; 2828​: case SEEK_END​: 2829​: fseek(pf\, 0\, SEEK_END); 2830​: pos = _telli64(fileno(pf)); 2831​: offset += pos; 2832​: break; 2833​: case SEEK_SET​: 2834​: break; 2835​: default​: 2836​: errno = EINVAL; 00000001400CF31D call qword ptr [__imp__errno (1400D2470h)] 00000001400CF323 mov dword ptr [rax]\,16h 2837​: return -1; 00000001400CF329 jmp win32_fseek+5Eh (1400CF35Eh) 2813​: return ftell(pf); 2814​: #endif 2815​: } 2816​: 2817​: DllExport int 2818​: win32_fseek(FILE *pf\, Off_t offset\,int origin) 2819​: { 2820​: #if defined(WIN64) || defined(USE_LARGE_FILES) 2821​: fpos_t pos; 2822​: switch (origin) { 2823​: case SEEK_CUR​: 2824​: if (fgetpos(pf\, &pos)) 2825​: return -1; 2826​: offset += pos; 2827​: break; 2828​: case SEEK_END​: 2829​: fseek(pf\, 0\, SEEK_END); 00000001400CF32B xor edx\,edx 00000001400CF32D lea r8d\,[rdx+2] 00000001400CF331 call qword ptr [__imp_fseek (1400D2518h)] 2830​: pos = _telli64(fileno(pf)); 00000001400CF337 mov rcx\,rbx 00000001400CF33A call qword ptr [__imp_fileno (1400D2568h)] 00000001400CF340 mov ecx\,eax 00000001400CF342 call qword ptr [__imp__telli64 (1400D24C8h)] 00000001400CF348 mov qword ptr [pos]\,rax 2831​: offset += pos; 2832​: break; 00000001400CF34D jmp win32_fseek+67h (1400CF367h) 2823​: case SEEK_CUR​: 2824​: if (fgetpos(pf\, &pos)) 00000001400CF34F lea rdx\,[pos] ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;now do & on it\, upper 4 are uninit 00000001400CF354 call qword ptr [__imp_fgetpos (1400D2450h)] 00000001400CF35A test eax\,eax 00000001400CF35C je win32_fseek+63h (1400CF363h) 2825​: return -1; 00000001400CF35E or eax\,0FFFFFFFFh 00000001400CF361 jmp win32_fseek+79h (1400CF379h) 2826​: offset += pos; 00000001400CF363 mov eax\,dword ptr [pos] 00000001400CF367 add dword ptr [offset]\,eax 2838​: } 2839​: return fsetpos(pf\, &offset); 00000001400CF36B lea rdx\,[offset] 00000001400CF370 mov rcx\,rbx 00000001400CF373 call qword ptr [__imp_fsetpos (1400D2488h)] 2840​: #else 2841​: return fseek(pf\, (long)offset\, origin); 2842​: #endif 2843​: } 00000001400CF379 add rsp\,20h 00000001400CF37D pop rbx 00000001400CF37E ret ---------------------------------------------------------

Sending win32_fseek through preprocessor shows everything is correct. Below is original win32_fseek --------------------------------------------------------- DllExport int win32_fseek(FILE *pf\, Off_t offset\,int origin) { #if defined(WIN64) || defined(USE_LARGE_FILES) fpos_t pos; switch (origin) { case SEEK_CUR​: if (fgetpos(pf\, &pos)) return -1; offset += pos; break; case SEEK_END​: fseek(pf\, 0\, SEEK_END); pos = _telli64(fileno(pf)); offset += pos; break; case SEEK_SET​: break; default​: errno = EINVAL; return -1; } return fsetpos(pf\, &offset); #else return fseek(pf\, (long)offset\, origin); #endif } ---------------------------------------------------------

after preprocessor\, as win32 miniperl --------------------------------------------------------- int win32_fseek(FILE * pf\, __int64 offset\, int origin) {

fpos_t pos; switch (origin) { case 1​: if (fgetpos(pf\, &pos)) return -1; offset += pos; break; case 2​: fseek(pf\, 0\, 2); pos = _telli64(fileno(pf)); offset += pos; break; case 0​: break; default​: (*_errno()) = 22; return -1; } return fsetpos(pf\, &offset);

#line 2843 "win32.c" } ---------------------------------------------------------

In the attached asm output from Visual C (NOT the asm from the final executable binary)\, also shows NO PROBLEM and the correct kind of mov op (rdx). I have no idea where the edx reg came from instead of rdx reg. The perlbug data below is irrelevant to this broken perl. The C code is correct\, the compiler is wrong\, right?

Perl Info ``` Flags: category=core severity=low Site configuration information for perl 5.19.10: Configured by Owner at Sat Mar 15 22:30:42 2014. Summary of my perl5 (revision 5 version 19 subversion 10) configuration: Derived from: 2179658b5e799a6e3c4e736ec7c84b0f50bf3473 Ancestor: e9251c1a8f4944e6dceff5240d9e109ba075ff29 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 -G7 -GL -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 -G7 -GL', cppflags='-DWIN32' ccversion='13.10.6030', gccversion='', gccosandvers='' intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=1234 d_longlong=undef, longlongsize=8, d_longdbl=define, longdblsize=8 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 -ltcg -libpath:"c:\perl519\lib\CORE" -machine:x86' libpth="C:\Program Files\Microsoft Visual Studio .NET 2003\VC7\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=perl519.lib gnulibc_version='' Dynamic Linking: dlsrc=dl_win32.xs, dlext=dll, d_dlsymun=undef, ccdlflags=' ' cccdlflags=' ', lddlflags='-dll -nologo -nodefaultlib -debug -opt:ref,icf -ltcg -libpath:"c:\perl519\lib\CORE" -machine:x86' Locally applied patches: uncommitted-changes 2179658b5e799a6e3c4e736ec7c84b0f50bf3473 @INC for perl 5.19.10: C:/perl519/site/lib C:/perl519/lib . Environment for perl 5.19.10: HOME (unset) LANG (unset) LANGUAGE (unset) LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=C:\perl519\bin;C:\Program Files\Microsoft Visual Studio .NET 2003\Common7\IDE;C:\Program Files\Microsoft Visual Studio .NET 2003\VC7\BIN;C:\Program Files\Microsoft Visual Studio .NET 2003\Common7\Tools;C:\Program Files\Microsoft Visual Studio .NET 2003\Common7\Tools\bin\prerelease;C:\WINDOWS\system32;C:\WINDOWS;C:\WINDOWS\system32\wbem; PERL_BADLANG (unset) SHELL (unset) ```
p5pRT commented 10 years ago

From @bulk88

; COMDAT pdata pdata SEGMENT $pdata$win32_fseek DD imagerel $LN12   DD imagerel $LN12+130   DD imagerel $unwind$win32_fseek pdata ENDS ; COMDAT xdata xdata SEGMENT $unwind$win32_fseek DD 020a01H   DD 03006320aH ; Function compile flags​: /Ogspy xdata ENDS ; COMDAT win32_fseek _TEXT SEGMENT pf$ = 48 offset$ = 56 origin$ = 64 pos$ = 72 win32_fseek PROC ; COMDAT

; 2819 : {

$LN12​:   mov QWORD PTR [rsp+16]\, rdx;;;;;;;;;;;;;;;;;;;;;;;;;THIS IS CORRECT\, 8 byte copy   push rbx   sub rsp\, 32 ; 00000020H   mov rbx\, rcx

; 2820 : #if defined(WIN64) || defined(USE_LARGE_FILES) ; 2821 : fpos_t pos; ; 2822 : switch (origin) {

  test r8d\, r8d   je SHORT $LN2@​win32_fsee   sub r8d\, 1   je SHORT $LN5@​win32_fsee   cmp r8d\, 1   je SHORT $LN3@​win32_fsee

; 2833 : case SEEK_SET​: ; 2834 : break; ; 2835 : default​: ; 2836 : errno = EINVAL;

  call QWORD PTR __imp__errno   mov DWORD PTR [rax]\, 22

; 2837 : return -1;

  jmp SHORT $LN11@​win32_fsee $LN3@​win32_fsee​:

; 2827 : break; ; 2828 : case SEEK_END​: ; 2829 : fseek(pf\, 0\, SEEK_END);

  xor edx\, edx   lea r8d\, QWORD PTR [rdx+2]   call QWORD PTR __imp_fseek

; 2830 : pos = _telli64(fileno(pf));

  mov rcx\, rbx   call QWORD PTR __imp_fileno   mov ecx\, eax   call QWORD PTR __imp__telli64   mov QWORD PTR pos$[rsp]\, rax

; 2831 : offset += pos; ; 2832 : break;

  jmp SHORT $LN10@​win32_fsee $LN5@​win32_fsee​:

; 2823 : case SEEK_CUR​: ; 2824 : if (fgetpos(pf\, &pos))

  lea rdx\, QWORD PTR pos$[rsp]   call QWORD PTR __imp_fgetpos   test eax\, eax   je SHORT $LN4@​win32_fsee $LN11@​win32_fsee​:

; 2825 : return -1;

  or eax\, -1   jmp SHORT $LN8@​win32_fsee $LN4@​win32_fsee​:

; 2826 : offset += pos;

  mov rax\, QWORD PTR pos$[rsp] $LN10@​win32_fsee​:   add QWORD PTR offset$[rsp]\, rax $LN2@​win32_fsee​:

; 2838 : } ; 2839 : return fsetpos(pf\, &offset);

  lea rdx\, QWORD PTR offset$[rsp]   mov rcx\, rbx   call QWORD PTR __imp_fsetpos $LN8@​win32_fsee​:

; 2840 : #else ; 2841 : return fseek(pf\, (long)offset\, origin); ; 2842 : #endif ; 2843 : }

  add rsp\, 32 ; 00000020H   pop rbx   ret 0 win32_fseek ENDP _TEXT ENDS PUBLIC win32_fgetpos ; Function compile flags​: /Ogspy

p5pRT commented 10 years ago

From @steve-m-hay

I wish you had mentioned in the subject that this affects the creation of wperl.exe!

Lost in the detail of your report you mention the win32/bin/exetype.pl command-line\, but I hadn't clocked that it was what built wperl.exe.

I've just stumbled across this bug when testing a 64-bit build of blead and found (when trying to copy the installation somewhere and wondering why it was taking so long!) that my wperl.exe is 4GB. Then I recalled seeing your report recently...

This seems like a serious bug to me -- surely bad news for anyone that uses wperl.exe in a 64-bit build. I wonder if this should be marked a blocker for 5.20 since (after a quick test to double-check) 5.18.2 does not have this problem?

p5pRT commented 10 years ago

From [Unknown Contact. See original ticket]

I wish you had mentioned in the subject that this affects the creation of wperl.exe!

Lost in the detail of your report you mention the win32/bin/exetype.pl command-line\, but I hadn't clocked that it was what built wperl.exe.

I've just stumbled across this bug when testing a 64-bit build of blead and found (when trying to copy the installation somewhere and wondering why it was taking so long!) that my wperl.exe is 4GB. Then I recalled seeing your report recently...

This seems like a serious bug to me -- surely bad news for anyone that uses wperl.exe in a 64-bit build. I wonder if this should be marked a blocker for 5.20 since (after a quick test to double-check) 5.18.2 does not have this problem?

p5pRT commented 10 years ago

From @steve-m-hay

On 28 March 2014 23​:30\, Steve Hay via RT \perlbug\-comment@​perl\.org wrote​:

I wish you had mentioned in the subject that this affects the creation of wperl.exe!

Lost in the detail of your report you mention the win32/bin/exetype.pl command-line\, but I hadn't clocked that it was what built wperl.exe.

I've just stumbled across this bug when testing a 64-bit build of blead and found (when trying to copy the installation somewhere and wondering why it was taking so long!) that my wperl.exe is 4GB. Then I recalled seeing your report recently...

This seems like a serious bug to me -- surely bad news for anyone that uses wperl.exe in a 64-bit build. I wonder if this should be marked a blocker for 5.20 since (after a quick test to double-check) 5.18.2 does not have this problem?

I've bisected this to a change made by Ruslan Zakirov and committed by Father C on 30 Jun 2013​:

This commit is ok​: 36925d9e116d2779c960f8cac847f9ccd4c43e53 - change magic_methcall to use SV with shared hash value

but the very next commit breaks the 64-bit build on Windows\, causing wperl.exe to end up being 4GB​: 3e0cb5de60bed90602a75e9d726f90b2e60701e0 - change tied_method to use SVs with precomputed hash values

I do not see anything obviously wrong with the change or why it has this effect. Any suggestions? It would be good to have this fixed for 5.20 if possible.

p5pRT commented 10 years ago

From @bulk88

On Sat Mar 29 11​:58​:21 2014\, shay wrote​:

On 28 March 2014 23​:30\, Steve Hay via RT \perlbug\-comment@​perl\.org wrote​:

I wish you had mentioned in the subject that this affects the creation of wperl.exe!

Lost in the detail of your report you mention the win32/bin/exetype.pl command-line\, but I hadn't clocked that it was what built wperl.exe.

I've just stumbled across this bug when testing a 64-bit build of blead and found (when trying to copy the installation somewhere and wondering why it was taking so long!) that my wperl.exe is 4GB. Then I recalled seeing your report recently...

This seems like a serious bug to me -- surely bad news for anyone that uses wperl.exe in a 64-bit build. I wonder if this should be marked a blocker for 5.20 since (after a quick test to double-check) 5.18.2 does not have this problem?

IDK about it being a blocker\, since I have no estimate of time to fix this by me. I'm a bit busy atm. I think its a compiler bug since the preprocessor C is fine (feel free to verify on your side the .i file\, that "Off_t offset" really is a 8 byte type according to the rules of C and I'm not crazy)\, and the asm (not machine) code outputed shows a 8 byte mov\, but C compiler bug is a heavy accusation to make. It might be related to the -GL flag\, since VC will not produce an asm file if you pass -GL (WPO/IPO/LTO). And when you remove -GL\, the asm code is fine.

I've bisected this to a change made by Ruslan Zakirov and committed by Father C on 30 Jun 2013​:

This commit is ok​: 36925d9e116d2779c960f8cac847f9ccd4c43e53 - change magic_methcall to use SV with shared hash value

but the very next commit breaks the 64-bit build on Windows\, causing wperl.exe to end up being 4GB​: 3e0cb5de60bed90602a75e9d726f90b2e60701e0 - change tied_method to use SVs with precomputed hash values

I do not see anything obviously wrong with the change or why it has this effect. Any suggestions? It would be good to have this fixed for 5.20 if possible.

It is an uninit memory bug. If the C stack happens to have 0x00000000 on it\, in the area which later becomes "Off_t offset"\, no failure will be observed. Looking at the machine code from the binary is the only way to figure it out (is the source for the x64 mov rdx or edx?)\, or put into the runloop something like

void * getStack(int param) { return (void *)¶m; }

// in runloop before each pp_

void * freeCstack = getStack(param); memset(freeCstack-4096\, 0xFF\, 4096);

Looking at ActivePerl Build 1402 [295342] 5.14.2 x64\, win32_fseek uses rdx (correct).


mov [rsp+arg_8]\, rdx push rbx sub rsp\, 20h test r8d\, r8d mov rbx\, rcx


My VC 2008 x64 -Od perl519.dll\, win32_fseek uses rdx (correct).


mov [rsp+arg_10]\, r8d mov [rsp+arg_8]\, rdx mov [rsp+arg_0]\, rcx sub rsp\, 38h mov eax\, [rsp+38h+arg_10] mov [rsp+38h+var_10]\, eax cmp [rsp+38h+var_10]\, 0


Same build as above\, miniperl.exe has


mov [rsp+arg_10]\, r8d mov [rsp+arg_8]\, edx ;BADDDDDDDDDDDDD mov [rsp+arg_0]\, rcx sub rsp\, 38h mov eax\, [rsp+38h+arg_10] mov [rsp+38h+var_10]\, eax cmp [rsp+38h+var_10]\, 0


In addition to -GL hypothesis\, lack of win32_fseek being PE exported is another thing that can be affecting this. Exporting a C function or using it as a fnc ptr on x86-32 with VC kills the random calling convention optimization of -GL or "static". Then again\, with VC x64\, I've never ever seen the random calling convention feature since that would probably (not 100% sure) break MS's highly limited SEH on x64 exception unwind table syntax. miniperl.exe rightfully has no exports nowadays after some optimizations I did a long time ago http​://perl5.git.perl.org/perl.git/commitdiff/a19baa613c9769701790afd2f9ebfc422a1e97e7 .

-- bulk88 ~ bulk88 at hotmail.com

p5pRT commented 10 years ago

From @steve-m-hay

On 29 March 2014 22​:13\, bulk88 via RT \perlbug\-followup@​perl\.org wrote​:

On Sat Mar 29 11​:58​:21 2014\, shay wrote​:

On 28 March 2014 23​:30\, Steve Hay via RT \perlbug\-comment@​perl\.org wrote​:

I wish you had mentioned in the subject that this affects the creation of wperl.exe!

Lost in the detail of your report you mention the win32/bin/exetype.pl command-line\, but I hadn't clocked that it was what built wperl.exe.

I've just stumbled across this bug when testing a 64-bit build of blead and found (when trying to copy the installation somewhere and wondering why it was taking so long!) that my wperl.exe is 4GB. Then I recalled seeing your report recently...

This seems like a serious bug to me -- surely bad news for anyone that uses wperl.exe in a 64-bit build. I wonder if this should be marked a blocker for 5.20 since (after a quick test to double-check) 5.18.2 does not have this problem?

IDK about it being a blocker\, since I have no estimate of time to fix this by me. I'm a bit busy atm. I think its a compiler bug since the preprocessor C is fine (feel free to verify on your side the .i file\, that "Off_t offset" really is a 8 byte type according to the rules of C and I'm not crazy)\, and the asm (not machine) code outputed shows a 8 byte mov\, but C compiler bug is a heavy accusation to make. It might be related to the -GL flag\, since VC will not produce an asm file if you pass -GL (WPO/IPO/LTO). And when you remove -GL\, the asm code is fine.

I don't think whether any individual has time to fix it should have any bearing on whether it's a blocker\, although a blocker that nobody can fix would be a problem!... ;-)

Fotunately\, I've now fixed it anyway after staring at things for longer and reading and then re-reading what you've written...

[...]

My VC 2008 x64 -Od perl519.dll\, win32_fseek uses rdx (correct). ------------------------------- mov [rsp+arg_10]\, r8d mov [rsp+arg_8]\, rdx mov [rsp+arg_0]\, rcx sub rsp\, 38h mov eax\, [rsp+38h+arg_10] mov [rsp+38h+var_10]\, eax cmp [rsp+38h+var_10]\, 0 -------------------------------

Same build as above\, miniperl.exe has

---------------------------------------- mov [rsp+arg_10]\, r8d mov [rsp+arg_8]\, edx ;BADDDDDDDDDDDDD mov [rsp+arg_0]\, rcx sub rsp\, 38h mov eax\, [rsp+38h+arg_10] mov [rsp+38h+var_10]\, eax cmp [rsp+38h+var_10]\, 0 ---------------------------------------

I see the same thing too (using dumpbin /disasm on perl519.dll and miniperl.exe).

I then tried to generate the asm output from the compiler which you attached in your original report. I failed to get the /FAs switch working to get this output (is that what you used? I copied and pasted the compiler command-line from the build process\, added /FAs\, and ran it\, expecting to get a .asm file as described at http​://msdn.microsoft.com/en-us/library/367y26c6.aspx but it didn't generate any such file for me)\, but in the course of trying this -- with two different command-lines (one from the miniperl.exe build and one from the perl519.dll build) -- the penny dropped what the problem was​:

When building miniperl.exe we use a canned configuration file copied from win32/config_H.vc\, which win32/Makefile then appends some stuff to the end of to fix up 64-bit build options so that miniperl.exe builds and its -V outputs the correct information (see http​://perl5.git.perl.org/perl.git/commit/1f64ae1564).

But by the time we build perl519.dll we have rewritten config.h using config_h.SH\, which uses information from config.sh\, written out by win32/config_sh.PL.

The latter contains some tweaks for large file options (the USE_LARGE_FILES option in win32/Makefile)\, which set the $lseeksize and $lseektype used in config_h.SH to set Off_t\, LSEEKSIZE and Off_t_size\, but the win32/Makefile magic added by the commit cited above fails to manipulate Off_t\, LSEEKSIZE and Off_t_size at all.

Thus\, when building miniperl.exe Off_t is actually the canned config.h value\, which is 'long'\, hence the 4-byte mov in the assembly; but when building perl519.dll Off_t is correctly set to '__int64'\, hence the 8-byte mov then (assuming USE_LARGE_FILES is defined\, which it is by default). This seems not to be a problem (or at least not one that's bitten yet!) in the 32-bit build\, but does screw up in the 64-bit build.

The attached patch adds the correct setting of Off_t et al to win32/Makefile (and win32/makefile.mk) so that win32_fseek() now works in miniperl.exe :-)

As for whether this is a blocker... It could be argued not since the bug is far from new (it's probably always been this way? prior to the change I cited above we had separate canned config files for 64-bit builds\, but even they had Off_t set to 'long'; and we've never had separate files for USE_LARGE_FILES builds\, so the bug has presumably existed since USE_LARGE_FILES support was added) -- and yet it's odd that we're only seeing the problem actually bite in fairly recent times as my earlier bisect showed.

I would like to see this patch go in for 5.20\, but it's Ricardo's call.

p5pRT commented 10 years ago

From @steve-m-hay

Off_t.patch ```diff diff --git a/win32/Makefile b/win32/Makefile index 5991a00..9a6df99 100644 --- a/win32/Makefile +++ b/win32/Makefile @@ -887,6 +887,9 @@ config.w32 : $(CFGSH_TMPL) @echo.>>$@ @echo #ifndef _config_h_footer_>>$@ @echo #define _config_h_footer_>>$@ + @echo #undef Off_t>>$@ + @echo #undef LSEEKSIZE>>$@ + @echo #undef Off_t_size>>$@ @echo #undef PTRSIZE>>$@ @echo #undef SSize_t>>$@ @echo #undef HAS_ATOLL>>$@ @@ -905,6 +908,15 @@ config.w32 : $(CFGSH_TMPL) @echo #undef UVXf>>$@ @echo #undef USE_64_BIT_INT>>$@ @echo #undef Size_t_size>>$@ +!IF "$(USE_LARGE_FILES)"=="define" + @echo #define Off_t __int64>>$@ + @echo #define LSEEKSIZE ^8>>$@ + @echo #define Off_t_size ^8>>$@ +!ELSE + @echo #define Off_t long>>$@ + @echo #define LSEEKSIZE ^4>>$@ + @echo #define Off_t_size ^4>>$@ +!ENDIF !IF "$(WIN64)"=="define" @echo #define PTRSIZE ^8>>$@ @echo #define SSize_t __int64>>$@ diff --git a/win32/makefile.mk b/win32/makefile.mk index 269f4b7..0c89340 100644 --- a/win32/makefile.mk +++ b/win32/makefile.mk @@ -1015,6 +1015,9 @@ config.w32 : $(CFGSH_TMPL) @echo.>>$@ @echo #ifndef _config_h_footer_>>$@ @echo #define _config_h_footer_>>$@ + @echo #undef Off_t>>$@ + @echo #undef LSEEKSIZE>>$@ + @echo #undef Off_t_size>>$@ @echo #undef PTRSIZE>>$@ @echo #undef SSize_t>>$@ @echo #undef HAS_ATOLL>>$@ @@ -1033,6 +1036,15 @@ config.w32 : $(CFGSH_TMPL) @echo #undef UVXf>>$@ @echo #undef USE_64_BIT_INT>>$@ @echo #undef Size_t_size>>$@ +.IF "$(USE_LARGE_FILES)"=="define" + @echo #define Off $(INT64)>>$@ + @echo #define LSEEKSIZE ^8>>$@ + @echo #define Off_t_size ^8>>$@ +.ELSE + @echo #define Off long>>$@ + @echo #define LSEEKSIZE ^4>>$@ + @echo #define Off_t_size ^4>>$@ +.ENDIF .IF "$(WIN64)"=="define" @echo #define PTRSIZE ^8>>$@ @echo #define SSize_t $(INT64)>>$@ ```
p5pRT commented 10 years ago

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

p5pRT commented 10 years ago

From @bulk88

On Sat Mar 29 17​:37​:38 2014\, shay wrote​:

I then tried to generate the asm output from the compiler which you attached in your original report. I failed to get the /FAs switch working to get this output (is that what you used? I copied and pasted the compiler command-line from the build process\, added /FAs\, and ran it\, expecting to get a .asm file as described at http​://msdn.microsoft.com/en-us/library/367y26c6.aspx but it didn't generate any such file for me)\, but in the course of trying this -- with two different command-lines (one from the miniperl.exe build and one from the perl519.dll build) -- the penny dropped what the problem was​:

Remove -GL from the command line. If you use /FAs and -GL\, no assembly file is made. Probably because with -GL the .objs dont contain machine code but instead MS internal bytecode\, so there is no assembly code to write to a file\, so no file is written. MS needs to add a better error.

When building miniperl.exe we use a canned configuration file copied from win32/config_H.vc\, which win32/Makefile then appends some stuff to the end of to fix up 64-bit build options so that miniperl.exe builds and its -V outputs the correct information (see http​://perl5.git.perl.org/perl.git/commit/1f64ae1564).

That commit is 5.17.3 but also Off_t is "long" in old config_H.vc64 file. I'm surprised AS never reported it.

But by the time we build perl519.dll we have rewritten config.h using config_h.SH\, which uses information from config.sh\, written out by win32/config_sh.PL.

Maybe the rewritten config.h is why my .i files showed 8 byte types even though I passed -DPERL_IS_MINIPERL . The config.h wasn't the one used to compile miniperl anymore. This does raise another bug problem\, if you delete win32/mini\, then "nmake ../miniperl.exe"\, it will be partially a full perl then inside that miniperl.

The latter contains some tweaks for large file options (the USE_LARGE_FILES option in win32/Makefile)\, which set the $lseeksize and $lseektype used in config_h.SH to set Off_t\, LSEEKSIZE and Off_t_size\, but the win32/Makefile magic added by the commit cited above fails to manipulate Off_t\, LSEEKSIZE and Off_t_size at all.

Thus\, when building miniperl.exe Off_t is actually the canned config.h value\, which is 'long'\, hence the 4-byte mov in the assembly; but when building perl519.dll Off_t is correctly set to '__int64'\, hence the 8-byte mov then (assuming USE_LARGE_FILES is defined\, which it is by default). This seems not to be a problem (or at least not one that's bitten yet!) in the 32-bit build\, but does screw up in the 64-bit build.

The attached patch adds the correct setting of Off_t et al to win32/Makefile (and win32/makefile.mk) so that win32_fseek() now works in miniperl.exe :-)

As for whether this is a blocker... It could be argued not since the bug is far from new (it's probably always been this way? prior to the change I cited above we had separate canned config files for 64-bit builds\, but even they had Off_t set to 'long'; and we've never had separate files for USE_LARGE_FILES builds\, so the bug has presumably existed since USE_LARGE_FILES support was added) -- and yet it's odd that we're only seeing the problem actually bite in fairly recent times as my earlier bisect showed.

Uninit memory\, if I really wanted to I'd step the pp opcodes until I figured out the C statment that did it but that wouldn't help in solving this bug. Plus the sea of 64 bit trunction warnings that a VC win64 build has that nobody (me?) pays attention to contributed to this bug.

I would like to see this patch go in for 5.20\, but it's Ricardo's call.

Building perl and getting a 4GB file of nulls written out to your disk isnt a friendly thing to do. It should go in. I haven't personally tested this patch.

-- bulk88 ~ bulk88 at hotmail.com

p5pRT commented 10 years ago

From @steve-m-hay

On 30 Mar 2014 04​:23\, "bulk88 via RT" \perlbug\-followup@​perl\.org wrote​:

On Sat Mar 29 17​:37​:38 2014\, shay wrote​:

I then tried to generate the asm output from the compiler which you attached in your original report. I failed to get the /FAs switch working to get this output (is that what you used? I copied and pasted the compiler command-line from the build process\, added /FAs\, and ran it\, expecting to get a .asm file as described at http​://msdn.microsoft.com/en-us/library/367y26c6.aspx but it didn't generate any such file for me)\, but in the course of trying this -- with two different command-lines (one from the miniperl.exe build and one from the perl519.dll build) -- the penny dropped what the problem was​:

Remove -GL from the command line. If you use /FAs and -GL\, no assembly file is made. Probably because with -GL the .objs dont contain machine code but instead MS internal bytecode\, so there is no assembly code to write to a file\, so no file is written. MS needs to add a better error.

Thanks\, I will try this later.

When building miniperl.exe we use a canned configuration file copied from win32/config_H.vc\, which win32/Makefile then appends some stuff to the end of to fix up 64-bit build options so that miniperl.exe builds and its -V outputs the correct information (see http​://perl5.git.perl.org/perl.git/commit/1f64ae1564).

That commit is 5.17.3 but also Off_t is "long" in old config_H.vc64 file. I'm surprised AS never reported it.

Indeed. If the old config_H.vc64 file had had Off_t being '__int64' then I'm sure I'd have accounted for it in that commit​: it was essentially a no-op as far as the end result for existing build configurations was concerned. All it did was add a new config option and remove some big files from the distro\, saving maintenance work on them in the process.

But by the time we build perl519.dll we have rewritten config.h using config_h.SH\, which uses information from config.sh\, written out by win32/config_sh.PL.

Maybe the rewritten config.h is why my .i files showed 8 byte types even though I passed -DPERL_IS_MINIPERL . The config.h wasn't the one used to compile miniperl anymore. This does raise another bug problem\, if you delete win32/mini\, then "nmake ../miniperl.exe"\, it will be partially a full perl then inside that miniperl.

Yes\, you should really delete config.h too if you delete mini/

Not a problem that will affect end-users\, though.

The latter contains some tweaks for large file options (the USE_LARGE_FILES option in win32/Makefile)\, which set the $lseeksize and $lseektype used in config_h.SH to set Off_t\, LSEEKSIZE and Off_t_size\, but the win32/Makefile magic added by the commit cited above fails to manipulate Off_t\, LSEEKSIZE and Off_t_size at all.

Thus\, when building miniperl.exe Off_t is actually the canned config.h value\, which is 'long'\, hence the 4-byte mov in the assembly; but when building perl519.dll Off_t is correctly set to '__int64'\, hence the 8-byte mov then (assuming USE_LARGE_FILES is defined\, which it is by default). This seems not to be a problem (or at least not one that's bitten yet!) in the 32-bit build\, but does screw up in the 64-bit build.

The attached patch adds the correct setting of Off_t et al to win32/Makefile (and win32/makefile.mk) so that win32_fseek() now works in miniperl.exe :-)

As for whether this is a blocker... It could be argued not since the bug is far from new (it's probably always been this way? prior to the change I cited above we had separate canned config files for 64-bit builds\, but even they had Off_t set to 'long'; and we've never had separate files for USE_LARGE_FILES builds\, so the bug has presumably existed since USE_LARGE_FILES support was added) -- and yet it's odd that we're only seeing the problem actually bite in fairly recent times as my earlier bisect showed.

Uninit memory\, if I really wanted to I'd step the pp opcodes until I figured out the C statment that did it but that wouldn't help in solving this bug. Plus the sea of 64 bit trunction warnings that a VC win64 build has that nobody (me?) pays attention to contributed to this bug.

I've had fixing them on my TODO list for too long... :-/ I will try harder to find time to go through them when 5.21.0 is out.

I would like to see this patch go in for 5.20\, but it's Ricardo's call.

Building perl and getting a 4GB file of nulls written out to your disk isnt a friendly thing to do. It should go in. I haven't personally tested this patch.

I agree. Even if wperl.exe isn't widely used (and the fact that it's completely borked in the ever more popular 64-bit build and nobody has complained suggests maybe it isn't\, although I've yet to see the problem surface in a 'stable' perl\, i.e. perl 5.18) it's still a nasty thing to have happen every time you do a 64-bit build.

I think it should go in.

Ricardo?

p5pRT commented 10 years ago

From @steve-m-hay

On Sat Mar 29 20​:23​:26 2014\, bulk88 wrote​:

On Sat Mar 29 17​:37​:38 2014\, shay wrote​:

I would like to see this patch go in for 5.20\, but it's Ricardo's call.

Building perl and getting a 4GB file of nulls written out to your disk isnt a friendly thing to do. It should go in. I haven't personally tested this patch.

Do you have time to test the patch? It would be good to gets a head-sup from the OP that it fixes the problem.

p5pRT commented 10 years ago

From @bulk88

On Sun Apr 06 11​:34​:38 2014\, shay wrote​:

On Sat Mar 29 20​:23​:26 2014\, bulk88 wrote​:

On Sat Mar 29 17​:37​:38 2014\, shay wrote​:

I would like to see this patch go in for 5.20\, but it's Ricardo's call.

Building perl and getting a 4GB file of nulls written out to your disk isnt a friendly thing to do. It should go in. I haven't personally tested this patch.

Do you have time to test the patch? It would be good to gets a head- sup from the OP that it fixes the problem.

Its fixed. rdx used in miniperl asm. wperl.exe is normal size\, not 4 gb. Next time can you post a git patch instead of a diff patch? Git patches are more convenient for me than diff patches.

-- bulk88 ~ bulk88 at hotmail.com

p5pRT commented 10 years ago

From @steve-m-hay

On Sun Apr 06 15​:57​:18 2014\, bulk88 wrote​:

On Sun Apr 06 11​:34​:38 2014\, shay wrote​:

On Sat Mar 29 20​:23​:26 2014\, bulk88 wrote​:

On Sat Mar 29 17​:37​:38 2014\, shay wrote​:

I would like to see this patch go in for 5.20\, but it's Ricardo's call.

Building perl and getting a 4GB file of nulls written out to your disk isnt a friendly thing to do. It should go in. I haven't personally tested this patch.

Do you have time to test the patch? It would be good to gets a head- sup from the OP that it fixes the problem.

Its fixed. rdx used in miniperl asm. wperl.exe is normal size\, not 4 gb. Next time can you post a git patch instead of a diff patch? Git patches are more convenient for me than diff patches.

Thanks for reporting/analyzing this problem and testing the fix\, which is now applied in commit 200b4fd964.

p5pRT commented 10 years ago

@steve-m-hay - Status changed from 'open' to 'resolved'