[rt.cpan.org #125503] [PATCH] Fix build with 64-bit perl-5.28.0
Tue Jun 12 15:09:54 2018: Request 125503 was acted upon. Transaction: Correspondence added by RSCHUPP Queue: PAR-Packer Subject: [PATCH] Fix build with 64-bit perl-5.28.0 Broken in: 1.043 Severity: (no value) Owner: Nobody Requestors: s...@cpan.org Status: new Ticket https://rt.cpan.org/Ticket/Display.html?id=125503 > On 2018-06-08 05:04:20, SHAY wrote: > I found another problem: Executables compiled with pp's -C argument > crashed on exit due to another 64-bit bug, in par_rmtmpdir(). The > _findfirst() function returns an intptr_t (a 32-bit integer in x86 > builds, but 64-bit in x64 builds -- to hold the larger pointers) but > it was being assigned to a long (a 32-bit integer in both x86 and x64 > builds), so it got truncated. > > I've also spotted one more use of struct stat which I think needs > changing to par_stat_t, which is also done in this updated patch. I > haven't tested this change myself since it affects only the non-WIN32 > version of par_rmtmpdir(), so please be sure to test with the -C > argument on a x64 build on some non-WIN32 platform. Merged the second half of your patch (a fix similar to your first half was already applied in 1.044) and released as 1.045. Thanks, Steve! Cheers. Roderich
[rt.cpan.org #125503] [PATCH] Fix build with 64-bit perl-5.28.0
Fri Jun 08 05:04:20 2018: Request 125503 was acted upon.
Transaction: Correspondence added by SHAY
Queue: PAR-Packer
Subject: [PATCH] Fix build with 64-bit perl-5.28.0
Broken in: 1.043
Severity: (no value)
Owner: Nobody
Requestors: s...@cpan.org
Status: new
Ticket https://rt.cpan.org/Ticket/Display.html?id=125503 >
I found another problem: Executables compiled with pp's -C argument crashed on
exit due to another 64-bit bug, in par_rmtmpdir(). The _findfirst() function
returns an intptr_t (a 32-bit integer in x86 builds, but 64-bit in x64 builds
-- to hold the larger pointers) but it was being assigned to a long (a 32-bit
integer in both x86 and x64 builds), so it got truncated.
The attached pp-x64-v2.patch is a new version of the previous patch, which now
includes this fix as well.
I've also spotted one more use of struct stat which I think needs changing to
par_stat_t, which is also done in this updated patch. I haven't tested this
change myself since it affects only the non-WIN32 version of par_rmtmpdir(), so
please be sure to test with the -C argument on a x64 build on some non-WIN32
platform.
diff -ruN --binary PAR-Packer-1.043.orig/myldr/mktmpdir.c PAR-Packer-1.043/myldr/mktmpdir.c
--- PAR-Packer-1.043.orig/myldr/mktmpdir.c 2017-02-11 17:19:06.0 +
+++ PAR-Packer-1.043/myldr/mktmpdir.c 2018-06-08 09:54:02.853109100 +0100
@@ -21,7 +21,7 @@
static int isWritableDir(const char* val)
{
#ifndef PL_statbuf
-struct stat PL_statbuf;
+par_stat_t PL_statbuf;
#endif
return par_lstat(val, &PL_statbuf) == 0 &&
@@ -38,7 +38,7 @@
static int isSafeDir(const char* val)
{
#ifndef PL_statbuf
-struct stat PL_statbuf;
+par_stat_t PL_statbuf;
#endif
return par_lstat(val, &PL_statbuf) == 0 &&
@@ -301,7 +301,7 @@
int subsub_len;
char *subsubdir;
char *slashdot;
-long hFile;
+intptr_t hFile;
int tries = 0;
HMODULE dll;
@@ -347,7 +347,7 @@
Direntry_t *dp;
char *subsubdir = NULL;
int subsub_len;
-struct stat stbuf;
+par_stat_t stbuf;
/* remove temporary PAR directory */
if (!stmpdir || !*stmpdir) return;
diff -ruN --binary PAR-Packer-1.043.orig/myldr/mktmpdir.h PAR-Packer-1.043/myldr/mktmpdir.h
--- PAR-Packer-1.043.orig/myldr/mktmpdir.h 2017-02-04 23:29:19.0 +
+++ PAR-Packer-1.043/myldr/mktmpdir.h 2018-06-08 09:53:19.082324400 +0100
@@ -67,6 +67,12 @@
#define par_lstat stat
#endif
+#ifdef Stat_t
+#define par_stat_t Stat_t
+#else
+#define par_stat_t struct stat
+#endif
+
#if defined(WIN32) || defined(OS2)
static const char *dir_sep = "\\";
static const char *path_sep = ";";
diff -ruN --binary PAR-Packer-1.043.orig/myldr/utils.c PAR-Packer-1.043/myldr/utils.c
--- PAR-Packer-1.043.orig/myldr/utils.c 2017-03-12 17:26:54.0 +
+++ PAR-Packer-1.043/myldr/utils.c 2018-06-08 09:53:19.149620900 +0100
@@ -95,7 +95,7 @@
* to a struct stat. Try to distinguish these cases by checking
* whether PL_statbuf is defined. */
#ifndef PL_statbuf
-struct stat PL_statbuf;
+par_stat_t PL_statbuf;
#endif
#ifdef WIN32
[rt.cpan.org #125503] [PATCH] Fix build with 64-bit perl-5.28.0
Wed Jun 06 09:16:51 2018: Request 125503 was acted upon.
Transaction: Ticket created by SHAY
Queue: PAR-Packer
Subject: [PATCH] Fix build with 64-bit perl-5.28.0
Broken in: 1.043
Severity: (no value)
Owner: Nobody
Requestors: s...@cpan.org
Status: new
Ticket https://rt.cpan.org/Ticket/Display.html?id=125503 >
In perl-5.28.0 the PL_statbuf interpreter variable has been removed. This
causes code in several places in PAR-Packer to define it as a "struct stat":
#ifndef PL_statbuf
struct stat PL_statbuf;
#endif
That's fine in boot.c where the code ends up in a plain C program, but not in
mktmpdir.c and utils.c when the code may end up in the custom Perl interpreter
because in that case PL_statbuf is passed to par_lstat(), which is defined as
either lstat() or stat(), which may themselves be defined as calls into PerlIO.
In my case, in an x64 build on Windows, stat() ends up calling perl's
win32_stat(), which calls _stati64(), which expects a struct _stati64 to handle
64-bit times. I found that the 64-bit times got written anyway - into the
struct stat, which only has space for 32-bit times, causing a buffer overrun
and resulting in the following crash when exiting isWritableDir(): "Unhandled
exception at 0x7FF7183F4554 in par.exe: Stack cookie instrumentation code
detected a stack-based buffer overrun."
(It works in 32-bit builds because win32_stat() calls stat() in that case, for
which struct stat is correct.)
I think the answer is the attached patch, which introduces a par_stat_t to be
used with par_lstat(), and is defined to be perl's Stat_t (which will already
be set up as a suitable struct type to match whatever flavour of stat() gets
used) if that's defined (i.e. in the customer Perl interpreter case), or struct
stat otherwise (i.e. in the plain C program case).
diff -ruN --binary PAR-Packer-1.043.orig/myldr/mktmpdir.c PAR-Packer-1.043/myldr/mktmpdir.c
--- PAR-Packer-1.043.orig/myldr/mktmpdir.c 2017-02-11 17:19:06.0 +
+++ PAR-Packer-1.043/myldr/mktmpdir.c 2018-06-06 13:03:36.410791600 +0100
@@ -21,7 +21,7 @@
static int isWritableDir(const char* val)
{
#ifndef PL_statbuf
-struct stat PL_statbuf;
+par_stat_t PL_statbuf;
#endif
return par_lstat(val, &PL_statbuf) == 0 &&
@@ -38,7 +38,7 @@
static int isSafeDir(const char* val)
{
#ifndef PL_statbuf
-struct stat PL_statbuf;
+par_stat_t PL_statbuf;
#endif
return par_lstat(val, &PL_statbuf) == 0 &&
diff -ruN --binary PAR-Packer-1.043.orig/myldr/mktmpdir.h PAR-Packer-1.043/myldr/mktmpdir.h
--- PAR-Packer-1.043.orig/myldr/mktmpdir.h 2017-02-04 23:29:19.0 +
+++ PAR-Packer-1.043/myldr/mktmpdir.h 2018-06-06 13:03:07.349713300 +0100
@@ -67,6 +67,12 @@
#define par_lstat stat
#endif
+#ifdef Stat_t
+#define par_stat_t Stat_t
+#else
+#define par_stat_t struct stat
+#endif
+
#if defined(WIN32) || defined(OS2)
static const char *dir_sep = "\\";
static const char *path_sep = ";";
diff -ruN --binary PAR-Packer-1.043.orig/myldr/utils.c PAR-Packer-1.043/myldr/utils.c
--- PAR-Packer-1.043.orig/myldr/utils.c 2017-03-12 17:26:54.0 +
+++ PAR-Packer-1.043/myldr/utils.c 2018-06-06 13:03:48.029684100 +0100
@@ -95,7 +95,7 @@
* to a struct stat. Try to distinguish these cases by checking
* whether PL_statbuf is defined. */
#ifndef PL_statbuf
-struct stat PL_statbuf;
+par_stat_t PL_statbuf;
#endif
#ifdef WIN32
