On Tue, Sep 4, 2012 at 8:21 AM, Andy Dougherty <[email protected]> wrote: > 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); > } > }
Yes, I remember reading Nick's earlier analysis, but I was away for 3 weeks. Thanks for the reminder, and Andy, yes, this was it. Confirmed and applied as 0cc54e6972ae830e7943a15c8db1e14fc9c3cb90 -- Reini Urban http://cpanel.net/ http://www.perl-compiler.org/ _______________________________________________ http://lists.parrot.org/mailman/listinfo/parrot-dev
