1101905 - Make bytecode contain a notion of strictness

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Eric Faust [:efaust]]

Part 1 of bug 1101903, with more description there.

I have a prototype of this; patches forthcoming.

Comment 1

[Eric Faust [:efaust]]

Attachment: Part -1: Cleanup emitting JSOP_DELNAME

We used to have a notion where we could change JSOP_DELNAME to JSOP_FALSE if we saw the right conditions, but that's no longer the case. Remove some machinery that looks for it.

Comment 2

[Eric Faust [:efaust]]

Attachment: Part 0: Add general machinery for making strict opcodes

Comment 3

[Eric Faust [:efaust]]

Attachment: Part 1: Strictify JSOP_DELPROP and JSOP_DELELEM

Comment 4

[Eric Faust [:efaust]]

Attachment: Part 2: Strictify JSOP_SETPROP

Comment 5

[Eric Faust [:efaust]]

Attachment: Part 3: Strictify JSOP_SETNAME

Comment 6

[Eric Faust [:efaust]]

Attachment: Part 4: Strictify JSOP_SETGNAME

Comment 7

[Eric Faust [:efaust]]

Attachment: Part 5: Strictify JSOP_SETELEM

Comment 8

[Eric Faust [:efaust]]

Attachment: Part 6: Cleanup remaining uses of script->strict() in property set paths

Comment 9

[Eric Faust [:efaust]]

Attachment: Part 7: Use options.strictOption to pass strictness into Eval script compilation, rather than the strictness of the calling script

Comment 10

[Eric Faust [:efaust]]

Attachment: Part 8: Strictify JSOP_EVAL and JSOP_SPREADEVAL

Comment 11

[Eric Faust [:efaust]]

Attachment: Part 9: Remove script->strict from the error reporter, using the strictness of the failing opcode instead.

Comment 12

[Eric Faust [:efaust]]

Still not stricified:
JSOP_THIS
JSOP_ARGUMENTS
JSOP_DEFFUN

The first two clearly rely on the actual strictness of the script, rather than the site of the op, so they remain as they were.

The last is also arguably in this category, as it only uses the strictness to bind the function. Perhaps it should also be done?

Comment 13

[Jeff Walden [:Waldo]]

Comment on attachment 8525615 [details] [diff] [review]
Part -1: Cleanup emitting JSOP_DELNAME

