all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Paul Eggert <eggert@cs.ucla.edu>
To: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>,
	35711@debbugs.gnu.org
Cc: Andreas Schwab <schwab@suse.de>,
	Michael Karcher <debian@mkarcher.dialup.fu-berlin.de>
Subject: bug#35711: emacs crashes on m68k after d2f1971dd5
Date: Mon, 13 May 2019 12:47:44 -0700	[thread overview]
Message-ID: <98a8bb69-486e-20b2-891d-65015bab9579@cs.ucla.edu> (raw)
In-Reply-To: <f937020d-0922-841d-1b73-71daf7d5a5bf@physik.fu-berlin.de>

[-- Attachment #1: Type: text/plain, Size: 179 bytes --]

Thanks for the bug report. Please try the attached, more-general patch,
which I've installed on master. I suppose I should backport it to the
emacs-26 branch if it works for you.

[-- Attachment #2: 0001-Fix-broken-build-on-m68k.patch --]
[-- Type: text/x-patch, Size: 5634 bytes --]

From 5190acc14f58d5f96887b1d9b6981aa1c48ced6f Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Mon, 13 May 2019 12:43:13 -0700
Subject: [PATCH] Fix broken build on m68k

The GCC + valgrind fix caused the m68k build to fail (Bug#35711).
Simplify string allocation a bit to make similar problems less
likely in the future.
* src/alloc.c (sdata, SDATA_NBYTES, SDATA_DATA) [GC_CHECK_STRING_BYTES]:
Use the same implementation as with !GC_CHECK_STRING_BYTES,
as the special case is no longer needed.
(SDATA_ALIGN): New constant.
(SDATA_SIZE): Remove this macro, replacing with ...
(sdata_size): ... this new function.  All uses changed.
Properly account for sizes and alignments even in the m68k case,
and even if GC_CHECK_STRING_BYTES is not defined.
---
 src/alloc.c | 77 +++++++++++++++++------------------------------------
 1 file changed, 25 insertions(+), 52 deletions(-)

diff --git a/src/alloc.c b/src/alloc.c
index 948a0e8a2d..af4adb3856 100644
--- a/src/alloc.c
+++ b/src/alloc.c
@@ -1447,9 +1447,7 @@ mark_interval_tree (INTERVAL i)
 
 #define LARGE_STRING_BYTES 1024
 
-/* The SDATA typedef is a struct or union describing string memory
-   sub-allocated from an sblock.  This is where the contents of Lisp
-   strings are stored.  */
+/* The layout of a nonnull string.  */
 
 struct sdata
 {
@@ -1468,13 +1466,8 @@ struct sdata
   unsigned char data[FLEXIBLE_ARRAY_MEMBER];
 };
 
-#ifdef GC_CHECK_STRING_BYTES
-
-typedef struct sdata sdata;
-#define SDATA_NBYTES(S)	(S)->nbytes
-#define SDATA_DATA(S)	(S)->data
-
-#else
+/* A union describing string memory sub-allocated from an sblock.
+   This is where the contents of Lisp strings are stored.  */
 
 typedef union
 {
@@ -1502,8 +1495,6 @@ typedef union
 #define SDATA_NBYTES(S)	(S)->n.nbytes
 #define SDATA_DATA(S)	((struct sdata *) (S))->data
 
-#endif /* not GC_CHECK_STRING_BYTES */
-
 enum { SDATA_DATA_OFFSET = offsetof (struct sdata, data) };
 
 /* Structure describing a block of memory which is sub-allocated to
@@ -1586,31 +1577,20 @@ static char const string_overrun_cookie[GC_STRING_OVERRUN_COOKIE_SIZE] =
 # define GC_STRING_OVERRUN_COOKIE_SIZE 0
 #endif
 
-/* Value is the size of an sdata structure large enough to hold NBYTES
-   bytes of string data.  The value returned includes a terminating
-   NUL byte, the size of the sdata structure, and padding.  */
-
-#ifdef GC_CHECK_STRING_BYTES
-
-#define SDATA_SIZE(NBYTES) FLEXSIZEOF (struct sdata, data, (NBYTES) + 1)
+/* Return the size of an sdata structure large enough to hold N bytes
+   of string data.  This counts the sdata structure, the N bytes, a
+   terminating NUL byte, and alignment padding.  */
 
-#else /* not GC_CHECK_STRING_BYTES */
-
-/* The 'max' reserves space for the nbytes union member even when NBYTES + 1 is
-   less than the size of that member.  The 'max' is not needed when
-   SDATA_DATA_OFFSET is a multiple of FLEXALIGNOF (struct sdata),
-   because then the alignment code reserves enough space.  */
-
-#define SDATA_SIZE(NBYTES)				      \
-     ((SDATA_DATA_OFFSET				      \
-       + (SDATA_DATA_OFFSET % FLEXALIGNOF (struct sdata) == 0 \
-	  ? NBYTES					      \
-	  : max (NBYTES, FLEXALIGNOF (struct sdata) - 1))     \
-       + 1						      \
-       + FLEXALIGNOF (struct sdata) - 1)		      \
-      & ~(FLEXALIGNOF (struct sdata) - 1))
-
-#endif /* not GC_CHECK_STRING_BYTES */
+static ptrdiff_t
+sdata_size (ptrdiff_t n)
+{
+  /* Reserve space for the nbytes union member even when N + 1 is less
+     than the size of that member.  */
+  ptrdiff_t unaligned_size = max (SDATA_DATA_OFFSET + n + 1,
+				  sizeof (sdata));
+  int sdata_align = max (FLEXALIGNOF (struct sdata), alignof (sdata));
+  return (unaligned_size + sdata_align - 1) & ~(sdata_align - 1);
+}
 
 /* Extra bytes to allocate for each string.  */
 #define GC_STRING_EXTRA GC_STRING_OVERRUN_COOKIE_SIZE
@@ -1664,21 +1644,14 @@ string_bytes (struct Lisp_String *s)
 static void
 check_sblock (struct sblock *b)
 {
-  sdata *from, *end, *from_end;
-
-  end = b->next_free;
+  sdata *end = b->next_free;
 
-  for (from = b->data; from < end; from = from_end)
+  for (sdata *from = b->data; from < end; )
     {
-      /* Compute the next FROM here because copying below may
-	 overwrite data we need to compute it.  */
-      ptrdiff_t nbytes;
-
-      /* Check that the string size recorded in the string is the
-	 same as the one recorded in the sdata structure.  */
-      nbytes = SDATA_SIZE (from->string ? string_bytes (from->string)
-			   : SDATA_NBYTES (from));
-      from_end = (sdata *) ((char *) from + nbytes + GC_STRING_EXTRA);
+      ptrdiff_t nbytes = sdata_size (from->string
+				     ? string_bytes (from->string)
+				     : SDATA_NBYTES (from));
+      from = (sdata *) ((char *) from + nbytes + GC_STRING_EXTRA);
     }
 }
 
@@ -1810,14 +1783,14 @@ allocate_string_data (struct Lisp_String *s,
 {
   sdata *data, *old_data;
   struct sblock *b;
-  ptrdiff_t needed, old_nbytes;
+  ptrdiff_t old_nbytes;
 
   if (STRING_BYTES_MAX < nbytes)
     string_overflow ();
 
   /* Determine the number of bytes needed to store NBYTES bytes
      of string data.  */
-  needed = SDATA_SIZE (nbytes);
+  ptrdiff_t needed = sdata_size (nbytes);
   if (s->u.s.data)
     {
       old_data = SDATA_OF_STRING (s);
@@ -2068,7 +2041,7 @@ compact_small_strings (void)
 	      nbytes = s ? STRING_BYTES (s) : SDATA_NBYTES (from);
 	      eassert (nbytes <= LARGE_STRING_BYTES);
 
-	      ptrdiff_t size = SDATA_SIZE (nbytes);
+	      ptrdiff_t size = sdata_size (nbytes);
 	      sdata *from_end = (sdata *) ((char *) from
 					   + size + GC_STRING_EXTRA);
 
-- 
2.21.0


  reply	other threads:[~2019-05-13 19:47 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-13 12:15 bug#35711: emacs crashes on m68k after d2f1971dd5 John Paul Adrian Glaubitz
2019-05-13 19:47 ` Paul Eggert [this message]
2019-05-14  6:01   ` John Paul Adrian Glaubitz
2019-05-14  6:04     ` Paul Eggert
2019-05-14  9:10       ` John Paul Adrian Glaubitz
2019-05-14 18:10         ` Paul Eggert

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=98a8bb69-486e-20b2-891d-65015bab9579@cs.ucla.edu \
    --to=eggert@cs.ucla.edu \
    --cc=35711@debbugs.gnu.org \
    --cc=debian@mkarcher.dialup.fu-berlin.de \
    --cc=glaubitz@physik.fu-berlin.de \
    --cc=schwab@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.