On Mon, 18 Jul 2011, Christoph Otto wrote:
> On Mon, 18 Jul 2011 14:06 -0400, "Andy Dougherty"
> <[email protected]> wrote:
> > OpenBSD 4.9/i386 failed on parrot master as follows:
> >
> > <snip>
> >
> > git bisect blames
> >
> > 1d270a0331242a9e50170697e0b5d7d4d65096f9 is the first bad commit
> > commit 1d270a0331242a9e50170697e0b5d7d4d65096f9 Author: chromatic
> > <[email protected]> Date: Tue Jul 5 17:03:38 2011 -0700
> >
> > [GC] Fixed Win32-killing type errors in d789b5.
> >
> >
> > --
> > Andy Dougherty [email protected]
>
> What happens when you revert that commit? If that lets you get the
> build working again, let us know and we can revert it, pull it into its
> own branch and investigate after the release.
> Thanks for reporting this.
Yes, reverting that commit "fixes" it, but based on the commit message, I
imagine reverting it would break Win32, so that's not a good option
either.
Staring at that patch for a while, I think I found the problem -- when
arena or ptr have their high bit set, conversion to a signed type (namely
ptrdiff_t) results in negative numbers, which causes the <= test to be
backwards. The suggested patch below avoids the sign extension errors by
only using ptrdiff_t to hold differences of pointers. As a side effect,
the conditional can be simplified.
I have only tested this on 32-bit OpenBSD with gcc.
diff --git a/src/gc/fixed_allocator.c b/src/gc/fixed_allocator.c
index 7e4883d..425c472 100644
--- a/src/gc/fixed_allocator.c
+++ b/src/gc/fixed_allocator.c
@@ -510,19 +510,15 @@ pool_is_owned(ARGMOD(Pool_Allocator *pool), ARGIN(void
*ptr))
/* We can cache these values. All arenas are same size */
const ptrdiff_t a_size = arena_size(pool);
- const ptrdiff_t ptritem = (ptrdiff_t)ptr;
const ptrdiff_t objsize = pool->object_size;
while (arena) {
- const ptrdiff_t arena_item = (ptrdiff_t)(arena + 1);
+ const Pool_Allocator_Arena *arena_item = arena + 1;
+ const ptrdiff_t ptr_diff = (char *) ptr - (const char *)
arena_item;
- if (arena_item <= ptritem) {
- const ptrdiff_t ptr_diff = ptritem - arena_item;
-
- if (ptr_diff < a_size
+ if (ptr_diff >= 0 && ptr_diff < a_size
&& ptr_diff % pool->object_size == 0)
return 1;
- }
arena = arena->next;
}
--
Andy Dougherty [email protected]
_______________________________________________
http://lists.parrot.org/mailman/listinfo/parrot-dev
