bug#49261: Segfault during loadup

[Paul Eggert <eggert@cs.ucla.edu>]

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

On 7/11/21 8:25 AM, Eli Zaretskii wrote:

>>    lisp.h:251:18: warning: unsigned conversion from 'long long int' to 'uintptr_t' {aka 'unsigned int'} changes value from '2305843009213693951' to '4294967295' [-Woverflow]
>>      251 | # define VALMASK (USE_LSB_TAG ? - (1 << GCTYPEBITS) : VAL_MAX)
>> 	|                  ^
>>    alloc.c:4767:24: note: in expansion of macro 'VALMASK'
>>     4767 |       uintptr_t mask = VALMASK;
>> 	|                        ^~~~~~~
> I tried to fix this on master, please take a look.

Yes that GCC warning was bogus, and your pacification of GCC is valid 
now that we no longer tag the MSB of pointers. Still, there should be a 
simpler way to pacify GCC so I installed a further fix that I hope does 
that (see first attached patch). This fix simply uses a cast (uintptr_t) 
VALMASK to pacify GCC; if GCC issues a bogus warning even for that cast, 
we could substitute (uintptr_t) (VALMASK & UINTPTR_MAX) though this is 
starting to get a little ridiculous.

The version of GCC that I tried (11.1.1 20210531 (Red Hat 11.1.1-3)) 
don't warn about the original code, so perhaps the bogus warning that 
you saw is a GCC bug that's been fixed in later GCC versions. However, 
GCC 11.1.1 does warn about some other stuff so I installed the remaining 
patches to pacify it. Some of these patches fix real (albeit unlikely) 
bugs in Emacs. Some work around what are evidently flaws in GCC 11.1.1. 
Oh well.

[-- Attachment #2: 0001-Pacify-gcc-Woverflow-more-nicely.patch --]
[-- Type: text/x-patch, Size: 1121 bytes --]

From da2f772fe575b20bff51b49aa5ded2bf15a2c89d Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sun, 11 Jul 2021 23:54:32 -0700
Subject: [PATCH 1/5] Pacify gcc -Woverflow more nicely

* src/alloc.c (mark_maybe_pointer): Simplify pacification
of gcc -Woverflow (unknown GCC version).
---
 src/alloc.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/src/alloc.c b/src/alloc.c
index e3b038c51c..ee3fd64a00 100644
--- a/src/alloc.c
+++ b/src/alloc.c
@@ -4764,12 +4764,7 @@ mark_maybe_pointer (void *p, bool symbol_only)
 	 from Emacs source code, it can occur in some cases.  To fix
 	 this problem, the pdumper code should grok non-initial
 	 addresses, as the non-pdumper code does.  */
-#ifdef WIDE_EMACS_INT
-      uintptr_t mask = ~((uintptr_t) 0);
-#else
-      uintptr_t mask = VALMASK;
-#endif
-      void *po = (void *) ((uintptr_t) p & mask);
+      void *po = (void *) ((uintptr_t) p & (uintptr_t) VALMASK);
       char *cp = p;
       char *cpo = po;
       /* Don't use pdumper_object_p_precise here! It doesn't check the
-- 
2.30.2


[-- Attachment #3: 0002-Pacify-gcc-11.1.1-Wanalyzer-null-argument.patch --]
[-- Type: text/x-patch, Size: 7349 bytes --]

From 2337869fbf8b967eb53ee57f978f3751987e43dc Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Mon, 12 Jul 2021 00:00:20 -0700
Subject: [PATCH 2/5] Pacify gcc 11.1.1 -Wanalyzer-null-argument
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* lib-src/etags.c (regexp): Omit member force_explicit_name,
since it’s always true.  All uses removed.  This lets us
remove calls to strlen (name) where GCC isn’t smart enough
to deduce that name must be nonnull.
* lib-src/movemail.c (main): Fix bug that could cause
link (tempname, NULL) to be called.
* src/emacs.c (argmatch): Break check into two ‘if’s,
since GCC doesn’t seem to be smart enough to check the single ‘if’.
* src/gtkutil.c (xg_update_menu_item): Fix bug where strcmp
could be given a NULL arg.
* src/xfont.c (xfont_list_family): Use nonnull value for dummy
initial value.
---
 lib-src/etags.c    | 49 ++++++++++++++++++----------------------------
 lib-src/movemail.c | 14 +++++--------
 src/emacs.c        |  4 +++-
 src/gtkutil.c      |  2 +-
 src/xfont.c        |  5 ++++-
 5 files changed, 32 insertions(+), 42 deletions(-)

diff --git a/lib-src/etags.c b/lib-src/etags.c
index c39c93db33..88b49f803e 100644
--- a/lib-src/etags.c
+++ b/lib-src/etags.c
@@ -340,7 +340,6 @@ #define xrnew(op, n, m) ((op) = xnrealloc (op, n, (m) * sizeof *(op)))
   struct re_pattern_buffer *pat; /* the compiled pattern */
   struct re_registers regs;	/* re registers */
   bool error_signaled;		/* already signaled for this regexp */
-  bool force_explicit_name;	/* do not allow implicit tag name */
   bool ignore_case;		/* ignore case when matching */
   bool multi_line;		/* do a multi-line match on the whole file */
 } regexp;
