Closed Bug 634417 Opened 9 years ago Closed 6 years ago

Remove uses of vanilla malloc/calloc/realloc/free

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla28

People

(Reporter: njn, Assigned: njn)

References

Details

(Whiteboard: [qa-])

Attachments

(3 files)

Bug 624878 removed uses of vanilla new/new[] from SpiderMonkey.  This bug is for removing uses of vanilla malloc/calloc/realloc/free, the presence of which makes life difficult for embedders who use JS_USE_CUSTOM_ALLOCATOR.

js_malloc/js_calloc/js_realloc/js_free should be used instead.  find_vanilla_new_calls should be augmented to search for malloc/calloc/realloc as well, which should be easy.
There are some uses in nanojit/avmplus.h, ctypes/*, assembler/assembler/AssemblerBuffer{,WithConstantPool}.h, jsprf.cpp, shell/js.cpp, and probably some others.
It would be awesome if 2.0 ships with a completely working JS_USE_CUSTOM_ALLOCATOR. This, in my mind, means you can build a js shell with JS_USE_CUSTOM_ALLOCATOR and run the test suite successfully with an LD_PRELOAD of an .so which defined malloc/calloc/realloc/free/new/new[]/delete/strdup and makes them assert().
Embedders will rejoice all around.

/be
Based on bug 634155, (js_new should be cx->new) I guess mallocs should become cx->malloc rather than js_malloc whenever possible (this keeps the GC accounting up to date).
Blocks: 666042
Some notes.

- I used js_malloc et al instead of cx->malloc_ et al everywhere, because in
  all the relevant places we didn't have a |cx| handy.

- In Profilers.cpp I avoided strdup() by just manually emulating it.  Eh.

- mozilla::Vector uses mozilla::MallocAllocPolicy by default, so we should use
  js::Vector instead.

If you're wondering how I found these, it was partly with grep, and partly with
a Python script I wrote that grovels through the output of |nm|.

I was hoping to land that script in order to prevent backsliding.  But in the
end I can't get it to work sufficiently reliably with a normal build of
SpiderMonkey.  If I build SpiderMonkey with custom versions of js_malloc (et
al) that don't call system malloc (et al) then it is reliable... but that's not
an easily testable configuration, unfortunately.
Attachment #828451 - Flags: review?(luke)
Attachment #828451 - Flags: review?(luke) → review+
Is this worth shipping in the 24 release of the engine?
> Is this worth shipping in the 24 release of the engine?

You mean, backporting to ESR 24?
(In reply to Nicholas Nethercote [:njn] from comment #7)
> > Is this worth shipping in the 24 release of the engine?
> 
> You mean, backporting to ESR 24?

I didn't know whether the standalone engine release was coming off the branch or a completely different repository.  So yes, I suppose I do.
sstangl, is it worth trying to sneak this into the SpiderMonkey 24 standalone release?  AIUI, there's a few days until that release's deadline is hit.
Flags: needinfo?(sstangl)
https://hg.mozilla.org/mozilla-central/rev/de3af7ac0598
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
(In reply to Nicholas Nethercote [:njn] from comment #10)
> sstangl, is it worth trying to sneak this into the SpiderMonkey 24
> standalone release?  AIUI, there's a few days until that release's deadline
> is hit.

Release is being postponed for other reasons, so I'll include this patch. Thanks!
Flags: needinfo?(sstangl)
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.