The current implementation CompilerOutput can still own a dandling pointer to a compiler output which has been invalidated. The current implementation is not supposed to use this pointer because we ensure that everything is valid before making any changes.
Making the CompilerOutput mirror the validity would be nicer. This imply to modify addPendingRecompiles(cx, script, pc) to reuse the current CompilerOutputs instead of forging new one from scratch.
With such modifications we can ensure that a compilation output can be invalidated only once and thus remove the output pointer because the script is already holding it.

Created attachment 651002[details][diff][review]
Ensure validity of CompilerOutput without a dandling pointer.
Add a constraint index on each result of a compilation. This index correspond to the CompilerOutput structure which identify the result of one compilation in a JSScript.
This patch definitively fix unrelated invalidation.
Currently this patch is block by some marking issue in IonMonkey, but a subset of it can be used for mozilla-central.

(In reply to Nicolas B. Pierron [:pierron] [:nbp] from comment #1)
> Currently this patch is block by some marking issue in IonMonkey, but a
> subset of it can be used for mozilla-central.
Update: it works fine against the HEAD of IonMonkey.
& Rename the Title to mention that this Bug targets both IonMonkey and central.

(In reply to Brian Hackett (:bhackett) from comment #3)
> Comment on attachment 651002[details][diff][review]
> Ensure validity of CompilerOutput without a dandling pointer.
>
> Review of attachment 651002[details][diff][review]:
> -----------------------------------------------------------------
>
> ::: js/src/ion/CodeGenerator.cpp
> @@ +1231,5 @@
> >
> > +#if defined(DEBUG) && (defined(JS_CPU_X64) || defined(JS_CPU_X86))
> > + // No-op used to index the LIR instructions in IonGraph output.
> > + masm.store16(Imm32(iter->id()), Address(StackPointer, -8));
> > +#endif
>
> What is this change for?
This is mostly for tracking issues in debug builds, this is supposed to be a no-op because we should never look below the stack pointer (due to an ARM restriction). So we can safely erase the space below the stack pointer on arch other than ARM to track the LIR index listed in IonGraph output.
This is unrelated to the current patch.
> ::: js/src/ion/IonCode.h
> @@ +206,5 @@
> >
> > // Number of times this function has tried to call a non-IM compileable function
> > uint32 slowCallCount;
> >
> > + types::RecompileInfo constraintIndex_;
>
> Needs a comment, and a better name (recompileInfo_?)
I feel like the constraint mechanism is orthogonal to the type inference, and that we should move it out-side the TypeCompartment. The RecompileInfo is only used to index a compiler output in the list of constrained outputs. This explain why I would prefer to do the opposite renaming. But in the mean time I will apply your nit and this feeling can potentially be part of another bug later.

This broke non-methodjit builds :
js/src/jsinfer.h:1021: error: 'mjit' has not been declared
js/src/jsinfer.h:1021: error: ISO C++ forbids declaration of 'JITScript' with no type
js/src/jsinfer.h:1021: error: expected ';' before '*' token
See http://buildbot.rhaalovely.net/builders/mozilla-central-sparc64/builds/382/steps/build/logs/stdio
Dunno what's the best fix here, put the mjit namespace declaration at the beginning of jsinfer.h outside #ifdef JS_METHODJIT or add more #ifdefs to the decl of ::mjit() and callers.
I'd go for the first solution, anyone has an advice on it ?