904809 - OdinMonkey: root ProfiledFunction::name

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Luke Wagner [:luke]]

Currently it is unrooted which leads to dangling and badness if we GC between compile and link.  Fortunately, this bug is only present if defined(MOZ_VTUNE) or defined(JS_ION_PERF).

Comment 1

[Luke Wagner [:luke]]

Attachment: fix-perf

Douglas: does this fix your problem?

Comment 2

[Douglas Crosher [:dougc]]

A slightly different solution was used in the bug 893363 - declaring the names as 'RelocatablePtr<JSAtom> name' and marking using 'MarkString'.   Have not seen any more corruption or crashes on the ARM with that.

Are the function name atoms not movable?

What about the other names in the module that are declared as a RelocatablePtr.  What is the distinction between the function names and these?

Comment 3

[Luke Wagner [:luke]]

(In reply to Douglas Crosher [:dougc] from comment #2)
> What about the other names in the module that are declared as a
> RelocatablePtr.  What is the distinction between the function names and
> these?

Good question!  If you pull inbound again you'll notice that they no longer are: I just landed a patch in bug 900669 that removed them.  Basically RelocatablePtr was overkill for these fields since they don't need any of the barriers and it is incompatible with off-main-thread asm.js compilation.

Comment 4

[Douglas Crosher [:dougc]]

(In reply to Luke Wagner [:luke] from comment #3)
> (In reply to Douglas Crosher [:dougc] from comment #2)
> > What about the other names in the module that are declared as a
> > RelocatablePtr.  What is the distinction between the function names and
> > these?
> 
> Good question!  If you pull inbound again you'll notice that they no longer
> are: I just landed a patch in bug 900669 that removed them.  Basically
> RelocatablePtr was overkill for these fields since they don't need any of
> the barriers and it is incompatible with off-main-thread asm.js compilation.

Ok, thank you. The patch in bug 893363 has been updated to use this approach and testing on the ARM shows no corruption of the names or crashes.

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/16ff56b1021e