Review of attachment 8525615 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/frontend/BytecodeEmitter.cpp
@@ +5891,5 @@
>        {
>          if (!BindNameToSlot(cx, bce, pn2))
>              return false;
> +        if (!EmitAtomOp(cx, pn2, pn2->getOp(), bce))
> +            return false;

Woo woo glad to see this dead.

Comment 14

[Jeff Walden [:Waldo]]

Comment on attachment 8525616 [details] [diff] [review]
Part 0: Add general machinery for making strict opcodes

Review of attachment 8525616 [details] [diff] [review]:
-----------------------------------------------------------------

Legit.

Comment 15

[Jeff Walden [:Waldo]]

Comment on attachment 8525617 [details] [diff] [review]
Part 1: Strictify JSOP_DELPROP and JSOP_DELELEM

Review of attachment 8525617 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/frontend/BytecodeEmitter.cpp
@@ +5915,2 @@
>        {
> +        JSOp strictOp = bce->sc->strict ? JSOP_STRICTDELPROP : JSOP_DELPROP;

delOp?  strictOp suggests it's always JSOP_STRICT* to me.  Same below.

::: js/src/jit/BaselineCompiler.cpp
@@ +2112,5 @@
>      pushArg(ImmGCPtr(script->getName(pc)));
>      pushArg(R0);
>  
> +    bool strict = JSOp(*pc) == JSOP_STRICTDELPROP;
> +    if (!callVM(strict ? DeletePropertyStrictInfo : DeletePropertyNonStrictInfo))

So in the strict case, we could push a constant true as the result.  This is simpler for now, so let's roll with this, and file a (good first?) bug to be more optimal for the strict-mode case (for both strictdel* ops).

::: js/src/jit/CodeGenerator.cpp
@@ -8600,5 @@
>  {
>      pushArg(ImmGCPtr(lir->mir()->name()));
>      pushArg(ToValue(lir, LCallDeleteProperty::Value));
>  
> -    if (lir->mir()->block()->info().script()->strict())

Holy dereferencing Batman!

::: js/src/jit/MIR.h
@@ +10158,5 @@
>    public:
>      INSTRUCTION_HEADER(DeleteElement)
>  
> +    static MDeleteElement *New(TempAllocator &alloc, MDefinition *value, MDefinition *index,
> +                               bool strict) 

WS

::: js/src/vm/Interpreter.cpp
@@ +2267,5 @@
>  }
>  END_CASE(JSOP_DELNAME)
>  
>  CASE(JSOP_DELPROP)
> +CASE(JSOP_STRICTDELPROP)

I might be inclined to duplicate the code for these, myself, but it doesn't much matter.

@@ +2280,5 @@
>  
>      bool succeeded;
>      if (!JSObject::deleteGeneric(cx, obj, id, &succeeded))
>          goto error;
> +    if (!succeeded && (JSOp(*REGS.pc) == JSOP_STRICTDELPROP)) {

No need to overparenthesize here.

@@ +2307,5 @@
>      if (!ValueToId<CanGC>(cx, propval, &id))
>          goto error;
>      if (!JSObject::deleteGeneric(cx, obj, id, &succeeded))
>          goto error;
> +    if (!succeeded && (JSOp(*REGS.pc) == JSOP_STRICTDELELEM)) {

Again overparen.

::: js/src/vm/Opcodes.h
@@ +426,5 @@
>      macro(JSOP_SYMBOL,    45, "symbol",     NULL,         2,  0,  1,  JOF_UINT8) \
>      \
> +    /*
> +     * Pops the top of stack value, deletes property from it, pushes 'true' onto
> +     * the stack if succeeded, 'false' if not. (Strict mode)

Pops the top stack value and attempts to delete the given property from it.  Pushes 'true' on success, else throws a TypeError per strict mode property-deletion requirements.

@@ +436,5 @@
> +    macro(JSOP_STRICTDELPROP,   46, "strict-delprop",    NULL,         5,  1,  1, JOF_ATOM|JOF_PROP|JOF_CHECKSTRICT) \
> +    /*
> +     * Pops the top two values on the stack as 'propval' and 'obj',
> +     * deletes 'propval' property from 'obj', pushes 'true'  onto the stack if
> +     * succeeded, 'false' if not. (Strict mode)

Pops 'obj' and 'propval' and attempts to delete the 'propval' property from 'obj'.  Pushes 'true' on success, else throws a TypeError per strict mode property-deletion requirements.

These ops (notwithstanding their current state) should probably be in the Special Operators category.  Fix these new ones and the existing ones, please.

Comment 16

[Jeff Walden [:Waldo]]

Comment on attachment 8525619 [details] [diff] [review]
Part 2: Strictify JSOP_SETPROP

Review of attachment 8525619 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/frontend/BytecodeEmitter.cpp
@@ +2422,5 @@
>          if (Emit1(cx, bce, JSOP_SWAP) < 0)                  // N? OBJ N+1
>              return false;
>      }
>  
> +    JSOp setOp = bce->sc->strict ? JSOP_STRICTSETPROP : JSOP_SETPROP;

setOp, like the delOp I just suggested.  Nice!

::: js/src/jit/BaselineCompiler.cpp
@@ +2052,5 @@
>  
>  bool
> +BaselineCompiler::emit_JSOP_STRICTSETPROP()
> +{
> +    return emit_JSOP_SETPROP();

Hmm, this spooky strictness-checking at a distance is sadmaking.  Bleh.

::: js/src/jit/BaselineIC.cpp
@@ +7893,5 @@
>      JSOp op = JSOp(*pc);
>      FallbackICSpew(cx, stub, "SetProp(%s)", js_CodeName[op]);
>  
>      MOZ_ASSERT(op == JSOP_SETPROP ||
> +               op == JSOP_STRICTSETPROP ||

So fixing fallback is certainly necessary, but I kind of doubt it's sufficient for caches to work.  :-\  That said, brief investigation couldn't find anything that the IonBuilder.cpp changes wouldn't have kept working, so....

::: js/src/jit/IonBuilder.cpp
@@ +10010,5 @@
>  
>      // Always use a call if we are doing the definite properties analysis and
>      // not actually emitting code, to simplify later analysis.
>      if (info().executionModeIsAnalysis()) {
> +        // XXX: Dropped later in stack, but for now other ops rely on the old mechanic.

wat

@@ +10393,5 @@
>  {
>      MOZ_ASSERT(*emitted == false);
>  
> +    // XXX Dropped later in stack, but while others rely on it, we must retain the check.
> +    bool strict = IsStrictSetPC(pc) || script()->strict();

**2

::: js/src/jsopcode.h
@@ +667,5 @@
>  IsSetPropPC(jsbytecode *pc)
>  {
>      JSOp op = JSOp(*pc);
> +    return op == JSOP_SETPROP || op == JSOP_SETNAME || op == JSOP_SETGNAME ||
> +           op == JSOP_STRICTSETPROP;

Mild preference for setprops adjacent.

::: js/src/vm/Opcodes.h
@@ +445,5 @@
>       */ \
>      macro(JSOP_STRICTDELELEM,   47, "strict-delelem",    NULL,         1,  2,  1, JOF_BYTE|JOF_ELEM|JOF_CHECKSTRICT) \
> +    /*
> +     * Pops the top two values on the stack as 'val' and 'obj', sets property of
> +     * 'obj' as 'val', pushes 'obj' onto the stack. (Strict Mode)

Pops the top two values on the stack as 'val' and 'obj' and performs 'obj.prop = val', pushing 'val' back onto the stack.  Throws a TypeError if the set-operation failed (per strict mode semantics).

@@ +469,3 @@
>      /*
>       * Pops the top two values on the stack as 'val' and 'obj', sets property of
>       * 'obj' as 'val', pushes 'obj' onto the stack.

Same comment as before, minus the strict-mode bit.

Comment 17

[Jeff Walden [:Waldo]]

Comment on attachment 8525621 [details] [diff] [review]
Part 3: Strictify JSOP_SETNAME

Review of attachment 8525621 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/frontend/BytecodeEmitter.cpp
@@ +1517,5 @@
> +StrictifySetNameOp(JSOp op, BytecodeEmitter *bce)
> +{
> +    switch (op) {
> +        case JSOP_SETNAME:
> +            if (bce->sc->strict) op = JSOP_STRICTSETNAME;

Two lines, please.  And ixnay on the crazy indentation.

@@ +1525,5 @@
> +    return op;
> +}
> +
> +static void
> +StrictifySetNameOp(ParseNode *pn, BytecodeEmitter *bce)

Prefer a different name for this to overloading: StrictifySetNameNode or so.

::: js/src/jit/BaselineCompiler.cpp
@@ +2064,5 @@
>  
>  bool
> +BaselineCompiler::emit_JSOP_STRICTSETNAME()
> +{
> +    return emit_JSOP_SETPROP();

I guess this means this patch and the previous one require folding, for the SetProp fallback to be consistent with JSOP_*SETNAME at all times in this patch-stack.

::: js/src/jsopcode.h
@@ +668,5 @@
>  IsSetPropPC(jsbytecode *pc)
>  {
>      JSOp op = JSOp(*pc);
>      return op == JSOP_SETPROP || op == JSOP_SETNAME || op == JSOP_SETGNAME ||
> +           op == JSOP_STRICTSETPROP || op == JSOP_STRICTSETNAME;

Adjacency.

::: js/src/vm/Interpreter-inl.h
@@ +311,4 @@
>      MOZ_ASSERT_IF(*pc == JSOP_SETGNAME, scope == cx->global());
>  
> +    // XXX: Removed later in stack. SETGNAME still relies on script.
> +    bool strict = *pc == JSOP_STRICTSETNAME || script->strict();

watwat

::: js/src/vm/Interpreter.cpp
@@ +2386,3 @@
>  {
> +    static_assert(JSOP_SETNAME_LENGTH == JSOP_STRICTSETNAME_LENGTH,
> +                  "setname and strict-setname must be the same size");

Retroactive nitpick that s/-// in these messages -- there's no dash/underscore/etc. in the name, so don't put one here either.

::: js/src/vm/Opcodes.h
@@ +454,5 @@
>       */ \
>      macro(JSOP_STRICTSETPROP,   48, "strict-setprop",    NULL,         5,  2,  1, JOF_ATOM|JOF_PROP|JOF_SET|JOF_DETECTING|JOF_CHECKSTRICT) \
> +    /*
> +     * Pops a scope and value from the stack, assigns value to the given name,
> +     * and pushes the value back on the stack. (Strict mode)

s/\(Strict mode\)/If the set failed, then throw a TypeError per usual strict mode semantics./

Comment 18

[Jeff Walden [:Waldo]]

Comment on attachment 8525622 [details] [diff] [review]
Part 4: Strictify JSOP_SETGNAME

Review of attachment 8525622 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/frontend/BytecodeEmitter.cpp
@@ +1520,5 @@
>          case JSOP_SETNAME:
>              if (bce->sc->strict) op = JSOP_STRICTSETNAME;
>              break;
> +        case JSOP_SETGNAME:
> +            if (bce->sc->strict) op = JSOP_STRICTSETGNAME;

Two lines.

@@ +1658,5 @@
>  
>      JSOp op;
>      switch (pn->getOp()) {
>        case JSOP_NAME:     op = JSOP_GETGNAME; break;
> +      case JSOP_SETNAME:  op = StrictifySetNameOp(JSOP_SETGNAME, bce); break;

These really should be multiple lines as well.

::: js/src/jit/BaselineIC.cpp
@@ +7942,5 @@
>          }
>      } else if (op == JSOP_SETNAME ||
>                 op == JSOP_STRICTSETNAME ||
> +               op == JSOP_SETGNAME ||
> +               op == JSOP_STRICTSETGNAME)

...I guess this has to be folded with the others, too, for DoSetPropFallback consistency with the actual ops.  Weeeeeeeeeeeee.

::: js/src/jit/IonBuilder.cpp
@@ +1684,4 @@
>        {
>          PropertyName *name = info().getAtom(pc)->asPropertyName();
>          JSObject *obj = &script()->global();
>          return setStaticName(obj, name);

So setStaticName also appears to depend on jsop_setprop behavior, so it too needs folding into all of this.  La la la...

You would win massive bonus points if you could point out tests that fail at these intermediate stages due to this fallback-behavior inconsistency, or alternatively wrote tests that demonstrated the bugginess.

::: js/src/jit/VMFunctions.cpp
@@ +507,5 @@
>      if (MOZ_LIKELY(!obj->getOps()->setProperty)) {
>          return baseops::SetPropertyHelper<SequentialExecution>(
>              cx, obj.as<NativeObject>(), obj.as<NativeObject>(), id,
>              (op == JSOP_SETNAME || op == JSOP_SETGNAME ||
> +             op == JSOP_STRICTSETNAME || op == JSOP_STRICTSETGNAME)

Mild preference for setname || strictsetname || setgname || strictsetgname ordering to have adjacency.

::: js/src/vm/Interpreter.cpp
@@ +2386,5 @@
>  {
>      static_assert(JSOP_SETNAME_LENGTH == JSOP_STRICTSETNAME_LENGTH,
>                    "setname and strict-setname must be the same size");
> +    static_assert(JSOP_SETGNAME_LENGTH == JSOP_STRICTSETGNAME_LENGTH,
> +                  "setgname and strict-setgname must be the same size");

s/-//

::: js/src/vm/Opcodes.h
@@ +1443,4 @@
>      \
> +    /*
> +     * Pops the top two values on the stack as 'val' and 'scope', sets property
> +     * of 'scope' as 'val' and pushes 'val' back on the stack. (Strict mode)

Same sort of "If the set fails, throw a TypeError per strict mode semantics." thing.

Comment 19

[Jeff Walden [:Waldo]]

Comment on attachment 8525624 [details] [diff] [review]
Part 5: Strictify JSOP_SETELEM

Review of attachment 8525624 [details] [diff] [review]:
-----------------------------------------------------------------

So the IonBuilder comment is pretty not-standards-conforming, but it's an existing problem, so good enough as long as we promptly fix it in more patches.

::: js/src/jit/IonBuilder.cpp
@@ +8480,5 @@
>          current->add(MPostWriteBarrier::New(alloc(), object, value));
>  
>      // Emit SetElementCache.
> +    bool strict = JSOp(*pc) == JSOP_STRICTSETELEM;
> +    MInstruction *ins = MSetElementCache::New(alloc(), object, index, value, strict, guardHoles);

Shouldn't strictness, instead of being part of this instruction, be part of MSetElementInstruction, just as it's part of MSetPropertyInstruction?  The consequence is that MCallSetElement has no sense of strictness at all, and lowering/codegen just pass a constant 0 for strictness.  That's gotta be buggy.  Pre-existingly, but we need this fixed, with a testcase that would fail.  Probably shouldn't hold up *this* particular patch, but it should be fixt pronto.

::: js/src/vm/Opcodes.h
@@ +504,5 @@
> +    macro(JSOP_SETELEM,   56, "setelem",    NULL,         1,  3,  1, JOF_BYTE |JOF_ELEM|JOF_SET|JOF_DETECTING|JOF_CHECKSLOPPY) \
> +    /*
> +     * Pops the top three values on the stack as 'val', 'propval' and 'obj',
> +     * sets 'propval' property of 'obj' as 'val', pushes 'obj' onto the
> +     * stack.

Same "Throws a TypeError if the set fails, per strict mode semantics." thing.

Comment 20

[Jeff Walden [:Waldo]]

Comment on attachment 8525625 [details] [diff] [review]
Part 6: Cleanup remaining uses of script->strict() in property set paths

Review of attachment 8525625 [details] [diff] [review]:
-----------------------------------------------------------------

This is pretty stampy, would rather do this after removal of script->strict() somehow, but this is probably far enough to assume it's okay (in concert with prior patch-reading).  If not, lolfuzzing.

Comment 21

[Jeff Walden [:Waldo]]

Comment on attachment 8525626 [details] [diff] [review]
Part 7: Use options.strictOption to pass strictness into Eval script compilation, rather than the strictness of the calling script

Review of attachment 8525626 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/builtin/Eval.cpp
@@ +394,5 @@
>                 .setForEval(true)
>                 .setNoScriptRval(false)
>                 .setMutedErrors(mutedErrors)
> +               .setIntroductionInfo(introducerFilename, "eval", lineno, maybeScript, pcOffset)
> +               .maybeMakeStrictMode(callerScript && callerScript->strict());

I...think you can assert |callerScript| here, maybe?

::: js/src/vm/Debugger.cpp
@@ +5602,5 @@
>             .setNoScriptRval(false)
>             .setFileAndLine(filename, lineno)
>             .setCanLazilyParse(false)
> +           .setIntroductionType("debugger eval")
> +           .maybeMakeStrictMode(frame ? frame.script()->strict() : false);

Oh, I guess JSScript::strict() doesn't quite die.  Blah.  I remembered that, yessir I did.

Comment 22

[Jeff Walden [:Waldo]]

Comment on attachment 8525627 [details] [diff] [review]
Part 8: Strictify JSOP_EVAL and JSOP_SPREADEVAL

Review of attachment 8525627 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/builtin/Eval.cpp
@@ +203,1 @@
>          return EvalJSON_NotJSON;

Rather than introduce EvalJSONType and further split things, instead make the argument EvalType directly.  Now that ES6 allows duplicated properties in an object literal, this restriction is no longer necessary.  There might be a test or two requiring adjustment for this, tho I guess with no semantic difference now, maybe not.

@@ +292,5 @@
>  
>      RootedScript callerScript(cx, caller ? caller.script() : nullptr);
> +    EvalJSONType jsonType = evalType == INDIRECT_EVAL ? JSON_INDIRECT_EVAL :
> +                                IsStrictEvalPC(pc) ? JSON_STRICT_DIRECT_EVAL : JSON_DIRECT_EVAL;
> +    EvalJSONResult ejr = TryEvalJSON(cx, jsonType, flatStr, args.rval());

You can just pass |evalType| unmodified given the ES6 change.

@@ +371,5 @@
>      if (!flatStr)
>          return false;
>  
> +    EvalJSONType ejt = IsStrictEvalPC(pc) ? JSON_STRICT_DIRECT_EVAL : JSON_DIRECT_EVAL;
> +    EvalJSONResult ejr = TryEvalJSON(cx, ejt, flatStr, vp);

And just DIRECT_EVAL here.

::: js/src/jit/BaselineIC.cpp
@@ +8654,5 @@
>  TryAttachCallStub(JSContext *cx, ICCall_Fallback *stub, HandleScript script, jsbytecode *pc,
>                    JSOp op, uint32_t argc, Value *vp, bool constructing, bool isSpread,
>                    bool useNewType)
>  {
> +    if (useNewType || op == JSOP_EVAL || op == JSOP_STRICTEVAL)

Dilatory, but should the spreadeval ops be here, too?  Seems like we don't want subs for them, either.

::: js/src/jsscript.cpp
@@ +2936,5 @@
> +        MOZ_ASSERT(JSOp(*pc) == JSOP_EVAL || JSOp(*pc) == JSOP_STRICTEVAL ||
> +                   JSOp(*pc) == JSOP_SPREADEVAL || JSOp(*pc) == JSOP_STRICTSPREADEVAL);
> +        mozilla::DebugOnly<bool> isSpread = JSOp(*pc) == JSOP_SPREADEVAL ||
> +                                            JSOp(*pc) == JSOP_STRICTSPREADEVAL;
> +        MOZ_ASSERT(*(pc + (isSpread ? JSOP_SPREADEVAL_LENGTH : JSOP_EVAL_LENGTH)) == JSOP_LINENO);

Before this:

static_assert(JSOP_SPREADEVAL_LENGTH == JSOP_STRICTSPREADEVAL_LENGTH,
              "next op after a spread must be at consistent offset");
static_assert(JSOP_EVAL_LENGTH == JSOP_STRICTEVAL_LENGTH,
              "next op after a direct eval must be at consistent offset");

(Sure, we have some of this elsewhere, but really you want such things directly by each call site.)

::: js/src/vm/Interpreter.cpp
@@ +2454,3 @@
>  {
> +    static_assert(JSOP_EVAL_LENGTH == JSOP_STRICTEVAL_LENGTH,
> +                  "eval and strict-eval must be the same size");

s/-// and below

@@ +4040,2 @@
>          if (cx->global()->valueIsEval(args.calleev())) {
>              if (!DirectEval(cx, args))

Hm, we could/should make this pass in strictness at some point, maybe.

::: js/src/vm/Opcodes.h
@@ +464,5 @@
>      macro(JSOP_STRICTSETNAME,   49, "strict-setname",    NULL,         5,  2,  1,  JOF_ATOM|JOF_NAME|JOF_SET|JOF_DETECTING|JOF_CHECKSTRICT) \
> +    /*
> +     * spreadcall variant of JSOP_EVAL
> +     *
> +     * Invokes 'eval' with 'args' and pushes the return value onto the stack. (Strict mode)

s/\..*/, performing a direct eval if 'eval' is the current realm's eval function./ perhaps.  And same futzing for JSOP_SPREADEVAL.

@@ +1179,4 @@
>      \
> +    /* ECMA-compliant call to eval op. */ \
> +    /*
> +     * Invokes 'eval' with 'args' and pushes return value onto the stack. (Strict Mode)

Same sort of futzing as above, for both.

Comment 23

[Jason Orendorff [:jorendorff]]

Comment on attachment 8525616 [details] [diff] [review]
Part 0: Add general machinery for making strict opcodes

Review of attachment 8525616 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/frontend/BytecodeEmitter.cpp
@@ +287,1 @@
>      /* These should filter through EmitVarOp. */

Put a blank line before this comment.

Comment 24

[Eric Faust [:efaust]]

(In reply to Jeff Walden [:Waldo] (remove +bmo to email) from comment #19)
> Comment on attachment 8525624 [details] [diff] [review]
> Part 5: Strictify JSOP_SETELEM
> 
> Review of attachment 8525624 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> So the IonBuilder comment is pretty not-standards-conforming, but it's an
> existing problem, so good enough as long as we promptly fix it in more
> patches.
> 
> ::: js/src/jit/IonBuilder.cpp
> @@ +8480,5 @@
> >          current->add(MPostWriteBarrier::New(alloc(), object, value));
> >  
> >      // Emit SetElementCache.
> > +    bool strict = JSOp(*pc) == JSOP_STRICTSETELEM;
> > +    MInstruction *ins = MSetElementCache::New(alloc(), object, index, value, strict, guardHoles);
> 
> Shouldn't strictness, instead of being part of this instruction, be part of
> MSetElementInstruction, just as it's part of MSetPropertyInstruction?  The
> consequence is that MCallSetElement has no sense of strictness at all, and
> lowering/codegen just pass a constant 0 for strictness.  That's gotta be
> buggy.  Pre-existingly, but we need this fixed, with a testcase that would
> fail.  Probably shouldn't hold up *this* particular patch, but it should be
> fixt pronto.

Filed bug 1105498.

Comment 25

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/04910ef33dea
https://hg.mozilla.org/mozilla-central/rev/2aa25878111a
https://hg.mozilla.org/mozilla-central/rev/4bf193e9c2f4
https://hg.mozilla.org/mozilla-central/rev/00e2dae48e06
https://hg.mozilla.org/mozilla-central/rev/6ec9033a4535
https://hg.mozilla.org/mozilla-central/rev/b1eae4acfc90
https://hg.mozilla.org/mozilla-central/rev/663083ca3c12
https://hg.mozilla.org/mozilla-central/rev/5b7b730484e4
https://hg.mozilla.org/mozilla-central/rev/9d174d48ff39
https://hg.mozilla.org/mozilla-central/rev/e39cad980949
https://hg.mozilla.org/mozilla-central/rev/d6ec04c0c78f