Closed
Bug 600687
Opened 14 years ago
Closed 14 years ago
ArenaBitmap does not account for color
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
blocking2.0 | --- | betaN+ |
People
(Reporter: igor, Assigned: igor)
References
Details
(Whiteboard: fixed-in-tracemonkey)
Attachments
(1 file)
(deleted),
patch
|
gwagner
:
review+
|
Details | Diff | Splinter Review |
ArenaBitmap contains:
JS_ALWAYS_INLINE void mark(size_t bit, uint32 color) {
uintptr_t *word = &bitmap[bit / BitsPerWord];
JS_ASSERT(word < &bitmap[JS_ARRAY_LENGTH(bitmap)]);
*word |= (uintptr_t(1) << ((bit + color) % BitsPerWord));
}
Here the word calculation does not add color to the bit leading to the wrong word selected when bit + color divides BitsPerWord effectively treating the pair (bit, color) as if it is (bit + color - BitsPerWord, 0).
In practice, given that the code always use the above pattern, the bug can lead to bad consequences only if the number of cells in GC thing divides BitsPerWord - 1. Only then it is possible that the code maps black bit into used white one. On 32 bit CPU that means only GC things with 31 cells will be affected and the bug is not possible. On 64 bits, given that 63 = 3*3*7, more bad sizes are possible but none of them matches the size of object, function or xml (13, 18 or 20 GC cells).
So we are safe until the bug 584917 lands.
Comment 1•14 years ago
|
||
This function is not used at all if I remember correctly. I wanted to remove it but it somehow stayed in. Lets just remove it?
Assignee | ||
Comment 2•14 years ago
|
||
(In reply to comment #1)
> This function is not used at all if I remember correctly. I wanted to remove it
> but it somehow stayed in. Lets just remove it?
All 3 functions has this bug. But if mark is not used, then I will remove it.
Assignee | ||
Comment 3•14 years ago
|
||
Cell::mark was not used indeed. For consistency with the rest of code the patch replaces the custom BitsPerWord constant with the usage of JS_BITS_PER_WORD.
Attachment #479581 -
Flags: review?(anygregor)
Comment 4•14 years ago
|
||
Comment on attachment 479581 [details] [diff] [review]
v1
thx!
Attachment #479581 -
Flags: review?(anygregor) → review+
Assignee | ||
Comment 5•14 years ago
|
||
Nominating for 2.0 to have this regression fixed to avoid bad suprises if the structure of JSObject changes or if the bug 584917 would land there.
blocking2.0: --- → ?
Assignee | ||
Comment 6•14 years ago
|
||
Whiteboard: fixed-in-tracemonkey
Comment 7•14 years ago
|
||
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Updated•14 years ago
|
blocking2.0: ? → betaN+
You need to log in
before you can comment on or make changes to this bug.
Description
•