@@ -6910,7 +6909,6 @@ add_regex (char *regexp_pattern, language *lang)
   struct re_pattern_buffer *patbuf;
   regexp *rp;
   bool
-    force_explicit_name = true, /* do not use implicit tag names */
     ignore_case = false,	/* case is significant */
     multi_line = false,		/* matches are done one line at a time */
     single_line = false;	/* dot does not match newline */
@@ -6949,7 +6947,8 @@ add_regex (char *regexp_pattern, language *lang)
       case 'N':
 	if (modifiers == name)
 	  error ("forcing explicit tag name but no name, ignoring");
-	force_explicit_name = true;
+	/* This option has no effect and is present only for backward
+	   compatibility.  */
 	break;
       case 'i':
 	ignore_case = true;
@@ -7004,7 +7003,6 @@ add_regex (char *regexp_pattern, language *lang)
   p_head->pat = patbuf;
   p_head->name = savestr (name);
   p_head->error_signaled = false;
-  p_head->force_explicit_name = force_explicit_name;
   p_head->ignore_case = ignore_case;
   p_head->multi_line = multi_line;
 }
@@ -7144,20 +7142,15 @@ regex_tag_multiline (void)
 		name = NULL;
 	      else /* make a named tag */
 		name = substitute (buffer, rp->name, &rp->regs);
-	      if (rp->force_explicit_name)
-		{
-		  /* Force explicit tag name, if a name is there. */
-		  pfnote (name, true, buffer + linecharno,
-			  charno - linecharno + 1, lineno, linecharno);
-
-		  if (debug)
-		    fprintf (stderr, "%s on %s:%"PRIdMAX": %s\n",
-			     name ? name : "(unnamed)", curfdp->taggedfname,
-			     lineno, buffer + linecharno);
-		}
-	      else
-		make_tag (name, strlen (name), true, buffer + linecharno,
-			  charno - linecharno + 1, lineno, linecharno);
+
+	      /* Force explicit tag name, if a name is there. */
+	      pfnote (name, true, buffer + linecharno,
+		      charno - linecharno + 1, lineno, linecharno);
+
+	      if (debug)
+		fprintf (stderr, "%s on %s:%"PRIdMAX": %s\n",
+			 name ? name : "(unnamed)", curfdp->taggedfname,
+			 lineno, buffer + linecharno);
 	      break;
 	    }
 	}
@@ -7471,18 +7464,14 @@ readline (linebuffer *lbp, FILE *stream)
 		name = NULL;
 	      else /* make a named tag */
 		name = substitute (lbp->buffer, rp->name, &rp->regs);
-	      if (rp->force_explicit_name)
-		{
-		  /* Force explicit tag name, if a name is there. */
-		  pfnote (name, true, lbp->buffer, match, lineno, linecharno);
-		  if (debug)
-		    fprintf (stderr, "%s on %s:%"PRIdMAX": %s\n",
-			     name ? name : "(unnamed)", curfdp->taggedfname,
-			     lineno, lbp->buffer);
-		}
-	      else
-		make_tag (name, strlen (name), true,
-			  lbp->buffer, match, lineno, linecharno);
+
+	      /* Force explicit tag name, if a name is there. */
+	      pfnote (name, true, lbp->buffer, match, lineno, linecharno);
+
+	      if (debug)
+		fprintf (stderr, "%s on %s:%"PRIdMAX": %s\n",
+			 name ? name : "(unnamed)", curfdp->taggedfname,
+			 lineno, lbp->buffer);
 	      break;
 	    }
 	}
diff --git a/lib-src/movemail.c b/lib-src/movemail.c
index cfdebccb8d..e683da179d 100644
--- a/lib-src/movemail.c
+++ b/lib-src/movemail.c
@@ -270,6 +270,7 @@ main (int argc, char **argv)
 	 You might also wish to verify that your system is one which
 	 uses lock files for this purpose.  Some systems use other methods.  */
 
