Bug 777776 - Properly barrier JSScript::scriptSource_. r=terrence
authorBenjamin Peterson <benjamin@python.org>
Sat, 28 Jul 2012 15:18:33 -0700
changeset 100830 b6ac2095d264ad64b33c733563ae8d9c0f73e96a
parent 100829 87693d37bff2c1be6adb65e9ec7fcf127061e693
child 100831 c92a53bb1cba9c893b88223422e439d450aa2a30
push id23195
push userryanvm@gmail.com
push dateSun, 29 Jul 2012 22:23:22 +0000
treeherdermozilla-central@36c30260e7fa [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersterrence
bugs777776
milestone17.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 777776 - Properly barrier JSScript::scriptSource_. r=terrence
js/src/frontend/BytecodeEmitter.cpp
js/src/jit-test/tests/basic/bug777776.js
js/src/jsfun.cpp
js/src/jsscript.cpp
js/src/jsscript.h
--- a/js/src/frontend/BytecodeEmitter.cpp
+++ b/js/src/frontend/BytecodeEmitter.cpp
@@ -4859,17 +4859,17 @@ EmitFunc(JSContext *cx, BytecodeEmitter 
         CompileOptions options(cx);
         options.setPrincipals(parent->principals)
                .setOriginPrincipals(parent->originPrincipals)
                .setCompileAndGo(parent->compileAndGo)
                .setNoScriptRval(false)
                .setVersion(parent->getVersion());
         Rooted<JSScript*> script(cx, JSScript::Create(cx, enclosingScope, false, options,
                                                       parent->staticLevel + 1,
-                                                      bce->script->source,
+                                                      bce->script->scriptSource(),
                                                       funbox->bufStart, funbox->bufEnd));
         if (!script)
             return false;
 
         BytecodeEmitter bce2(bce, bce->parser, &sc, script, bce->callerFrame, bce->hasGlobalScope,
                              pn->pn_pos.begin.lineno);
         if (!bce2.init())
             return false;
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/basic/bug777776.js
@@ -0,0 +1,5 @@
+gczeal(9, 2)
+for (a = 0; a < 4; ++a) {
+    b =
+    evaluate("/x/g");
+}
--- a/js/src/jsfun.cpp
+++ b/js/src/jsfun.cpp
@@ -598,31 +598,31 @@ js::FunctionToString(JSContext *cx, Hand
         if (!out.append("function "))
             return NULL;
         if (fun->atom) {
             if (!out.append(fun->atom))
                 return NULL;
         }
     }
     bool haveSource = fun->isInterpreted();
-    if (haveSource && !fun->script()->source && !fun->script()->loadSource(cx, &haveSource))
+    if (haveSource && !fun->script()->scriptSource() && !fun->script()->loadSource(cx, &haveSource))
             return NULL;
     if (haveSource) {
         RootedScript script(cx, fun->script());
         RootedString src(cx, fun->script()->sourceData(cx));
         if (!src)
             return NULL;
         const jschar *chars = src->getChars(cx);
         if (!chars)
             return NULL;
         bool exprBody = fun->flags & JSFUN_EXPR_CLOSURE;
 
         // The source data for functions created by calling the Function
         // constructor is only the function's body.
-        bool funCon = script->sourceStart == 0 && script->source->argumentsNotIncluded();
+        bool funCon = script->sourceStart == 0 && script->scriptSource()->argumentsNotIncluded();
 
         // Functions created with the constructor should not be using the
         // expression body extension.
         JS_ASSERT_IF(funCon, !exprBody);
         JS_ASSERT_IF(!funCon, src->length() > 0 && chars[0] == '(');
 
         // If a function inherits strict mode by having scopes above it that
         // have "use strict", we insert "use strict" into the body of the
--- a/js/src/jsscript.cpp
+++ b/js/src/jsscript.cpp
@@ -514,19 +514,19 @@ js::XDRScript(XDRState<mode> *xdr, Handl
             scriptBits |= (1 << ArgumentsHasVarBinding);
         if (script->analyzedArgsUsage() && script->needsArgsObj())
             scriptBits |= (1 << NeedsArgsObj);
         if (script->filename) {
             scriptBits |= (enclosingScript && enclosingScript->filename == script->filename)
                           ? (1 << ParentFilename)
                           : (1 << OwnFilename);
         }
-        if (script->source) {
+        if (script->scriptSource()) {
             scriptBits |= (1 << HaveSource);
-            if (!enclosingScript || enclosingScript->source != script->source)
+            if (!enclosingScript || enclosingScript->scriptSource() != script->scriptSource())
                 scriptBits |= (1 << OwnSource);
         }
         if (script->isGenerator)
             scriptBits |= (1 << IsGenerator);
         if (script->isGeneratorExp)
             scriptBits |= (1 << IsGeneratorExp);
 
         JS_ASSERT(!script->compileAndGo);
@@ -630,27 +630,29 @@ js::XDRScript(XDRState<mode> *xdr, Handl
         }
     } else if (scriptBits & (1 << ParentFilename)) {
         JS_ASSERT(enclosingScript);
         if (mode == XDR_DECODE)
             script->filename = enclosingScript->filename;
     }
 
     if (scriptBits & (1 << HaveSource)) {
+        ScriptSource *ss = script->scriptSource();
         if (scriptBits & (1 << OwnSource)) {
-            if (!ScriptSource::performXDR<mode>(xdr, &script->source))
+            if (!ScriptSource::performXDR<mode>(xdr, &ss))
                 return false;
         } else {
             JS_ASSERT(enclosingScript);
-            if (mode == XDR_DECODE)
-                script->source = enclosingScript->source;
+            ss = enclosingScript->scriptSource();
         }
+        if (mode == XDR_DECODE)
+            script->setScriptSource(cx, ss);
     } else if (mode == XDR_DECODE) {
-        script->source = NULL;
-        JS_ASSERT_IF(enclosingScript, !enclosingScript->source);
+        script->setScriptSource(cx, NULL);
+        JS_ASSERT_IF(enclosingScript, !enclosingScript->scriptSource());
     }
     if (!xdr->codeUint32(&script->sourceStart))
         return false;
     if (!xdr->codeUint32(&script->sourceEnd))
         return false;
 
     if (mode == XDR_DECODE) {
         script->lineno = lineno;
@@ -1066,45 +1068,56 @@ SourceCompressorThread::waitOnCompressio
     JS_ASSERT(tok->ss->ready());
     tok->ss = NULL;
     tok->chars = NULL;
     tok = NULL;
     PR_Unlock(lock);
 }
 #endif /* JS_THREADSAFE */
 
+void
+JSScript::setScriptSource(JSContext *cx, ScriptSource *ss)
+{
+#ifdef JSGC_INCREMENTAL
+    // During IGC, we need to barrier writing to scriptSource_.
+    if (ss && cx->runtime->gcIncrementalState != NO_INCREMENTAL && cx->runtime->gcIsFull)
+        ss->mark();
+#endif
+    scriptSource_ = ss;
+}
+
 bool
 JSScript::loadSource(JSContext *cx, bool *worked)
 {
-    JS_ASSERT(!source);
+    JS_ASSERT(!scriptSource_);
     *worked = false;
     if (!cx->runtime->sourceHook)
         return true;
     jschar *src = NULL;
     uint32_t length;
     if (!cx->runtime->sourceHook(cx, this, &src, &length))
         return false;
     if (!src)
         return true;
     ScriptSource *ss = ScriptSource::createFromSource(cx, src, length, false, NULL, true);
     if (!ss) {
         cx->free_(src);
         return false;
     }
-    source = ss;
+    setScriptSource(cx, ss);
     ss->attachToRuntime(cx->runtime);
     *worked = true;
     return true;
 }
 
 JSFixedString *
 JSScript::sourceData(JSContext *cx)
 {
-    JS_ASSERT(source);
-    return source->substring(cx, sourceStart, sourceEnd);
+    JS_ASSERT(scriptSource_);
+    return scriptSource_->substring(cx, sourceStart, sourceEnd);
 }
 
 JSFixedString *
 SourceDataCache::lookup(ScriptSource *ss)
 {
     if (!map_)
         return NULL;
     if (Map::Ptr p = map_->lookup(ss))
@@ -1194,26 +1207,16 @@ ScriptSource::createFromSource(JSContext
     ss->length_ = length;
     ss->compressedLength = 0;
     ss->marked = ss->onRuntime_ = false;
     ss->argumentsNotIncluded_ = argumentsNotIncluded;
 #ifdef DEBUG
     ss->ready_ = false;
 #endif
 
-#ifdef JSGC_INCREMENTAL
-    /*
-     * During the IGC we need to ensure that source is marked whenever it is
-     * accessed even if the name was already in the table. At this point old
-     * scripts pointing to the source may no longer be reachable.
-     */
-    if (cx->runtime->gcIncrementalState != NO_INCREMENTAL && cx->runtime->gcIsFull)
-        ss->marked = true;
-#endif
-
     JS_ASSERT_IF(ownSource, !tok);
 
 #ifdef JS_THREADSAFE
     if (tok && 0) {
         tok->ss = ss;
         tok->chars = src;
         cx->runtime->sourceCompressorThread.compress(tok);
     } else
@@ -1293,16 +1296,17 @@ ScriptSource::sizeOfIncludingThis(JSMall
     // data is a union, but both members are pointers to allocated memory, so
     // just using compressed will work.
     return mallocSizeOf(this) + mallocSizeOf(data.compressed);
 }
 
 void
 ScriptSource::sweep(JSRuntime *rt)
 {
+    JS_ASSERT(rt->gcIsFull);
     ScriptSource *next = rt->scriptSources, **prev = &rt->scriptSources;
     while (next) {
         ScriptSource *cur = next;
         next = cur->next;
         JS_ASSERT(cur->ready());
         JS_ASSERT(cur->onRuntime());
         if (cur->marked) {
             cur->marked = false;
@@ -1595,17 +1599,17 @@ JSScript::Create(JSContext *cx, HandleOb
     // stack if we nest functions more than a few hundred deep, so this will
     // never trigger.  Oh well.
     if (staticLevel > UINT16_MAX) {
         JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, JSMSG_TOO_DEEP, js_function_str);
         return NULL;
     }
     script->staticLevel = uint16_t(staticLevel);
 
-    script->source = ss;
+    script->setScriptSource(cx, ss);
     script->sourceStart = bufStart;
     script->sourceEnd = bufEnd;
 
     return script;
 }
 
 static inline uint8_t *
 AllocScriptData(JSContext *cx, size_t size)
@@ -2259,17 +2263,17 @@ js::CloneScript(JSContext *cx, HandleObj
     CompileOptions options(cx);
     options.setPrincipals(cx->compartment->principals)
            .setOriginPrincipals(src->originPrincipals)
            .setCompileAndGo(src->compileAndGo)
            .setNoScriptRval(src->noScriptRval)
            .setVersion(src->getVersion());
     JSScript *dst = JSScript::Create(cx, enclosingScope, src->savedCallerFun,
                                      options, src->staticLevel,
-                                     src->source, src->sourceStart, src->sourceEnd);
+                                     src->scriptSource(), src->sourceStart, src->sourceEnd);
     if (!dst) {
         Foreground::free_(data);
         return NULL;
     }
 
     new (&dst->bindings) Bindings;
     dst->bindings.transfer(&bindings);
 
@@ -2592,18 +2596,18 @@ JSScript::markChildren(JSTracer *trc)
 
     if (enclosingScope_)
         MarkObject(trc, &enclosingScope_, "enclosing");
 
     if (IS_GC_MARKING_TRACER(trc)) {
         if (filename)
             MarkScriptFilename(trc->runtime, filename);
 
-        if (trc->runtime->gcIsFull && source && source->onRuntime())
-            source->mark();
+        if (trc->runtime->gcIsFull && scriptSource_)
+            scriptSource_->mark();
     }
 
     bindings.trace(trc);
 
 #ifdef JS_METHODJIT
     for (int constructing = 0; constructing <= 1; constructing++) {
         for (int barriers = 0; barriers <= 1; barriers++) {
             mjit::JITScript *jit = getJIT((bool) constructing, (bool) barriers);
--- a/js/src/jsscript.h
+++ b/js/src/jsscript.h
@@ -432,19 +432,18 @@ struct JSScript : public js::gc::Cell
     js::HeapPtrAtom *atoms;     /* maps immediate index to literal struct */
 
     JSPrincipals    *principals;/* principals for this script */
     JSPrincipals    *originPrincipals; /* see jsapi.h 'originPrincipals' comment */
 
     /* Persistent type information retained across GCs. */
     js::types::TypeScript *types;
 
-    js::ScriptSource *source; /* source code */
-
   private:
+    js::ScriptSource *scriptSource_; /* source code */
 #ifdef JS_METHODJIT
     JITScriptSet *mJITInfo;
 #endif
     js::HeapPtrFunction function_;
     js::HeapPtrObject   enclosingScope_;
 
     // 32-bit fields.
 
@@ -616,16 +615,24 @@ struct JSScript : public js::gc::Cell
      */
     JSFunction *function() const { return function_; }
     void setFunction(JSFunction *fun);
 
     JSFixedString *sourceData(JSContext *cx);
 
     bool loadSource(JSContext *cx, bool *worked);
 
+    js::ScriptSource *scriptSource() {
+        return scriptSource_;
+    }
+
+    void setScriptSource(JSContext *cx, js::ScriptSource *ss);
+
+  public:
+
     /* Return whether this script was compiled for 'eval' */
     bool isForEval() { return isCachedEval || isActiveEval; }
 
 #ifdef DEBUG
     unsigned id();
 #else
     unsigned id() { return 0; }
 #endif
@@ -992,17 +999,17 @@ struct ScriptSource
   public:
     static ScriptSource *createFromSource(JSContext *cx,
                                           const jschar *src,
                                           uint32_t length,
                                           bool argumentsNotIncluded = false,
                                           SourceCompressionToken *tok = NULL,
                                           bool ownSource = false);
     void attachToRuntime(JSRuntime *rt);
-    void mark() { JS_ASSERT(ready_); JS_ASSERT(onRuntime_); marked = true; }
+    void mark() { marked = true; }
     void destroy(JSRuntime *rt);
     uint32_t length() const { return length_; }
     bool onRuntime() const { return onRuntime_; }
     bool argumentsNotIncluded() const { return argumentsNotIncluded_; }
 #ifdef DEBUG
     bool ready() const { return ready_; }
 #endif
     JSFixedString *substring(JSContext *cx, uint32_t start, uint32_t stop);