Closed
Bug 283234
Opened 19 years ago
Closed 19 years ago
jsarena.c arena reuse + reallocation can lose other allocations.
Categories
(Core :: JavaScript Engine, defect, P1)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla1.8beta2
People
(Reporter: dylan, Assigned: brendan)
Details
(Keywords: js1.5)
Attachments
(2 files)
1.46 KB,
text/plain
|
Details | |
750 bytes,
patch
|
shaver
:
review+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.5) Gecko/20041107 Firefox/1.0 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.5) Gecko/20041107 Firefox/1.0 In JS_ArenaAllocate(), if the allocation is oversized, the function may select an arena from the free list that is larger than necessary, as noted in this comment: /* * Insist on exact arenasize match if nb is not greater than * arenasize. Otherwise take any arena big enough, but not by * more than gross + arenasize. */ But this leaves room for more (small) allocations to be added after this allocation in the same arena, which violates the “every oversized allocation is the only allocation in its arena” rule. When the oversized allocation is subsequently grown, other allocations will be lost. This bug doesn’t get hit when all oversized allocations are powers of two, but the Sprinter code does seem to be something that can create unusually-sized allocations. One way to fix this would be to push avail all the way out to limit for any arena with an oversized allocation, and change the grow/realloc functions to simply leave the arena alone if the caller wants to grow the allocation into space that the arena already has. Reproducible: Always Steps to Reproduce:
Reporter | ||
Comment 1•19 years ago
|
||
Comments in this file show where the smaller allocation will get lost.
Assignee | ||
Comment 2•19 years ago
|
||
This could explain talkback from crashes to-do with NewOrRecycledNode and other arena-based allocators. I'd like to fix it ASAP for impending releases. /be
Flags: blocking1.7.6?
Flags: blocking-aviary1.0.1?
Keywords: js1.5
Priority: -- → P1
Target Milestone: --- → mozilla1.8beta2
Assignee | ||
Updated•19 years ago
|
Assignee: general → brendan
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee | ||
Comment 3•19 years ago
|
||
I don't see the point in messing around here. /be
Attachment #175232 -
Flags: review?(shaver)
Assignee | ||
Updated•19 years ago
|
Status: NEW → ASSIGNED
Comment 4•19 years ago
|
||
Don't you need to fix the comment right above that?
Assignee | ||
Comment 5•19 years ago
|
||
Jband, yeah: I was rushing to get into Firefox 1.0.1, but that train has left. Here's the revised comment I'll check into the trunk: /* - * Insist on exact arenasize match if nb is not greater than - * arenasize. Otherwise take any arena big enough, but not by - * more than gross + arenasize. + * Insist on exact arenasize match to avoid leaving alloc'able + * space after an oversized allocation as it grows. */ /be
Comment on attachment 175232 [details] [diff] [review] simplest fix r=shaver, thanks for the IRC explanations as well.
Attachment #175232 -
Flags: review?(shaver) → review+
Assignee | ||
Comment 7•19 years ago
|
||
Fixed on trunk. Candidate for future dot releases off branches, but not gonna hold up 1.0.1 (therefore ditto 1.7.6). /be
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Flags: blocking1.7.6?
Flags: blocking-aviary1.0.1?
Resolution: --- → FIXED
Updated•19 years ago
|
Flags: testcase-
You need to log in
before you can comment on or make changes to this bug.
Description
•