GNU bug report logs - #10042
VIRT_ADDR_VARIES fix for Emacs under valgrind etc.

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 10042 in the body.
You can then email your comments to 10042 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: bug-gnu-emacs <at> gnu.org
Subject: VIRT_ADDR_VARIES fix for Emacs under valgrind etc.
Date: Sun, 13 Nov 2011 20:55:03 -0800

This is a followup to the thread that began at
<http://lists.gnu.org/archive/html/emacs-devel/2011-11/msg00081.html>.

This patch fixes a bug where Emacs won't run correctly
under valgrind.  As Andreas mentioned, without the patch Emacs
probably isn't all that safe even when run outside of valgrind.

# Bazaar merge directive format 2 (Bazaar 0.90)
# revision_id: eggert <at> cs.ucla.edu-20111114043522-l0gmk07ckh964xtp
# target_branch: bzr+ssh://eggert <at> bzr.savannah.gnu.org/emacs/trunk
# testament_sha1: 6e35062324740218b862c3bc27e0aed206fee1ee
# timestamp: 2011-11-13 20:42:27 -0800
# base_revision_id: rgm <at> gnu.org-20111114021401-gnrqc3mar3ra6cjp
# 
# Begin patch
=== modified file 'src/ChangeLog'
--- src/ChangeLog	2011-11-12 12:11:54 +0000
+++ src/ChangeLog	2011-11-14 04:35:22 +0000
@@ -1,3 +1,22 @@
+2011-11-14  Paul Eggert  <eggert <at> cs.ucla.edu>
+
+	Standardize on VIRT_ADDR_VARIES behavior; otherwise, valgrind
+	does not work on some platforms.  Problem reported by Andreas Schwab in
+	<http://lists.gnu.org/archive/html/emacs-devel/2011-11/msg00081.html>.
+	* puresize.h (pure, PURE_P): Always behave as if VIRT_ADDR_VARIES
+	is set, removing the need for VIRT_ADDRESS_VARIES.
+	(PURE_P): Use a more-efficient implementation that needs just one
+	comparison, not two: on x86-64 with GCC 4.6.2, this cut down the
+	number of instructions from 6 (xorl, cmpq, jge, xorl, cmpq, setge)
+	to 4 (xorl, subq, cmpq, setbe).
+	* alloc.c (pure): Always extern now, since that's the
+	VIRT_ADDR_VARIES behavior.
+	(PURE_POINTER_P): Use a single comparison, not two, for
+	consistency with the new puresize.h.
+	* lisp.h (PNTR_COMPARISON_TYPE): Remove; no longer needed.
+	* m/ibms390.h, m/intel386.h, m/template.h, s/cygwin.h, s/hpux10-20.h:
+	Remove VIRT_ADDR_VARIES no longer needed.
+
 2011-11-12  Eli Zaretskii  <eliz <at> gnu.org>
 
 	* makefile.w32-in (HEAPSIZE): New variable, allows to build temacs

=== modified file 'src/alloc.c'
--- src/alloc.c	2011-11-07 05:37:49 +0000
+++ src/alloc.c	2011-11-10 08:14:27 +0000
@@ -203,9 +203,6 @@
    remapping on more recent systems because this is less important
    nowadays than in the days of small memories and timesharing.  */
 
-#ifndef VIRT_ADDR_VARIES
-static
-#endif
 EMACS_INT pure[(PURESIZE + sizeof (EMACS_INT) - 1) / sizeof (EMACS_INT)] = {1,};
 #define PUREBEG (char *) pure
 
@@ -222,10 +219,7 @@
 /* Value is non-zero if P points into pure space.  */
 
 #define PURE_POINTER_P(P)					\
-     (((PNTR_COMPARISON_TYPE) (P)				\
-       < (PNTR_COMPARISON_TYPE) ((char *) purebeg + pure_size))	\
-      && ((PNTR_COMPARISON_TYPE) (P)				\
-	  >= (PNTR_COMPARISON_TYPE) purebeg))
+  ((uintptr_t) (P) - (uintptr_t) purebeg <= pure_size)
 
 /* Index in pure at which next pure Lisp object will be allocated.. */
 

=== modified file 'src/lisp.h'
--- src/lisp.h	2011-11-07 17:04:01 +0000
+++ src/lisp.h	2011-11-10 08:14:27 +0000
@@ -1877,9 +1877,6 @@
     CHECK_NATNUM (tmp);			\
     XSETCDR ((x), tmp);			\
   } while (0)
