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

Reini Urban rurban at x-ray.at
Tue Sep 4 17:49:34 UTC 2012


On Tue, Sep 4, 2012 at 8:21 AM, Andy Dougherty <doughera at lafayette.edu> 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/


More information about the parrot-dev mailing list