[Emacs-diffs] /srv/bzr/emacs/trunk r107914: Simplify enforcement of object address alignment

[Andreas Schwab]

------------------------------------------------------------
revno: 107914
committer: Andreas Schwab <address@hidden>
branch nick: emacs
timestamp: Sun 2012-04-15 18:20:54 +0200
message:
  Simplify enforcement of object address alignment
  
  * lisp.h (struct Lisp_Symbol): Remove explicit padding.
  (struct Lisp_Misc_Any): Likewise.
  (struct Lisp_Free): Likewise.
  * alloc.c (union aligned_Lisp_Symbol): Define.
  (SYMBOL_BLOCK_SIZE, struct symbol_block): Use union
  aligned_Lisp_Symbol instead of struct Lisp_Symbol.
  (union aligned_Lisp_Misc): Define.
  (MARKER_BLOCK_SIZE, struct marker_block): Use union
  aligned_Lisp_Misc instead of union Lisp_Misc.
  (Fmake_symbol, allocate_misc, gc_sweep): Adjust
modified:
  src/ChangeLog
  src/alloc.c
  src/lisp.h

=== modified file 'src/ChangeLog'
--- a/src/ChangeLog     2012-04-15 02:59:30 +0000
+++ b/src/ChangeLog     2012-04-15 16:20:54 +0000
@@ -1,3 +1,16 @@
+2012-04-15  Andreas Schwab  <address@hidden>
+
+       * lisp.h (struct Lisp_Symbol): Remove explicit padding.
+       (struct Lisp_Misc_Any): Likewise.
+       (struct Lisp_Free): Likewise.
+       * alloc.c (union aligned_Lisp_Symbol): Define.
+       (SYMBOL_BLOCK_SIZE, struct symbol_block): Use union
+       aligned_Lisp_Symbol instead of struct Lisp_Symbol.
+       (union aligned_Lisp_Misc): Define.
+       (MARKER_BLOCK_SIZE, struct marker_block): Use union
+       aligned_Lisp_Misc instead of union Lisp_Misc.
+       (Fmake_symbol, allocate_misc, gc_sweep): Adjust
+
 2012-04-14  Paul Eggert  <address@hidden>
 
        Make GC_MAKE_GCPROS_NOOPS the default (Bug#9926).

=== modified file 'src/alloc.c'
--- a/src/alloc.c       2012-04-11 00:51:44 +0000
+++ b/src/alloc.c       2012-04-15 16:20:54 +0000
@@ -3136,17 +3136,29 @@
                           Symbol Allocation
  ***********************************************************************/
 
+/* Like struct Lisp_Symbol, but padded so that the size is a multiple
+   of the required alignment if LSB tags are used.  */
+
+union aligned_Lisp_Symbol
+{
+  struct Lisp_Symbol s;
+#ifdef USE_LSB_TAG
+  unsigned char c[(sizeof (struct Lisp_Symbol) + (1 << GCTYPEBITS) - 1)
+                 & -(1 << GCTYPEBITS)];
+#endif
+};
+
 /* Each symbol_block is just under 1020 bytes long, since malloc
    really allocates in units of powers of two and uses 4 bytes for its
    own overhead. */
 
 #define SYMBOL_BLOCK_SIZE \
-  ((1020 - sizeof (struct symbol_block *)) / sizeof (struct Lisp_Symbol))
+  ((1020 - sizeof (struct symbol_block *)) / sizeof (union 
aligned_Lisp_Symbol))
 
 struct symbol_block
 {
   /* Place `symbols' first, to preserve alignment.  */
-  struct Lisp_Symbol symbols[SYMBOL_BLOCK_SIZE];
+  union aligned_Lisp_Symbol symbols[SYMBOL_BLOCK_SIZE];
   struct symbol_block *next;
 };
 
@@ -3202,7 +3214,7 @@
          symbol_block = new;
          symbol_block_index = 0;
        }
-      XSETSYMBOL (val, &symbol_block->symbols[symbol_block_index]);
+      XSETSYMBOL (val, &symbol_block->symbols[symbol_block_index].s);
       symbol_block_index++;
     }
 
@@ -3230,16 +3242,28 @@
                       Marker (Misc) Allocation
  ***********************************************************************/
 
