Closed
Bug 499672
Opened 15 years ago
Closed 15 years ago
Verifier produces garbage and uses GCAlloc when FixedAlloc would be fine
Categories
(Tamarin Graveyard :: Virtual Machine, defect)
Tracking
(Not tracked)
VERIFIED
FIXED
People
(Reporter: edwsmith, Assigned: edwsmith)
References
Details
Attachments
(1 file)
13.38 KB,
patch
|
tharwood
:
review+
lhansen
:
superreview+
|
Details | Diff | Splinter Review |
Several memory management improvements are desired: * used fixed allocation instead of GCAlloc for blockStates, FrameState * only the jit code path is protected by try/catch, all uses of the verifier should be. * verifier produces two un-freed FrameState's per invocation of verify() and one of them is entirely unused. Coder & LIR need cleanup as well, will use separate bugs.
Assignee | ||
Comment 1•15 years ago
|
||
In this patch: * FrameState, blockStates, and Value allocated with FixedMem * Value[] array is a separate allocation from FrameState * TRY/CATCH around verifier handles interp-only invocation too * removed redundant initialState instance of FrameState * explicitly free verifier->state in addition to blockStates The only GC mem used by the verifier now is objects that get attached into the type system and live longer than the verifier: - ExceptionHandlerTable - ScopeTypeChain - things allocated by resolveTraits & resolveSignatures
Assignee: nobody → edwsmith
Attachment #384392 -
Flags: review?(lhansen)
Assignee | ||
Updated•15 years ago
|
Attachment #384392 -
Flags: superreview?(lhansen)
Attachment #384392 -
Flags: review?(tharwood)
Attachment #384392 -
Flags: review?(lhansen)
Comment 2•15 years ago
|
||
Comment on attachment 384392 [details] [diff] [review] Convert verifier blockStates & FrameState & Value to fixed mem Overall comment: new memory handling regime has lost some OOM checks. Are they done elsewhere? - scopeValue(i) could use an assertion to verify input as was done for value(i). scope overflow not very likely due to other checks? - The "// pass 2 - data flow" comment @ MethodInfo.cpp line 291 is obsolete? - Verifier::cleanup() could delete blockStates.at(i) -- delete NULL port issues? - Verifier::verify() passes self to FrameState ctor, but still has logic copying frame info into the new FrameState. Move into ctor? Unless NULL isn't binary zero on a platform, this loop looks redundant b/c the ctor does initialize the new FrameState's locals to 0.
Attachment #384392 -
Flags: review?(tharwood) → review+
Comment 3•15 years ago
|
||
Note, out of memory errors are no longer possible unless specific APIs (FixedMalloc::PleaseAlloc) are used. NULL checks are entirely redundant.
Updated•15 years ago
|
Attachment #384392 -
Flags: superreview?(lhansen) → superreview+
Assignee | ||
Comment 4•15 years ago
|
||
http://hg.mozilla.org/tamarin-redux/rev/bd9cc869b195
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Updated•15 years ago
|
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•