+      bool lockname_unlinked = false;
       inname_len = strlen (inname);
       lockname = xmalloc (inname_len + sizeof ".lock");
       strcpy (lockname, inname);
@@ -312,15 +313,10 @@ main (int argc, char **argv)
 	     Five minutes should be good enough to cope with crashes
 	     and wedgitude, and long enough to avoid being fooled
 	     by time differences between machines.  */
-	  if (stat (lockname, &st) >= 0)
-	    {
-	      time_t now = time (0);
-	      if (st.st_ctime < now - 300)
-		{
-		  unlink (lockname);
-		  lockname = 0;
-		}
-	    }
+	  if (!lockname_unlinked
+	      && stat (lockname, &st) == 0
+	      && st.st_ctime < time (0) - 300)
+	    lockname_unlinked = unlink (lockname) == 0 || errno == ENOENT;
 	}
 
       delete_lockname = lockname;
diff --git a/src/emacs.c b/src/emacs.c
index b7982ece64..866e43fda9 100644
--- a/src/emacs.c
+++ b/src/emacs.c
@@ -670,7 +670,9 @@ argmatch (char **argv, int argc, const char *sstr, const char *lstr,
     }
   arglen = (valptr != NULL && (p = strchr (arg, '=')) != NULL
 	    ? p - arg : strlen (arg));