-
-/* Cast pointers to this type to compare them.  */
-#define PNTR_COMPARISON_TYPE uintptr_t
 
 /* Define a built-in function for calling from Lisp.
  `lname' should be the name to give the function in Lisp,

=== modified file 'src/m/ibms390.h'
--- src/m/ibms390.h	2011-02-16 01:35:20 +0000
+++ src/m/ibms390.h	2011-11-10 08:14:27 +0000
@@ -17,11 +17,6 @@
 You should have received a copy of the GNU General Public License
 along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 
-
-/* Define VIRT_ADDR_VARIES if the virtual addresses of
-   pure and impure space as loaded can vary, and even their
-   relative order cannot be relied on.
-
-   Otherwise Emacs assumes that text space precedes data space,
-   numerically.  */
-#define VIRT_ADDR_VARIES
+/* This file is a placeholder -- it does not contain any definitions.
+   At some point we should probably fix this by removing the file
+   and removing all uses of it.  */

=== modified file 'src/m/intel386.h'
--- src/m/intel386.h	2011-01-31 23:54:50 +0000
+++ src/m/intel386.h	2011-11-10 08:14:27 +0000
@@ -19,7 +19,6 @@
 
 
 #ifdef WINDOWSNT
-#define VIRT_ADDR_VARIES
 #define DATA_START 	get_data_start ()
 #endif
 
@@ -28,4 +27,3 @@
 /* we cannot get the maximum address for brk */
 #define ULIMIT_BREAK_VALUE (32*1024*1024)
 #endif
-

=== modified file 'src/m/template.h'
--- src/m/template.h	2011-02-16 01:35:20 +0000
+++ src/m/template.h	2011-11-10 08:14:27 +0000
@@ -21,14 +21,6 @@
    does not define it automatically.
    Ones defined so far include m68k and many others */
 
-/* Define VIRT_ADDR_VARIES if the virtual addresses of
-   pure and impure space as loaded can vary, and even their
-   relative order cannot be relied on.
-
-   Otherwise Emacs assumes that text space precedes data space,
-   numerically.  */
-#define VIRT_ADDR_VARIES
-
 /* After adding support for a new machine, modify the large case
    statement in configure.in to recognize reasonable
    configuration names, and add a description of the system to

=== modified file 'src/puresize.h'
--- src/puresize.h	2011-06-09 19:08:29 +0000
+++ src/puresize.h	2011-11-10 08:14:27 +0000
@@ -75,21 +75,7 @@
 
 /* Define PURE_P.  */
 
-#ifdef VIRT_ADDR_VARIES
-/* For machines where text and data can go anywhere
-   in virtual memory.  */
-
 extern EMACS_INT pure[];
 
 #define PURE_P(obj) \
- ((PNTR_COMPARISON_TYPE) XPNTR (obj) < (PNTR_COMPARISON_TYPE) ((char *) pure + PURESIZE) \
-  && (PNTR_COMPARISON_TYPE) XPNTR (obj) >= (PNTR_COMPARISON_TYPE) pure)
-
-#else /* not VIRT_ADDR_VARIES */
-
-extern char my_edata[];
-
-#define PURE_P(obj) \
-  ((PNTR_COMPARISON_TYPE) XPNTR (obj) < (PNTR_COMPARISON_TYPE) my_edata)
-
-#endif /* VIRT_ADDRESS_VARIES */
+  ((uintptr_t) XPNTR (obj) - (uintptr_t) pure <= PURESIZE)

=== modified file 'src/s/cygwin.h'
--- src/s/cygwin.h	2011-03-17 05:15:08 +0000
+++ src/s/cygwin.h	2011-11-10 08:14:27 +0000
@@ -91,9 +91,6 @@
    why it needed to be changed.  */
 #define GC_MARK_STACK GC_MAKE_GCPROS_NOOPS
 
-/* Virtual addresses of pure and impure space can vary, as on Windows.  */
-#define VIRT_ADDR_VARIES
-
 /* Emacs supplies its own malloc, but glib (part of Gtk+) calls
    memalign and on Cygwin, that becomes the Cygwin-supplied memalign.
    As malloc is not the Cygwin malloc, the Cygwin memalign always

=== modified file 'src/s/hpux10-20.h'
--- src/s/hpux10-20.h	2011-11-11 23:17:16 +0000
+++ src/s/hpux10-20.h	2011-11-14 04:35:22 +0000
@@ -100,14 +100,6 @@
    header sections which lose when `static' is defined away, as it is
    on HP-UX.  (You get duplicate symbol errors on linking). */
 #undef _FILE_OFFSET_BITS
-
-/* Define VIRT_ADDR_VARIES if the virtual addresses of
-   pure and impure space as loaded can vary, and even their
-   relative order cannot be relied on.
-
-   Otherwise Emacs assumes that text space precedes data space,
-   numerically.  */
-#define VIRT_ADDR_VARIES
 
 /* The data segment on this machine always starts at address 0x40000000.  */
 #define DATA_SEG_BITS 0x40000000

Message #12 received at 10042-done <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: 10042-done <at> debbugs.gnu.org
Subject: Re: VIRT_ADDR_VARIES fix for Emacs under valgrind etc.
Date: Sat, 19 Nov 2011 19:21:19 -0800

No further comment and this does fix a bug
so I committed the fix into the trunk
as bzr 106440.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.