+/* Like union Lisp_Misc, but padded so that its size is a multiple of
+   the required alignment when LSB tags are used.  */
+
+union aligned_Lisp_Misc
+{
+  union Lisp_Misc m;
+#ifdef USE_LSB_TAG
+  unsigned char c[(sizeof (union Lisp_Misc) + (1 << GCTYPEBITS) - 1)
+                 & -(1 << GCTYPEBITS)];
+#endif
+};
+
 /* Allocation of markers and other objects that share that structure.
    Works like allocation of conses. */
 
 #define MARKER_BLOCK_SIZE \
-  ((1020 - sizeof (struct marker_block *)) / sizeof (union Lisp_Misc))
+  ((1020 - sizeof (struct marker_block *)) / sizeof (union aligned_Lisp_Misc))
 
 struct marker_block
 {
   /* Place `markers' first, to preserve alignment.  */
-  union Lisp_Misc markers[MARKER_BLOCK_SIZE];
+  union aligned_Lisp_Misc markers[MARKER_BLOCK_SIZE];
   struct marker_block *next;
 };
 
@@ -3284,7 +3308,7 @@
          marker_block_index = 0;
          total_free_markers += MARKER_BLOCK_SIZE;
        }
-      XSETMISC (val, &marker_block->markers[marker_block_index]);
+      XSETMISC (val, &marker_block->markers[marker_block_index].m);
       marker_block_index++;
     }
 