-  if (lstr == 0 || arglen < minlen || strncmp (arg, lstr, arglen) != 0)
+  if (!lstr)
+    return 0;
+  if (arglen < minlen || strncmp (arg, lstr, arglen) != 0)
     return 0;
   else if (valptr == NULL)
     {
diff --git a/src/gtkutil.c b/src/gtkutil.c
index dee2a93089..313cfc82c2 100644
--- a/src/gtkutil.c
+++ b/src/gtkutil.c
@@ -3221,7 +3221,7 @@ xg_update_menu_item (widget_value *val,
       gtk_label_set_text (wkey, utf8_key);
     }
 
-  if (! old_label || strcmp (utf8_label, old_label) != 0)
+  if (utf8_label && (! old_label || strcmp (utf8_label, old_label) != 0))
     {
       label_changed = true;
       gtk_label_set_text (wlbl, utf8_label);
diff --git a/src/xfont.c b/src/xfont.c
index 0570ee96a9..81d356175a 100644
--- a/src/xfont.c
+++ b/src/xfont.c
@@ -596,7 +596,10 @@ xfont_list_family (struct frame *f)
   char **names;
   int num_fonts, i;
   Lisp_Object list;
-  char *last_family UNINIT;
+  char const *last_family;
+#if defined GCC_LINT || defined lint
+  last_family = "";
+#endif
   int last_len;
 
   block_input ();
-- 
2.30.2


[-- Attachment #4: 0003-Pacify-gcc-11.1.1-Wanalyzer-possible-null-dereferenc.patch --]
[-- Type: text/x-patch, Size: 5627 bytes --]

From 1a0fe2a5184cd4c57972994cf4b688042aecc534 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Mon, 12 Jul 2021 00:06:34 -0700
Subject: [PATCH 3/5] Pacify gcc 11.1.1 -Wanalyzer-possible-null-dereference
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* oldXMenu/Create.c (XMenuCreate):
* oldXMenu/Internal.c (_XMRecomputePane, _XMRecomputeSelection):
* oldXMenu/XMakeAssoc.c (XMakeAssoc):
* test/src/emacs-module-resources/mod-test.c (Fmod_test_userptr_make):
Don’t assume that malloc and calloc succeed.
---
 oldXMenu/Create.c                          |  2 ++
 oldXMenu/Internal.c                        | 31 +++++++++-------------
 oldXMenu/XMakeAssoc.c                      |  2 ++
 test/src/emacs-module-resources/mod-test.c |  4 +++
 4 files changed, 20 insertions(+), 19 deletions(-)

diff --git a/oldXMenu/Create.c b/oldXMenu/Create.c
index 7eb17c508d..e209bbecee 100644
--- a/oldXMenu/Create.c
+++ b/oldXMenu/Create.c
@@ -598,6 +598,8 @@ XMenuCreate(Display *display, Window parent, register char const *def_env)
    * Create pane, active, and inactive GC's.
    */
   values = (XGCValues *)malloc(sizeof(XGCValues));
+  if (!values)
+    return NULL;
   valuemask = (GCForeground | GCBackground | GCFont | GCLineWidth);
 
   /*
diff --git a/oldXMenu/Internal.c b/oldXMenu/Internal.c
index f489e27bea..3e97f9ab3f 100644
--- a/oldXMenu/Internal.c
+++ b/oldXMenu/Internal.c
@@ -534,7 +534,6 @@ _XMRecomputePane(register Display *display, register XMenu *menu, register XMPan
     register int window_y;	/* Recomputed window Y coordinate. */
 
     unsigned long change_mask;	/* Value mask to reconfigure window. */
-    XWindowChanges *changes;	/* Values to use in configure window. */
 
     register Bool config_p = False;	/* Reconfigure pane window? */
 
@@ -612,21 +611,19 @@ _XMRecomputePane(register Display *display, register XMenu *menu, register XMPan
 	 * it for creation with the new configuration.
 	 */
 	if (p_ptr->window) {
+	    XWindowChanges changes;
 	    change_mask = (CWX | CWY | CWWidth | CWHeight);
-	    changes = (XWindowChanges *)malloc(sizeof(XWindowChanges));
-	    changes->x = p_ptr->window_x;
-	    changes->y = p_ptr->window_y;
-	    changes->width = p_ptr->window_w;
-	    changes->height = p_ptr->window_h;
+	    changes.x = p_ptr->window_x;
+	    changes.y = p_ptr->window_y;
+	    changes.width = p_ptr->window_w;
+	    changes.height = p_ptr->window_h;
 
 	    XConfigureWindow(
 			     display,
 			     p_ptr->window,
 			     change_mask,
-			     changes
+			     &changes
 			     );
-	    free(changes);
-
 	}
 	else {
 	    if (_XMWinQueAddPane(display, menu, p_ptr) == _FAILURE) {
@@ -681,7 +678,6 @@ _XMRecomputeSelection(register Display *display, register XMenu *menu, register
                        		/* Selection sequence number. */
 {
     register Bool config_s = False;	/* Reconfigure selection window? */
-    XWindowChanges *changes;		/* Values to change in configure. */
     unsigned long change_mask;		/* Value mask for XConfigureWindow. */
 
     /*
@@ -738,22 +734,19 @@ _XMRecomputeSelection(register Display *display, register XMenu *menu, register
 	 * for creation with the new configuration.
 	 */
 	if (s_ptr->window) {
-	    changes = (XWindowChanges *)malloc(sizeof(XWindowChanges));
+	    XWindowChanges changes;
 	    change_mask = (CWX | CWY | CWWidth | CWHeight);
-	    changes = (XWindowChanges *)malloc(sizeof(XWindowChanges));
-	    changes->x = s_ptr->window_x;
-	    changes->y = s_ptr->window_y;
-	    changes->width = s_ptr->window_w;
-	    changes->height = s_ptr->window_h;
+	    changes.x = s_ptr->window_x;
+	    changes.y = s_ptr->window_y;
+	    changes.width = s_ptr->window_w;
+	    changes.height = s_ptr->window_h;
 
 	    XConfigureWindow(
 			     display,
 			     s_ptr->window,
 			     change_mask,
-			     changes
+			     &changes
 			     );
-	    free(changes);
-
 	}
 	else {
 	    if (_XMWinQueAddSelection(display, menu, s_ptr) == _FAILURE) {
diff --git a/oldXMenu/XMakeAssoc.c b/oldXMenu/XMakeAssoc.c
index 9bbde2cf94..2530e8e507 100644
--- a/oldXMenu/XMakeAssoc.c
+++ b/oldXMenu/XMakeAssoc.c
@@ -69,6 +69,8 @@ XMakeAssoc(register Display *dpy, register XAssocTable *table, register XID x_id
 	/* before the current value of "Entry". */
 	/* Create a new XAssoc and load it with new provided data. */
 	new_entry = (XAssoc *) malloc(sizeof(XAssoc));
+	if (!new_entry)
+	  return; /* This obsolete API has no way to report failure!  */
 	new_entry->display = dpy;
 	new_entry->x_id = x_id;
 	new_entry->data = data;
diff --git a/test/src/emacs-module-resources/mod-test.c b/test/src/emacs-module-resources/mod-test.c
index ad59cfc18c..5720af8c60 100644
--- a/test/src/emacs-module-resources/mod-test.c
+++ b/test/src/emacs-module-resources/mod-test.c
@@ -288,6 +288,8 @@ Fmod_test_return_unibyte (emacs_env *env, ptrdiff_t nargs, emacs_value args[],
   char large_unused_buffer[512];
 };
 
+static void signal_errno (emacs_env *, char const *);
+
 /* Return a new user-pointer to a super_struct, with amazing_int set
    to the passed parameter.  */
 static emacs_value
@@ -295,6 +297,8 @@ Fmod_test_userptr_make (emacs_env *env, ptrdiff_t nargs, emacs_value args[],
 			void *data)
 {
   struct super_struct *p = calloc (1, sizeof *p);
+  if (!p)
+    signal_errno (env, "calloc");
   p->amazing_int = env->extract_integer (env, args[0]);
   return env->make_user_ptr (env, free, p);
 }
-- 
2.30.2


[-- Attachment #5: 0004-Pacify-gcc-11.1.1-Wclobbered.patch --]
[-- Type: text/x-patch, Size: 1043 bytes --]

From c22cf4d02ff7ebd85839aac5336f6e279f32db54 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Mon, 12 Jul 2021 00:07:38 -0700
Subject: [PATCH 4/5] Pacify gcc 11.1.1 -Wclobbered

* src/eval.c (Fprogn, internal_lisp_condition_case):
Add CACHEABLE to work around more instances of -Wclobbered bug.
---
 src/eval.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/eval.c b/src/eval.c
index 18faa0b9b1..b76ced79d6 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -462,7 +462,7 @@ DEFUN ("progn", Fprogn, Sprogn, 0, UNEVALLED, 0,
 usage: (progn BODY...)  */)
   (Lisp_Object body)
 {
-  Lisp_Object val = Qnil;
+  Lisp_Object CACHEABLE val = Qnil;
 
   while (CONSP (body))
     {
@@ -1429,7 +1429,7 @@ internal_lisp_condition_case (Lisp_Object var, Lisp_Object bodyform,
 	}
     }
 
-  Lisp_Object result = eval_sub (bodyform);
+  Lisp_Object CACHEABLE result = eval_sub (bodyform);
   handlerlist = oldhandlerlist;
   if (!NILP (success_handler))
     {
-- 
2.30.2


[-- Attachment #6: 0005-Port-test-module-to-glibc-2.33.patch --]
[-- Type: text/x-patch, Size: 1736 bytes --]

From a79c578f3d77101964b837e8fa8b8109f21c7a88 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Mon, 12 Jul 2021 00:11:22 -0700
Subject: [PATCH 5/5] Port test module to glibc 2.33

* test/Makefile.in (REPLACE_FREE, FREE_SOURCE_0, FREE_SOURCE_1):
New macros.
($(test_module)): Improve accuracy of test as to whether free.c
should be compiled; glibc 2.33 does not need it compiled and the
compilation breaks if you try, if you build with
--enable-gcc-warnings.
---
 test/Makefile.in | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/test/Makefile.in b/test/Makefile.in
index c1518d3dcd..7047c24482 100644
--- a/test/Makefile.in
+++ b/test/Makefile.in
@@ -49,6 +49,8 @@ SEPCHAR =
 
 HAVE_NATIVE_COMP = @HAVE_NATIVE_COMP@
 
+REPLACE_FREE = @REPLACE_FREE@
+
 -include ${top_builddir}/src/verbose.mk
 
 # Load any GNU ELPA dependencies that are present, for optional tests.
@@ -274,6 +276,9 @@ MODULE_CFLAGS =
 test_module = $(test_module_dir)/mod-test${SO}
 src/emacs-module-tests.log src/emacs-module-tests.elc: $(test_module)
 
+FREE_SOURCE_0 =
+FREE_SOURCE_1 = $(srcdir)/../lib/free.c
+
 # In the compilation command, we can't use any object or archive file
 # as source because those are not compiled with -fPIC.  Therefore we
 # use only source files.
@@ -282,7 +287,7 @@ $(test_module): $(test_module:
 	$(AM_V_CCLD)$(CC) -shared $(CPPFLAGS) $(MODULE_CFLAGS) $(LDFLAGS) \
 	  -o $@ $< $(LIBGMP) \
 	  $(and $(GMP_H),$(srcdir)/../lib/mini-gmp-gnulib.c) \
-	  $(if $(OMIT_GNULIB_MODULE_free-posix),,$(srcdir)/../lib/free.c) \
+	  $(FREE_SOURCE_$(REPLACE_FREE)) \
 	  $(srcdir)/../lib/timespec.c $(srcdir)/../lib/gettime.c
 endif
 
-- 
2.30.2