threads branch: t/pmc/filehandle.t: test results differ between gcc and g++ builds

Andy Dougherty doughera at lafayette.edu
Tue Sep 4 13:21:47 UTC 2012


On Tue, 4 Sep 2012, Nicholas Clark wrote:

> On Mon, Sep 03, 2012 at 09:41:28PM -0400, James E Keenan wrote:
> 
> > (These results were run at commit 5709835825; Sept 02 2012.  But I've 
> > submitted dozens of smolders with the same results in recent months.)
> > 
> > In my experience, it's really rare that we get consistently different 
> > test results between all-gcc and all-g++ builds on our most heavily 
> > tested platform.
> > 
> > Any thoughts as to the cause of this discrepancy?
> 
> >From the output of strace and equivalent I'd deduced that the bug is a
> failure to sign extend -10 from a 32 bit value to a 64 bit value. I thought
> I'd already said this.
> 
> Although the inconsistency between C++ and C building the same source code
> is curious. I've no idea if this is because Parrot (or Perl's) Configure
> picks up different types for the two languages even on the same system, or
> if it's because of different library headers between the two, or some
> subtly of difference in the type system between the two (Perl 5 had odd
> bugs due to using a real C++ bool for C++, and emulating it with a char for
> C), or something else.
> 
> But look for a conversion from a 32 bit signed type to a 32 bit unsigned
> type, that is then passed to a seek library call that actually expects a
> 64 bit signed type.

Fortunately, seek() isn't called all that often, and there's one one 
obvious likely spot that does precisely that.

Does this patch fix it?  I haven't tested this it, but it's exactly the 
sort of thing that would give rise to sign extension errors, and, if I 
recall correctly, the precise rules for sign extension are slightly 
different in c and c++, which would explain your discrepancy.

diff --git a/src/io/utilities.c b/src/io/utilities.c
index 08ed390..0acd4e9 100644
--- a/src/io/utilities.c
+++ b/src/io/utilities.c
@@ -551,7 +551,10 @@ io_sync_buffers_for_write(PARROT_INTERP, ARGMOD(PMC *handle),
     if (read_buffer && !BUFFER_IS_EMPTY(read_buffer)) {
         const size_t buffer_size = BUFFER_USED_SIZE(read_buffer);
         Parrot_io_buffer_clear(interp, read_buffer);
-        vtable->seek(interp, handle, -buffer_size, SEEK_CUR);
+	/* To avoid sign extension problems on 32-bit systems with 64-bit
+           file support, first cast the buffer_size to off_t, then change
+           the sign. */
+        vtable->seek(interp, handle, -((PIOOFF_T)buffer_size), SEEK_CUR);
     }
 }
 


-- 
    Andy Dougherty		doughera at lafayette.edu


More information about the parrot-dev mailing list