From 39aa1d5cf4871e9a74f86289b5c7ca631eabf906 Mon Sep 17 00:00:00 2001 From: Steve Fink Date: Thu, 27 Mar 2014 09:00:58 -0700 Subject: [PATCH 1/4] Bug 1001689 - Stop suppressing the xmanager log file, r=njn When suppressing logs (aka displaying output messages), we still need the xmanager output to go into its log file, because its port number is read out of it (yes, it's ugly.) So pipe through tee instead of redirecting to the log. --HG-- extra : rebase_source : a4142c1d287779cc05b04988071e21415ffca445 --- js/src/devtools/rootAnalysis/run_complete | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/js/src/devtools/rootAnalysis/run_complete b/js/src/devtools/rootAnalysis/run_complete index a3a7c7875ae..7c96b222d22 100755 --- a/js/src/devtools/rootAnalysis/run_complete +++ b/js/src/devtools/rootAnalysis/run_complete @@ -199,6 +199,12 @@ sub get_manager_address return $1; } +sub logging_suffix { + my ($show_logs, $log_file) = @_; + return $show_logs ? "2>&1 | tee $log_file" + : "> $log_file 2>&1"; +} + sub run_build { print "build started: "; @@ -214,7 +220,8 @@ sub run_build if (!$pid) { # this is the child process, fork another process to run a manager. defined(my $pid = fork) or die; - exec("$xmanager -terminate-on-assert > $manager_log_file 2>&1") if (!$pid); + my $logging = logging_suffix($suppress_logs, $manager_log_file); + exec("$xmanager -terminate-on-assert $logging") if (!$pid); $kill_on_exit{$pid} = 1; if (!$suppress_logs) { @@ -286,7 +293,8 @@ sub run_pass # fork off a manager process for the analysis. defined(my $pid = fork) or die; - exec("$xmanager $manager_extra > $log_file 2>&1") if (!$pid); + my $logging = logging_suffix($suppress_logs, $log_file); + exec("$xmanager $manager_extra $logging") if (!$pid); my $address = get_manager_address($log_file); From 914525f2b6183ecd5f910b90f8b6ca7190b4b8cf Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Sat, 3 May 2014 01:08:07 -0400 Subject: [PATCH 2/4] Bug 1004766. Make sure to enter the compartment of our new global before working with it when wrapping global objects. r=nsm --- dom/bindings/Codegen.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/dom/bindings/Codegen.py b/dom/bindings/Codegen.py index 3800adcb0ad..61d378be0c1 100644 --- a/dom/bindings/Codegen.py +++ b/dom/bindings/Codegen.py @@ -2966,6 +2966,9 @@ class CGWrapGlobalMethod(CGAbstractMethod): aOptions, aPrincipal); + // obj is a new global, so has a new compartment. Enter it + // before doing anything with it. + JSAutoCompartment ac(aCx, obj); $*{unforgeable} $*{slots} From d7004c64a6ad2e6f75f0072b2502f238fdfe8f64 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Sat, 3 May 2014 01:08:13 -0400 Subject: [PATCH 3/4] Bug 1004198. Improve codegen in testValueTruthyKernel to emit as few tests as we can get away with given our type inference information. r=jandem --- js/src/jit/CodeGenerator.cpp | 166 +++++++++++++++++++++++++---------- js/src/jit/CodeGenerator.h | 6 +- 2 files changed, 125 insertions(+), 47 deletions(-) diff --git a/js/src/jit/CodeGenerator.cpp b/js/src/jit/CodeGenerator.cpp index e2dc2dc36df..f68d52010f9 100644 --- a/js/src/jit/CodeGenerator.cpp +++ b/js/src/jit/CodeGenerator.cpp @@ -508,52 +508,122 @@ CodeGenerator::testValueTruthyKernel(const ValueOperand &value, const LDefinition *scratch1, const LDefinition *scratch2, FloatRegister fr, Label *ifTruthy, Label *ifFalsy, - OutOfLineTestObject *ool) + OutOfLineTestObject *ool, + MDefinition *valueMIR) { - Register tag = masm.splitTagForTest(value); + // Count the number of possible type tags we might have, so we'll know when + // we've checked them all and hence can avoid emitting a tag check for the + // last one. In particular, whenever tagCount is 1 that means we've tried + // all but one of them already so we know exactly what's left based on the + // mightBe* booleans. + bool mightBeUndefined = valueMIR->mightBeType(MIRType_Undefined); + bool mightBeNull = valueMIR->mightBeType(MIRType_Null); + bool mightBeBoolean = valueMIR->mightBeType(MIRType_Boolean); + bool mightBeInt32 = valueMIR->mightBeType(MIRType_Int32); + bool mightBeObject = valueMIR->mightBeType(MIRType_Object); + bool mightBeString = valueMIR->mightBeType(MIRType_String); + bool mightBeDouble = valueMIR->mightBeType(MIRType_Double); + int tagCount = int(mightBeUndefined) + int(mightBeNull) + + int(mightBeBoolean) + int(mightBeInt32) + int(mightBeObject) + + int(mightBeString) + int(mightBeDouble); - // Eventually we will want some sort of type filter here. For now, just - // emit all easy cases. For speed we use the cached tag for all comparison, - // except for doubles, which we test last (as the operation can clobber the - // tag, which may be in ScratchReg). - masm.branchTestUndefined(Assembler::Equal, tag, ifFalsy); - masm.branchTestNull(Assembler::Equal, tag, ifFalsy); + MOZ_ASSERT_IF(!valueMIR->emptyResultTypeSet(), tagCount > 0); - Label notBoolean; - masm.branchTestBoolean(Assembler::NotEqual, tag, ¬Boolean); - masm.branchTestBooleanTruthy(false, value, ifFalsy); - masm.jump(ifTruthy); - masm.bind(¬Boolean); - - Label notInt32; - masm.branchTestInt32(Assembler::NotEqual, tag, ¬Int32); - masm.branchTestInt32Truthy(false, value, ifFalsy); - masm.jump(ifTruthy); - masm.bind(¬Int32); - - if (ool) { - Label notObject; - - masm.branchTestObject(Assembler::NotEqual, tag, ¬Object); - - Register objreg = masm.extractObject(value, ToRegister(scratch1)); - testObjectEmulatesUndefined(objreg, ifFalsy, ifTruthy, ToRegister(scratch2), ool); - - masm.bind(¬Object); - } else { - masm.branchTestObject(Assembler::Equal, tag, ifTruthy); + // If we know we're null or undefined, we're definitely falsy, no + // need to even check the tag. + if (int(mightBeNull) + int(mightBeUndefined) == tagCount) { + masm.jump(ifFalsy); + return; } - // Test if a string is non-empty. - Label notString; - masm.branchTestString(Assembler::NotEqual, tag, ¬String); - masm.branchTestStringTruthy(false, value, ifFalsy); - masm.jump(ifTruthy); - masm.bind(¬String); + Register tag = masm.splitTagForTest(value); - // If we reach here the value is a double. - masm.unboxDouble(value, fr); - masm.branchTestDoubleTruthy(false, fr, ifFalsy); + if (mightBeUndefined) { + MOZ_ASSERT(tagCount > 1); + masm.branchTestUndefined(Assembler::Equal, tag, ifFalsy); + --tagCount; + } + + if (mightBeNull) { + MOZ_ASSERT(tagCount > 1); + masm.branchTestNull(Assembler::Equal, tag, ifFalsy); + --tagCount; + } + + if (mightBeBoolean) { + MOZ_ASSERT(tagCount != 0); + Label notBoolean; + if (tagCount != 1) + masm.branchTestBoolean(Assembler::NotEqual, tag, ¬Boolean); + masm.branchTestBooleanTruthy(false, value, ifFalsy); + if (tagCount != 1) + masm.jump(ifTruthy); + // Else just fall through to truthiness. + masm.bind(¬Boolean); + --tagCount; + } + + if (mightBeInt32) { + MOZ_ASSERT(tagCount != 0); + Label notInt32; + if (tagCount != 1) + masm.branchTestInt32(Assembler::NotEqual, tag, ¬Int32); + masm.branchTestInt32Truthy(false, value, ifFalsy); + if (tagCount != 1) + masm.jump(ifTruthy); + // Else just fall through to truthiness. + masm.bind(¬Int32); + --tagCount; + } + + if (mightBeObject) { + MOZ_ASSERT(tagCount != 0); + if (ool) { + Label notObject; + + if (tagCount != 1) + masm.branchTestObject(Assembler::NotEqual, tag, ¬Object); + + Register objreg = masm.extractObject(value, ToRegister(scratch1)); + testObjectEmulatesUndefined(objreg, ifFalsy, ifTruthy, ToRegister(scratch2), ool); + + masm.bind(¬Object); + } else { + if (tagCount != 1) + masm.branchTestObject(Assembler::Equal, tag, ifTruthy); + // Else just fall through to truthiness. + } + --tagCount; + } else { + MOZ_ASSERT(!ool, + "We better not have an unused OOL path, since the code generator will try to " + "generate code for it but we never set up its labels, which will cause null " + "derefs of those labels."); + } + + if (mightBeString) { + // Test if a string is non-empty. + MOZ_ASSERT(tagCount != 0); + Label notString; + if (tagCount != 1) + masm.branchTestString(Assembler::NotEqual, tag, ¬String); + masm.branchTestStringTruthy(false, value, ifFalsy); + if (tagCount != 1) + masm.jump(ifTruthy); + // Else just fall through to truthiness. + masm.bind(¬String); + --tagCount; + } + + if (mightBeDouble) { + MOZ_ASSERT(tagCount == 1); + // If we reach here the value is a double. + masm.unboxDouble(value, fr); + masm.branchTestDoubleTruthy(false, fr, ifFalsy); + --tagCount; + } + + MOZ_ASSERT(tagCount == 0); // Fall through for truthy. } @@ -563,9 +633,10 @@ CodeGenerator::testValueTruthy(const ValueOperand &value, const LDefinition *scratch1, const LDefinition *scratch2, FloatRegister fr, Label *ifTruthy, Label *ifFalsy, - OutOfLineTestObject *ool) + OutOfLineTestObject *ool, + MDefinition *valueMIR) { - testValueTruthyKernel(value, scratch1, scratch2, fr, ifTruthy, ifFalsy, ool); + testValueTruthyKernel(value, scratch1, scratch2, fr, ifTruthy, ifFalsy, ool, valueMIR); masm.jump(ifTruthy); } @@ -612,7 +683,11 @@ bool CodeGenerator::visitTestVAndBranch(LTestVAndBranch *lir) { OutOfLineTestObject *ool = nullptr; - if (lir->mir()->operandMightEmulateUndefined()) { + // XXXbz operandMightEmulateUndefined lies a lot. See bug 1004169. In + // practice, we don't need the OutOfLineTestObject if the input to our test + // is not an object. + MDefinition* input = lir->mir()->input(); + if (lir->mir()->operandMightEmulateUndefined() && input->mightBeType(MIRType_Object)) { ool = new(alloc()) OutOfLineTestObject(); if (!addOutOfLineCode(ool)) return false; @@ -624,7 +699,7 @@ CodeGenerator::visitTestVAndBranch(LTestVAndBranch *lir) testValueTruthy(ToValue(lir, LTestVAndBranch::Input), lir->temp1(), lir->temp2(), ToFloatRegister(lir->tempFloat()), - truthy, falsy, ool); + truthy, falsy, ool, input); return true; } @@ -5291,6 +5366,7 @@ CodeGenerator::visitNotV(LNotV *lir) OutOfLineTestObjectWithLabels *ool = nullptr; if (lir->mir()->operandMightEmulateUndefined()) { + MOZ_ASSERT(lir->mir()->operand()->mightBeType(MIRType_Object)); ool = new(alloc()) OutOfLineTestObjectWithLabels(); if (!addOutOfLineCode(ool)) return false; @@ -5305,7 +5381,7 @@ CodeGenerator::visitNotV(LNotV *lir) testValueTruthyKernel(ToValue(lir, LNotV::Input), lir->temp1(), lir->temp2(), ToFloatRegister(lir->tempFloat()), - ifTruthy, ifFalsy, ool); + ifTruthy, ifFalsy, ool, lir->mir()->operand()); Label join; Register output = ToRegister(lir->output()); diff --git a/js/src/jit/CodeGenerator.h b/js/src/jit/CodeGenerator.h index c47c3861f61..aabf2d8d959 100644 --- a/js/src/jit/CodeGenerator.h +++ b/js/src/jit/CodeGenerator.h @@ -389,7 +389,8 @@ class CodeGenerator : public CodeGeneratorSpecific const LDefinition *scratch1, const LDefinition *scratch2, FloatRegister fr, Label *ifTruthy, Label *ifFalsy, - OutOfLineTestObject *ool); + OutOfLineTestObject *ool, + MDefinition *valueMIR); // Test whether value is truthy or not and jump to the corresponding label. // If the value can be an object that emulates |undefined|, |ool| must be @@ -400,7 +401,8 @@ class CodeGenerator : public CodeGeneratorSpecific const LDefinition *scratch1, const LDefinition *scratch2, FloatRegister fr, Label *ifTruthy, Label *ifFalsy, - OutOfLineTestObject *ool); + OutOfLineTestObject *ool, + MDefinition *valueMIR); // This function behaves like testObjectEmulatesUndefined with the exception // that it can choose to let control flow fall through when the object From 0697387fc77ac37f8c3858980a2c8419b85a4c34 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Sat, 3 May 2014 01:08:14 -0400 Subject: [PATCH 4/4] Bug 1004169. Make sure MTest always uses TI information for deciding whether its operand might emulate undefined. r=jandem --- js/src/jit/CodeGenerator.cpp | 7 ++++--- js/src/jit/IonBuilder.cpp | 31 +++++++++++++++++++------------ js/src/jit/IonBuilder.h | 8 ++++++++ js/src/jit/MCallOptimize.cpp | 4 ++-- js/src/jit/MIR.cpp | 6 +++--- js/src/jit/MIR.h | 12 +++++++++--- 6 files changed, 45 insertions(+), 23 deletions(-) diff --git a/js/src/jit/CodeGenerator.cpp b/js/src/jit/CodeGenerator.cpp index f68d52010f9..6c579695959 100644 --- a/js/src/jit/CodeGenerator.cpp +++ b/js/src/jit/CodeGenerator.cpp @@ -683,10 +683,11 @@ bool CodeGenerator::visitTestVAndBranch(LTestVAndBranch *lir) { OutOfLineTestObject *ool = nullptr; - // XXXbz operandMightEmulateUndefined lies a lot. See bug 1004169. In - // practice, we don't need the OutOfLineTestObject if the input to our test - // is not an object. MDefinition* input = lir->mir()->input(); + // Unfortunately, it's possible that someone (e.g. phi elimination) switched + // out our input after we did cacheOperandMightEmulateUndefined. So we + // might think it can emulate undefined _and_ know that it can't be an + // object. if (lir->mir()->operandMightEmulateUndefined() && input->mightBeType(MIRType_Object)) { ool = new(alloc()) OutOfLineTestObject(); if (!addOutOfLineCode(ool)) diff --git a/js/src/jit/IonBuilder.cpp b/js/src/jit/IonBuilder.cpp index 0d649547a4b..11d41060f57 100644 --- a/js/src/jit/IonBuilder.cpp +++ b/js/src/jit/IonBuilder.cpp @@ -2168,7 +2168,7 @@ IonBuilder::processDoWhileCondEnd(CFGState &state) } // Create the test instruction and end the current block. - MTest *test = MTest::New(alloc(), vins, state.loop.entry, successor); + MTest *test = newTest(vins, state.loop.entry, successor); current->end(test); return finishLoop(state, successor); } @@ -2189,9 +2189,9 @@ IonBuilder::processWhileCondEnd(CFGState &state) MTest *test; if (JSOp(*pc) == JSOP_IFNE) - test = MTest::New(alloc(), ins, body, state.loop.successor); + test = newTest(ins, body, state.loop.successor); else - test = MTest::New(alloc(), ins, state.loop.successor, body); + test = newTest(ins, state.loop.successor, body); current->end(test); state.state = CFGState::WHILE_LOOP_BODY; @@ -2229,7 +2229,7 @@ IonBuilder::processForCondEnd(CFGState &state) if (!body || !state.loop.successor) return ControlStatus_Error; - MTest *test = MTest::New(alloc(), ins, body, state.loop.successor); + MTest *test = newTest(ins, body, state.loop.successor); current->end(test); state.state = CFGState::FOR_LOOP_BODY; @@ -3352,7 +3352,7 @@ IonBuilder::processCondSwitchCase(CFGState &state) cmpResult->infer(inspector, pc); JS_ASSERT(!cmpResult->isEffectful()); current->add(cmpResult); - current->end(MTest::New(alloc(), cmpResult, bodyBlock, caseBlock)); + current->end(newTest(cmpResult, bodyBlock, caseBlock)); // Add last case as predecessor of the body if the body is aliasing // the previous case body. @@ -3463,9 +3463,8 @@ IonBuilder::jsop_andor(JSOp op) return false; MTest *test = (op == JSOP_AND) - ? MTest::New(alloc(), lhs, evalRhs, join) - : MTest::New(alloc(), lhs, join, evalRhs); - test->infer(); + ? newTest(lhs, evalRhs, join) + : newTest(lhs, join, evalRhs); current->end(test); if (!cfgStack_.append(CFGState::AndOr(joinStart, join))) @@ -3516,7 +3515,7 @@ IonBuilder::jsop_ifeq(JSOp op) if (!ifTrue || !ifFalse) return false; - MTest *test = MTest::New(alloc(), ins, ifTrue, ifFalse); + MTest *test = newTest(ins, ifTrue, ifFalse); current->end(test); // The bytecode for if/ternary gets emitted either like this: @@ -3645,7 +3644,7 @@ IonBuilder::jsop_try() // Add MTest(true, tryBlock, successorBlock). MConstant *true_ = MConstant::New(alloc(), BooleanValue(true)); current->add(true_); - current->end(MTest::New(alloc(), true_, tryBlock, successor)); + current->end(newTest(true_, tryBlock, successor)); } else { successor = nullptr; current->end(MGoto::New(alloc(), tryBlock)); @@ -6005,6 +6004,14 @@ IonBuilder::newPendingLoopHeader(MBasicBlock *predecessor, jsbytecode *pc, bool return block; } +MTest * +IonBuilder::newTest(MDefinition *ins, MBasicBlock *ifTrue, MBasicBlock *ifFalse) +{ + MTest *test = MTest::New(alloc(), ins, ifTrue, ifFalse); + test->cacheOperandMightEmulateUndefined(); + return test; +} + // A resume point is a mapping of stack slots to MDefinitions. It is used to // capture the environment such that if a guard fails, and IonMonkey needs // to exit back to the interpreter, the interpreter state can be @@ -8284,7 +8291,7 @@ IonBuilder::jsop_not() MNot *ins = MNot::New(alloc(), value); current->add(ins); current->push(ins); - ins->infer(); + ins->cacheOperandMightEmulateUndefined(); return true; } @@ -9772,7 +9779,7 @@ IonBuilder::jsop_typeof() MDefinition *input = current->pop(); MTypeOf *ins = MTypeOf::New(alloc(), input, input->type()); - ins->infer(); + ins->cacheInputMaybeCallableOrEmulatesUndefined(); current->add(ins); current->push(ins); diff --git a/js/src/jit/IonBuilder.h b/js/src/jit/IonBuilder.h index ac758f0af88..e4ce94e993a 100644 --- a/js/src/jit/IonBuilder.h +++ b/js/src/jit/IonBuilder.h @@ -289,6 +289,14 @@ class IonBuilder : public MIRGenerator return newBlockAfter(at, nullptr, pc); } + // We want to make sure that our MTest instructions all check whether the + // thing being tested might emulate undefined. So we funnel their creation + // through this method, to make sure that happens. We don't want to just do + // the check in MTest::New, because that can run on background compilation + // threads, and we're not sure it's safe to touch that part of the typeset + // from a background thread. + MTest *newTest(MDefinition *ins, MBasicBlock *ifTrue, MBasicBlock *ifFalse); + // Given a list of pending breaks, creates a new block and inserts a Goto // linking each break to the new block. MBasicBlock *createBreakCatchBlock(DeferredEdge *edge, jsbytecode *pc); diff --git a/js/src/jit/MCallOptimize.cpp b/js/src/jit/MCallOptimize.cpp index 3b7fb75a39c..875d35194c2 100644 --- a/js/src/jit/MCallOptimize.cpp +++ b/js/src/jit/MCallOptimize.cpp @@ -1637,10 +1637,10 @@ IonBuilder::inlineHasClasses(CallInfo &callInfo, const Class *clasp1, const Clas current->add(either); // Convert to bool with the '!!' idiom MNot *resultInverted = MNot::New(alloc(), either); - resultInverted->infer(); + resultInverted->cacheOperandMightEmulateUndefined(); current->add(resultInverted); MNot *result = MNot::New(alloc(), resultInverted); - result->infer(); + result->cacheOperandMightEmulateUndefined(); current->add(result); current->push(result); } diff --git a/js/src/jit/MIR.cpp b/js/src/jit/MIR.cpp index 6d1815dbea6..9bb99a44c2e 100644 --- a/js/src/jit/MIR.cpp +++ b/js/src/jit/MIR.cpp @@ -230,7 +230,7 @@ MTest::New(TempAllocator &alloc, MDefinition *ins, MBasicBlock *ifTrue, MBasicBl } void -MTest::infer() +MTest::cacheOperandMightEmulateUndefined() { JS_ASSERT(operandMightEmulateUndefined()); @@ -2174,7 +2174,7 @@ MTypeOf::foldsTo(TempAllocator &alloc, bool useValueNumbers) } void -MTypeOf::infer() +MTypeOf::cacheInputMaybeCallableOrEmulatesUndefined() { JS_ASSERT(inputMaybeCallableOrEmulatesUndefined()); @@ -2669,7 +2669,7 @@ MCompare::filtersUndefinedOrNull(bool trueBranch, MDefinition **subject, bool *f } void -MNot::infer() +MNot::cacheOperandMightEmulateUndefined() { JS_ASSERT(operandMightEmulateUndefined()); diff --git a/js/src/jit/MIR.h b/js/src/jit/MIR.h index a8425faf515..1a434b88e42 100644 --- a/js/src/jit/MIR.h +++ b/js/src/jit/MIR.h @@ -1304,7 +1304,13 @@ class MTest AliasSet getAliasSet() const { return AliasSet::None(); } - void infer(); + + // We cache whether our operand might emulate undefined, but we don't want + // to do that from New() or the constructor, since those can be called on + // background threads. So make callers explicitly call it if they want us + // to check whether the operand might do this. If this method is never + // called, we'll assume our operand can emulate undefined. + void cacheOperandMightEmulateUndefined(); MDefinition *foldsTo(TempAllocator &alloc, bool useValueNumbers); void filtersUndefinedOrNull(bool trueBranch, MDefinition **subject, bool *filtersUndefined, bool *filtersNull); @@ -3272,7 +3278,7 @@ class MTypeOf } MDefinition *foldsTo(TempAllocator &alloc, bool useValueNumbers); - void infer(); + void cacheInputMaybeCallableOrEmulatesUndefined(); bool inputMaybeCallableOrEmulatesUndefined() const { return inputMaybeCallableOrEmulatesUndefined_; @@ -5839,7 +5845,7 @@ class MNot INSTRUCTION_HEADER(Not); - void infer(); + void cacheOperandMightEmulateUndefined(); MDefinition *foldsTo(TempAllocator &alloc, bool useValueNumbers); void markOperandCantEmulateUndefined() {