Ticket #148 (closed patch: fixed)

Opened 6 years ago

Last modified 6 years ago

Zero PMC_struct_val when allocating objects

Reported by: donaldh Owned by: whiteknight
Priority: normal Milestone:
Component: core Version:
Severity: medium Keywords: pmc gc
Cc: Language:
Patch status: Platform: all

Description

PMC_struct_val was not zeroed when taken off the free list. That means that allocated objects were pointing back into the free list.

pointer.pmc was actually casting PMC_struct_val to a custom mark function and trying to call it.

The fix is to zero PMC_struct_val when taking the object off the free list.

Attachments

smallobject.patch Download (346 bytes) - added by donaldh 6 years ago.

Change History

Changed 6 years ago by donaldh

Changed 6 years ago by whiteknight

  • keywords pmc gc added
  • owner set to whiteknight
  • status changed from new to assigned
  • component changed from none to core

I need to find where again, but we have documentation somewhere that says specifically that these values don't get zeroed on allocation. This isn't to say that we shouldn't zero them, but that somebody at one point seems to have specifically decided against it for some reason.

It might be worthwhile to also consider that src/pmc/Pointer.pmc:init() should zero these values out explicitly, instead of forcing all allocated PMCs to be zeroed.

I actually like this patch and would like to see it applied, but I'd also like to see some feedback first.

Changed 6 years ago by whiteknight

allison merged in the pdd09gc_part2 branch yesterday which completely rearranged most of the memory-related code files. This is a relatively small change so I should be able to add this line of code in manually once I figure out where it's supposed to go now.

donaldh: to help make my search shorter, what function was this line supposed to be added to?

Others: Would it be more beneficial to add this into src/pmc/Pointer.pmc:init() instead of adding it to the PMC header allocator, since Pointer.pmc is the only place where non-initialization of this data value appears to be a problem?

Changed 6 years ago by whiteknight

Okay, this patch appears to make the addition to gc_ms_get_free_object, which is only in the MS collector. Instead, I'm probably going to make the change to new_pmc_header which will be more universal. If there are no complaints I'll put this in tomorrow.

Changed 6 years ago by whiteknight

  • status changed from assigned to closed
  • resolution set to fixed

Work is currently being done to remove the PMC_struct_val and other macros associated with the Unionval. I'm rejecting this ticket for now since it doesn't make any sense to add this only to have it removed again later.

If we really have a problem with uninitialized Pointer PMCs, we should apply a fix to initialization routine of that PMC type, and do it in a way that doesn't rely on the Unionval.

Note: See TracTickets for help on using tickets.