Ticket #2159 (closed bug: fixed)
OpenBSD 4.9/i386 segfaults (fwd)
Reported by: | Andrew Dougherty <doughera@…> | Owned by: | coke |
---|---|---|---|
Priority: | normal | Milestone: | 2.11 |
Component: | core | Version: | 3.6.0 |
Severity: | medium | Keywords: | |
Cc: | Language: | ||
Patch status: | applied | Platform: |
Description
[Sending a copy to Trac so it doesn't get lost] > > OpenBSD 4.9/i386 failed on parrot master as follows: > > > > <snip> > > > > git bisect blames > > > > 1d270a0331242a9e50170697e0b5d7d4d65096f9 is the first bad commit > > commit 1d270a0331242a9e50170697e0b5d7d4d65096f9 Author: chromatic > > <chromatic@wgz.org> Date: Tue Jul 5 17:03:38 2011 -0700 > > > > [GC] Fixed Win32-killing type errors in d789b5. > 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 doughera@lafayette.edu
Change History
Note: See
TracTickets for help on using
tickets.