Repository navigation
uninit mem read in win32_fseek() randomly creates multi GB files on Win64 #13677
Description
Activity
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)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: /OgspyFrom @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?
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?
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:
36925d9 - change magic_methcall to
use SV with shared hash valuebut the very next commit breaks the 64-bit build on Windows, causing
wperl.exe to end up being 4GB:
3e0cb5d - change tied_method to use
SVs with precomputed hash valuesI 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.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:
36925d9 - change magic_methcall to
use SV with shared hash valuebut the very next commit breaks the 64-bit build on Windows, causing
wperl.exe to end up being 4GB:
3e0cb5d - change tied_method to use
SVs with precomputed hash valuesI 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.comFrom @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.
From @steve-m-hay
Off_t.patch
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)>>$@
The RT System itself - Status changed from 'new' to 'open'
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.comFrom @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?
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.
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.comFrom @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 200b4fd.
@steve-m-hay - Status changed from 'open' to 'resolved'
Migrated from rt.perl.org#121471 (status was 'resolved')
Searchable as RT121471$