@@ -6070,22 +6094,22 @@
     for (sblk = symbol_block; sblk; sblk = *sprev)
       {
        int this_free = 0;
-       struct Lisp_Symbol *sym = sblk->symbols;
-       struct Lisp_Symbol *end = sym + lim;
+       union aligned_Lisp_Symbol *sym = sblk->symbols;
+       union aligned_Lisp_Symbol *end = sym + lim;
 
        for (; sym < end; ++sym)
          {
            /* Check if the symbol was created during loadup.  In such a case
               it might be pointed to by pure bytecode which we don't trace,
               so we conservatively assume that it is live.  */
-           int pure_p = PURE_POINTER_P (XSTRING (sym->xname));
+           int pure_p = PURE_POINTER_P (XSTRING (sym->s.xname));
 
-           if (!sym->gcmarkbit && !pure_p)
+           if (!sym->s.gcmarkbit && !pure_p)
              {
-               if (sym->redirect == SYMBOL_LOCALIZED)
-                 xfree (SYMBOL_BLV (sym));
-               sym->next = symbol_free_list;
-               symbol_free_list = sym;
+               if (sym->s.redirect == SYMBOL_LOCALIZED)
+                 xfree (SYMBOL_BLV (&sym->s));
+               sym->s.next = symbol_free_list;
+               symbol_free_list = &sym->s;
 #if GC_MARK_STACK
                symbol_free_list->function = Vdead;
 #endif
@@ -6095,8 +6119,8 @@
              {
                ++num_used;
                if (!pure_p)
-                 UNMARK_STRING (XSTRING (sym->xname));
-               sym->gcmarkbit = 0;
+                 UNMARK_STRING (XSTRING (sym->s.xname));
+               sym->s.gcmarkbit = 0;
              }
          }
 
@@ -6108,7 +6132,7 @@
          {
            *sprev = sblk->next;
            /* Unhook from the free list.  */
-           symbol_free_list = sblk->symbols[0].next;
+           symbol_free_list = sblk->symbols[0].s.next;
            lisp_free (sblk);
          }
        else
@@ -6138,22 +6162,22 @@
 
        for (i = 0; i < lim; i++)
          {
-           if (!mblk->markers[i].u_any.gcmarkbit)
+           if (!mblk->markers[i].m.u_any.gcmarkbit)
              {
-               if (mblk->markers[i].u_any.type == Lisp_Misc_Marker)
-                 unchain_marker (&mblk->markers[i].u_marker);
+               if (mblk->markers[i].m.u_any.type == Lisp_Misc_Marker)
+                 unchain_marker (&mblk->markers[i].m.u_marker);
                /* Set the type of the freed object to Lisp_Misc_Free.
                   We could leave the type alone, since nobody checks it,
                   but this might catch bugs faster.  */
-               mblk->markers[i].u_marker.type = Lisp_Misc_Free;
-               mblk->markers[i].u_free.chain = marker_free_list;
-               marker_free_list = &mblk->markers[i];
+               mblk->markers[i].m.u_marker.type = Lisp_Misc_Free;
+               mblk->markers[i].m.u_free.chain = marker_free_list;
+               marker_free_list = &mblk->markers[i].m;
                this_free++;
              }
            else
              {
                num_used++;
-               mblk->markers[i].u_any.gcmarkbit = 0;
+               mblk->markers[i].m.u_any.gcmarkbit = 0;
              }
          }
        lim = MARKER_BLOCK_SIZE;
@@ -6164,7 +6188,7 @@
          {
            *mprev = mblk->next;
            /* Unhook from the free list.  */
-           marker_free_list = mblk->markers[0].u_free.chain;
+           marker_free_list = mblk->markers[0].m.u_free.chain;
            lisp_free (mblk);
          }
        else

=== modified file 'src/lisp.h'
--- a/src/lisp.h        2012-04-14 06:18:49 +0000
+++ b/src/lisp.h        2012-04-15 16:20:54 +0000
@@ -1134,8 +1134,6 @@
      special (with `defvar' etc), and shouldn't be lexically bound.  */
   unsigned declared_special : 1;
 
-  unsigned spacer : 23;
-
   /* The symbol's name, as a Lisp string.
      The name "xname" is used to intentionally break code referring to
      the old field "name" of type pointer to struct Lisp_String.  */
@@ -1337,8 +1335,6 @@
   ENUM_BF (Lisp_Misc_Type) type : 16;          /* = Lisp_Misc_??? */
   unsigned gcmarkbit : 1;
   int spacer : 15;
-  /* Make it as long as "Lisp_Free without padding".  */
-  void *fill;
 };
 
 struct Lisp_Marker
@@ -1530,13 +1526,6 @@
     unsigned gcmarkbit : 1;
     int spacer : 15;
     union Lisp_Misc *chain;
-#ifdef USE_LSB_TAG
-    /* Try to make sure that sizeof(Lisp_Misc) preserves TYPEBITS-alignment.
-       This assumes that Lisp_Marker is the largest of the alternatives and
-       that Lisp_Misc_Any has the same size as "Lisp_Free w/o padding".  */
-    char padding[((((sizeof (struct Lisp_Marker) - 1) >> GCTYPEBITS) + 1)
-                 << GCTYPEBITS) - sizeof (struct Lisp_Misc_Any)];
-#endif
   };
 
 /* To get the type field of a union Lisp_Misc, use XMISCTYPE.
@@ -1545,19 +1534,19 @@
 union Lisp_Misc
   {
     struct Lisp_Misc_Any u_any;           /* Supertype of all Misc types.  */
-    struct Lisp_Free u_free;      /* Includes padding to force alignment.  */
-    struct Lisp_Marker u_marker;                        /* 5 */
-    struct Lisp_Overlay u_overlay;                      /* 5 */
-    struct Lisp_Save_Value u_save_value;                /* 3 */
+    struct Lisp_Free u_free;
+    struct Lisp_Marker u_marker;
+    struct Lisp_Overlay u_overlay;
+    struct Lisp_Save_Value u_save_value;
   };
 
 union Lisp_Fwd
   {
-    struct Lisp_Intfwd u_intfwd;                        /* 2 */
-    struct Lisp_Boolfwd u_boolfwd;                      /* 2 */
-    struct Lisp_Objfwd u_objfwd;                        /* 2 */
-    struct Lisp_Buffer_Objfwd u_buffer_objfwd;          /* 2 */
-    struct Lisp_Kboard_Objfwd u_kboard_objfwd;          /* 2 */
+    struct Lisp_Intfwd u_intfwd;
+    struct Lisp_Boolfwd u_boolfwd;
+    struct Lisp_Objfwd u_objfwd;
+    struct Lisp_Buffer_Objfwd u_buffer_objfwd;
+    struct Lisp_Kboard_Objfwd u_kboard_objfwd;
   };
 
 /* Lisp floating point type */