Followup fix for 497789: work around apparent gcc 4.4 aliasing bug (r=dvander).
authorBrendan Eich <brendan@mozilla.org>
Mon, 22 Mar 2010 16:26:28 -0700
changeset 40331 d412747189f623a6bfc8f1e454eee95bb7d50383
parent 40330 b63d655726dd189a60b2302754a7ccc25ca136d4
child 40332 9ab9132b1055e71aff108122f80f9a85d63379a1
push id1
push userroot
push dateTue, 26 Apr 2011 22:38:44 +0000
treeherdermozilla-beta@bfdb6e623a36 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdvander
bugs497789
milestone1.9.3a3pre
Followup fix for 497789: work around apparent gcc 4.4 aliasing bug (r=dvander).
js/src/jspropertytree.cpp
js/src/jsscope.h
--- a/js/src/jspropertytree.cpp
+++ b/js/src/jspropertytree.cpp
@@ -142,65 +142,29 @@ PropertyTree::finish()
     if (hash.ops) {
         JS_DHashTableFinish(&hash);
         hash.ops = NULL;
     }
     JS_FinishArenaPool(&arenaPool);
 }
 
 /*
- * A property tree node on PropertyTree::freeList overlays the following prefix
- * struct on JSScopeProperty.
- */
-typedef struct FreeNode {
-    jsid        id;
-    FreeNode    *next;
-    FreeNode    **prevp;
-
-    void insert(FreeNode *&list) {
-        next = list;
-        prevp = &list;
-        if (list)
-            list->prevp = &next;
-        list = this;
-    }
-
-    void remove() {
-        *prevp = next;
-        if (next)
-            next->prevp = prevp;
-    }
-} FreeNode;
-
-#define FREENODE(sprop) ((FreeNode *) (sprop))
-
-#define FREENODE_INSERT(list, sprop)                                          \
-    JS_BEGIN_MACRO                                                            \
-        union { FreeNode *fn; JSScopeProperty *sp; } u;                       \
-        u.fn = (FreeNode *)(list);                                            \
-        FREENODE(sprop)->insert(u.fn);                                        \
-    JS_END_MACRO
-
-#define FREENODE_REMOVE(sprop)                                                \
-    (FREENODE(sprop)->remove())
-
-/*
  * NB: Called with cx->runtime->gcLock held if gcLocked is true.
  * On failure, return null after unlocking the GC and reporting out of memory.
  */
 JSScopeProperty *
 PropertyTree::newScopeProperty(JSContext *cx, bool gcLocked)
 {
     JSScopeProperty *sprop;
 
     if (!gcLocked)
         JS_LOCK_GC(cx->runtime);
     sprop = freeList;
     if (sprop) {
-        FREENODE_REMOVE(sprop);
+        sprop->removeFree();
     } else {
         JS_ARENA_ALLOCATE_CAST(sprop, JSScopeProperty *, &arenaPool,
                                sizeof(JSScopeProperty));
         if (!sprop) {
             JS_UNLOCK_GC(cx->runtime);
             JS_ReportOutOfMemory(cx);
             return NULL;
         }
@@ -790,19 +754,16 @@ js::RemoveNodeIfDead(JSDHashTable *table
         return JS_DHASH_REMOVE;
     }
     return JS_DHASH_NEXT;
 }
 
 void
 js::SweepScopeProperties(JSContext *cx)
 {
-    JS_STATIC_ASSERT(offsetof(FreeNode, id) == offsetof(JSScopeProperty, id));
-    JS_STATIC_ASSERT(sizeof(FreeNode) >= offsetof(JSScopeProperty, slot));
-
 #ifdef DEBUG
     JSBasicStats bs;
     uint32 livePropCapacity = 0, totalLiveCount = 0;
     static FILE *logfp;
     if (!logfp) {
         if (const char *filename = getenv("JS_PROPTREE_STATFILE"))
             logfp = fopen(filename, "w");
     }
@@ -934,26 +895,28 @@ js::SweepScopeProperties(JSContext *cx)
                  */
                 if (sprop->parent)
                     JS_PROPERTY_TREE(cx).removeChild(cx, sprop);
 
                 if (sprop->kids)
                     OrphanNodeKids(cx, sprop);
             }
 
-            /* Clear id so we know (above) that sprop is on the freelist. */
-            sprop->id = JSVAL_NULL;
-            FREENODE_INSERT(JS_PROPERTY_TREE(cx).freeList, sprop);
+            /*
+             * Note that JSScopeProperty::insertFree nulls sprop->id so we know
+             * that sprop is on the freelist.
+             */
+            sprop->insertFree(JS_PROPERTY_TREE(cx).freeList);
             JS_RUNTIME_UNMETER(cx->runtime, livePropTreeNodes);
         }
 
         /* If a contains no live properties, return it to the malloc heap. */
         if (liveCount == 0) {
             for (JSScopeProperty *sprop = (JSScopeProperty *) a->base; sprop < limit; sprop++)
-                FREENODE_REMOVE(sprop);
+                sprop->removeFree();
             JS_ARENA_DESTROY(&JS_PROPERTY_TREE(cx).arenaPool, a, ap);
         } else {
 #ifdef DEBUG
             livePropCapacity += limit - (JSScopeProperty *) a->base;
             totalLiveCount += liveCount;
 #endif
             ap = &a->next;
         }
--- a/js/src/jsscope.h
+++ b/js/src/jsscope.h
@@ -574,20 +574,46 @@ class PropertyTree;
 struct JSScopeProperty {
     friend struct JSScope;
     friend class js::PropertyTree;
     friend JSDHashOperator js::RemoveNodeIfDead(JSDHashTable *table, JSDHashEntryHdr *hdr,
                                                 uint32 number, void *arg);
     friend void js::SweepScopeProperties(JSContext *cx);
 
     jsid            id;                 /* int-tagged jsval/untagged JSAtom* */
+
   private:
-    JSPropertyOp    rawGetter;          /* getter and setter hooks or objects */
-    JSPropertyOp    rawSetter;          /* getter is JSObject* and setter is 0
+    union {
+        JSPropertyOp rawGetter;         /* getter and setter hooks or objects */
+        JSScopeProperty *next;          /* next node in freelist */
+    };
+
+    union {
+        JSPropertyOp rawSetter;         /* getter is JSObject* and setter is 0
                                            if sprop->isMethod() */
+        JSScopeProperty **prevp;        /* pointer to previous node's next, or
+                                           pointer to head of freelist */
+    };
+
+    void insertFree(JSScopeProperty *&list) {
+        id = JSVAL_NULL;
+        next = list;
+        prevp = &list;
+        if (list)
+            list->prevp = &next;
+        list = this;
+    }
+
+    void removeFree() {
+        JS_ASSERT(JSVAL_IS_NULL(id));
+        *prevp = next;
+        if (next)
+            next->prevp = prevp;
+    }
+
   public:
     uint32          slot;               /* abstract index in object slots */
   private:
     uint8           attrs;              /* attributes, see jsapi.h JSPROP_* */
     uint8           flags;              /* flags, see below for defines */
   public:
     int16           shortid;            /* tinyid, or local arg/var index */
     JSScopeProperty *parent;            /* parent node, reverse for..in order */