Subject: bug#10042: closed (Re: VIRT_ADDR_VARIES fix for Emacs under valgrind etc.)

From: [email-masked] (GNU bug Tracking System)
Date: Sun, 20 Nov 2011 03:23:01 +0000

Your bug report

#10042: VIRT_ADDR_VARIES fix for Emacs under valgrind etc.

which was filed against the emacs package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
10042: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=10042
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: VIRT_ADDR_VARIES fix for Emacs under valgrind etc.
From: Paul Eggert <[email-masked]>
To: [email-masked]
Date: Sat, 19 Nov 2011 19:21:19 -0800
List-Id: <debbugs-submit.debbugs.gnu.org>

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

Subject: VIRT_ADDR_VARIES fix for Emacs under valgrind etc.
From: Paul Eggert <[email-masked]>
To: [email-masked]
Date: Sun, 13 Nov 2011 20:55:03 -0800
List-Id: <debbugs-submit.debbugs.gnu.org>

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: [email-masked]
# target_branch: bzr+ssh://[email-masked]/emacs/trunk
# testament_sha1: 6e35062324740218b862c3bc27e0aed206fee1ee
# timestamp: 2011-11-13 20:42:27 -0800
# base_revision_id: [email-masked]
# 
# 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  <[email-masked]>
+
+	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  <[email-masked]>
 
 	* 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