Closed
Bug 519283
Opened 15 years ago
Closed 15 years ago
FixedMalloc should not return NULL when the requested object is too large
Categories
(Tamarin Graveyard :: Garbage Collection (mmGC), defect)
Tamarin Graveyard
Garbage Collection (mmGC)
Tracking
(Not tracked)
VERIFIED
FIXED
flash10.1
People
(Reporter: lhansen, Assigned: lhansen)
References
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
Details | Diff | Splinter Review |
... unless kCanFail is specified, that is.
Instead, FixedMalloc should call GCHeap::SignalObjectSizeOverflow, which is being introduced as part of the fix in bug #517856.
Must also check the code in GC::Alloc, GCLargeAlloc::Alloc, and maybe others.
Assignee | ||
Comment 1•15 years ago
|
||
So far, excluding the above mentioned bug:
FixedMalloc::Alloc
FixedMalloc::Calloc
GC::Alloc
Assignee | ||
Comment 2•15 years ago
|
||
GC::Calloc
Assignee | ||
Comment 3•15 years ago
|
||
More generally, a number of MMgc APIs return NULL when they should not, when various error conditions occur:
GC::Alloc returns NULL in DEBUG mode if MMGC_GCENTER has not been called.
GC::FindBeginningGuarded returns NULL if the address is not managed.
FixedAlloc::Alloc returns NULL if CreateChunk failed to fill the free list and did not abort.
GCHeap::Alloc returns NULL if AllocBlock failed to expand the heap and did not abort.
Assignee | ||
Comment 4•15 years ago
|
||
Attachment #403740 -
Flags: review?(edwsmith)
Assignee | ||
Updated•15 years ago
|
Attachment #403740 -
Flags: review?(stejohns)
Assignee | ||
Comment 5•15 years ago
|
||
Comment on attachment 403740 [details] [diff] [review]
Patch for all the issues
The patch is straightforward and includes the SignalObjectTooLarge patch from bug #517856.
The main issue is that I've introduced two methods that unceremoniously call VMPI_exit when they are called (after asserting and printing some debug info). This is an improvement on the old return-NULL behavior, and as an abstraction it's probably OK; the question is whether these methods should be doing something more sophisticated, like calling the GC's Abort() method as that will result in a clean shutdown of the host code (which can be a plugin). IMO we can land this patch (with your approval...) before settling that issue, but we need to settle it.
Updated•15 years ago
|
Attachment #403740 -
Flags: review?(stejohns) → review+
Assignee | ||
Comment 6•15 years ago
|
||
Looping in Rishit for a discussion of what the best thing to do is when we hit an implementation limit.
Assignee | ||
Comment 7•15 years ago
|
||
One case I missed before:
GCObject::operator new
Assignee | ||
Updated•15 years ago
|
Attachment #403740 -
Attachment is obsolete: true
Attachment #403740 -
Flags: review?(edwsmith)
Assignee | ||
Comment 8•15 years ago
|
||
Comment on attachment 403740 [details] [diff] [review]
Patch for all the issues
Another patch is going to be arriving first and will make this one largely obsolete; will resubmit if necessary.
Assignee | ||
Updated•15 years ago
|
Assignee | ||
Comment 9•15 years ago
|
||
This patch is a proper subset of the previous patch; it will land after bug #519980 has closed.
Assignee | ||
Comment 10•15 years ago
|
||
redux changeset: 2664:90b3d1f29e29
I made SignalObjectTooLarge and SignalHeapStateInconsistent call GCHeap::Abort instead of VMPI_exit, as that seems most reasonable.
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Updated•15 years ago
|
Status: RESOLVED → VERIFIED
Comment 11•15 years ago
|
||
QE: We can add a couple of simple checks for this into the extensions/ST_mmgc_basic.st
Flags: in-testsuite?
You need to log in
before you can comment on or make changes to this bug.
Description
•