Subject: Re: bug#49261: Segfault during loadup

From: Paul Eggert <[email-masked]>
Date: Mon, 12 Jul 2021 00:16:07 -0700

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.
From da2f772fe575b20bff51b49aa5ded2bf15a2c89d Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email-masked]>
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

From 2337869fbf8b967eb53ee57f978f3751987e43dc Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email-masked]>
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

From 1a0fe2a5184cd4c57972994cf4b688042aecc534 Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email-masked]>
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

From c22cf4d02ff7ebd85839aac5336f6e279f32db54 Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email-masked]>
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

From a79c578f3d77101964b837e8fa8b8109f21c7a88 Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email-masked]>
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