SortedMap should not be dynamically allocated

VERIFIED FIXED

Status

VERIFIED FIXED
10 years ago
9 years ago

People

(Reporter: stejohns, Unassigned)

Tracking

Details

Attachments

(1 attachment)

(Reporter)

Description

10 years ago
Created attachment 391452 [details] [diff] [review]
Patch

SortedMap allows you to allocate it via plain operator new (not gc-new), but this is unsafe: it uses List<> to store values, which assume that it lies on the stack, in a GCObject, or in a GCRoot, none of which apply here. This can cause the storage to get freed prematurely, causing amusingly hard to diagnose bugs.

Patch simply disallows dynamic allocation of SortedMap, requiring clients to use GCSortedMap instead.
Attachment #391452 - Flags: review?(rreitmai)

Comment 1

10 years ago
Comment on attachment 391452 [details] [diff] [review]
Patch

Just wondering if we should make SortedMap a GCObject and then get rid of GCSortedMap.  

Seems like it wouldn't add much overhead and would remove a potential source of bugs.
Attachment #391452 - Flags: review?(rreitmai) → review+
(Reporter)

Comment 2

10 years ago
Perhaps, but there is apparently one stack-based use in nanojit that I'm not prepared to try correcting at this time.
(Reporter)

Comment 3

10 years ago
pushed to redux as 2270:a40a2fb0fe71
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED

Comment 4

9 years ago
Resolved fixed engineering / work item that has been pushed.  Setting status to verified.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.