510004 - Don't rely on global new operator override

Status: VERIFIED FIXED

(Tamarin Graveyard :: Garbage Collection (mmGC), defect)

Description

[Tommy Reilly]

Attachment: patch

Add a feature controlled by MMGC_USE_ALLOC_MACROS that switches all allocations to avoid global new/delete

Comment 1

[Tommy Reilly]

Attachment: definition of new macros, no application

Comment 2

[Tommy Reilly]

Attachment: application of macros to tamarin codebase

requires 394077

Comment 3

[Edwin Smith]

Comment on attachment 394077 [details] [diff] [review]
definition of new macros, no application

MMGC_USE_ALLOC_MACROS should be an actual feature switch, like AVMFEATURE_USE_SYSTEM_MALLOC.

MMGC_USE_ALLOC_MACROS is a confusing name because what the switch does is switch between using FixedAlloc and global-new.  A separate switch, AVMFEATURE_USE_SYSTEM_MALLOC actually controls what global-new does.  Without bigger changes, the first switch should be MMGC_USE_FIXED_ALLOC or something like that.

if MMGC_USE_ALLOC_MACROS is not defined then system_new and mmfx_new do the same thing.  should system_new explicitly call malloc?  or, should MMGC_USE_ALLOC_MACROS require AVMFEATURE_USE_SYSTEM_MALLOC, so that system_new actually guarantees to map to the real system malloc?   in short... lets make these two switches interact in sensible ways and make sure the nonsense combinations are actually errors.

Need comments in the global-new macros.  

re: shorthand "newv" and "deletev" names -- lets either drop these or use them exclusively and drop the "_array" macros.  the aliases just increase required brainprint and api surface without adding value.

re: mmgc_ macros -- lets just remove them since they're commented out anyway and if they ever do get turned on, they won't be exactly in their current form.  maybe move to a separate bug/patch.  the comment is just going to go stale quickly.

Comment 4

[Tommy Reilly]

will post a new patch, the FixedMalloc flag stuff was separated out and pushed in bug 508037

Comment 5

[Tommy Reilly]

Attachment: update addressing Ed's concerns

Added AVMFEATURE stuff
removed newv short form
made AVMFEATURE_USE_SYSTEM_MALLOC work independently from AVMFEATURE_OVERRIDE_GLOBAL_NEW

Comment 6

[Tommy Reilly]

changeset:   2386:5145cc5bb9d1
tag:         tip
user:        Tommy Reilly <[e-mail]>
date:        Thu Aug 20 14:08:10 2009 -0400
summary:     Apply new mmfx macros to tamarin code base (r=edwsmith)

changeset:   2385:cb453f61fe73
user:        Tommy Reilly <[e-mail]>
date:        Thu Aug 20 13:07:53 2009 -0400
summary:     Missing from change 2384

changeset:   2384:97f74a6fb9b3
user:        Tommy Reilly <[e-mail]>
date:        Thu Aug 20 12:35:48 2009 -0400
summary:     Introduce new macros to replace overriding global new and delete which we don't do anymore unless AVMFEATURE_OVERRIDE_GLOBAL_NEW is on (r=edwsmith)

Comment 7

[Chris Peyer]

Resolved fixed engineering / work item that has been pushed.  Setting status to verified.