unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* VIRT_ADDR_VARIES
@ 2011-11-06 17:18 Andreas Schwab
  2011-11-07  1:50 ` VIRT_ADDR_VARIES Paul Eggert
  0 siblings, 1 reply; 25+ messages in thread
From: Andreas Schwab @ 2011-11-06 17:18 UTC (permalink / raw)
  To: emacs-devel

Is there any reason not to define VIRT_ADDR_VARIES?  The only difference
it makes is that the bounds of the pure array are checked accurately
instead of assuming that heap always follows data (which is wrong under
valgrind).

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: VIRT_ADDR_VARIES
  2011-11-06 17:18 VIRT_ADDR_VARIES Andreas Schwab
@ 2011-11-07  1:50 ` Paul Eggert
  2011-11-07  8:38   ` VIRT_ADDR_VARIES Andreas Schwab
  0 siblings, 1 reply; 25+ messages in thread
From: Paul Eggert @ 2011-11-07  1:50 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: emacs-devel

On 11/06/11 09:18, Andreas Schwab wrote:
> Is there any reason not to define VIRT_ADDR_VARIES?

I expect the main reason is performance.  For example,
on x86-64 with GCC 4.6.2 -O2, the function:

int pure (Lisp_Object obj) { return PURE_P (obj); }

has 8 instructions (not counting the 'ret') if VIRT_ADDR_VARIES
is defined, and 4 instructions if it is not defined.  The key difference
is that VIRT_ADDRESS_VARIES generates this:

        cmpq    $pure+1000000, %rdi
        jge     .L2
        xorl    %eax, %eax
        cmpq    $pure, %rdi
        setge   %al
  .L2:  rep

(where the "1000000" is a function of PURESIZE),
whereas omitting VIRT_ADDRESS_VARIES generates this:

        cmpq    $my_edata, %rdi
        setl    %al


> The only difference
> it makes is that the bounds of the pure array are checked accurately

True, but for Emacs it shouldn't matter whether PURE_P checks
accurately or loosely -- either way Emacs should operate correctly.



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: VIRT_ADDR_VARIES
  2011-11-07  1:50 ` VIRT_ADDR_VARIES Paul Eggert
@ 2011-11-07  8:38   ` Andreas Schwab
  2011-11-07 23:23     ` VIRT_ADDR_VARIES Paul Eggert
  0 siblings, 1 reply; 25+ messages in thread
From: Andreas Schwab @ 2011-11-07  8:38 UTC (permalink / raw)
  To: Paul Eggert; +Cc: emacs-devel

Paul Eggert <eggert@cs.ucla.edu> writes:

> I expect the main reason is performance.

How much of it matters?

> True, but for Emacs it shouldn't matter whether PURE_P checks
> accurately or loosely -- either way Emacs should operate correctly.

Which it doesn't.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: VIRT_ADDR_VARIES
  2011-11-07  8:38   ` VIRT_ADDR_VARIES Andreas Schwab
@ 2011-11-07 23:23     ` Paul Eggert
  2011-11-08  8:56       ` VIRT_ADDR_VARIES Andreas Schwab
  0 siblings, 1 reply; 25+ messages in thread
From: Paul Eggert @ 2011-11-07 23:23 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: emacs-devel

On 11/07/11 00:38, Andreas Schwab wrote:
> Paul Eggert <eggert@cs.ucla.edu> writes:
>> I expect the main reason is performance.
> 
> How much of it matters?

Compiling without VIRT_ADDR_VARIES speeds up aset by 44% on my host
(Fedora 15 x86-64, GCC 4.6.2, AMD Phenom II X4 910e).
Benchmark code is shown below; I ran (benchmark 100000000)
five times on the byte-compiled version of this code,
and took the median.

>> for Emacs it shouldn't matter whether PURE_P checks
>> accurately or loosely -- either way Emacs should operate correctly.
> 
> Which it doesn't.

Sorry, I don't know the context.  What's the bug here?
If Emacs doesn't define VIRT_ADDR_VARIES when it should,
then that can cause bugs -- is that the issue?

Here's the benchmark code.

(defun benchmark-with-aset (n)
  (let ((start (float-time (get-internal-run-time)))
	(v (make-vector 1 0))
	(i 0))
    (while (< i n)
      (aset v 0 1)
      (setq i (1+ i)))
    (- (float-time (get-internal-run-time)) start)))

(defun benchmark-without-aset (n)
  (let ((start (float-time (get-internal-run-time)))
	(v (make-vector 1 0))
	(i 0))
    (while (< i n)
      (setq i (1+ i)))
    (- (float-time (get-internal-run-time)) start)))

(defun benchmark (n)
  (- (benchmark-with-aset n)
     (benchmark-without-aset n)))



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: VIRT_ADDR_VARIES
  2011-11-07 23:23     ` VIRT_ADDR_VARIES Paul Eggert
@ 2011-11-08  8:56       ` Andreas Schwab
  2011-11-08 17:34         ` VIRT_ADDR_VARIES Paul Eggert
  0 siblings, 1 reply; 25+ messages in thread
From: Andreas Schwab @ 2011-11-08  8:56 UTC (permalink / raw)
  To: Paul Eggert; +Cc: emacs-devel

Paul Eggert <eggert@cs.ucla.edu> writes:

> Compiling without VIRT_ADDR_VARIES speeds up aset by 44% on my host

I cannot reproduce that.  The difference I get is lost in the noise.

> Sorry, I don't know the context.  What's the bug here?

$ valgrind src/temacs 
[...]
Segmentation fault

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: VIRT_ADDR_VARIES
  2011-11-08  8:56       ` VIRT_ADDR_VARIES Andreas Schwab
@ 2011-11-08 17:34         ` Paul Eggert
  2011-11-08 18:38           ` VIRT_ADDR_VARIES Andreas Schwab
  0 siblings, 1 reply; 25+ messages in thread
From: Paul Eggert @ 2011-11-08 17:34 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: emacs-devel

On 11/08/11 00:56, Andreas Schwab wrote:
> Paul Eggert <eggert@cs.ucla.edu> writes:
> 
>> Compiling without VIRT_ADDR_VARIES speeds up aset by 44% on my host
> 
> I cannot reproduce that.  The difference I get is lost in the noise.

This appears to be weirdness on my end -- my host gives reproducible
results when I use those executables, but when I build them again
I get results showing that VIRT_ADDR_VARIES speeds things up, which
can't be right.  I'll try to get to the bottom of this but
for now please ignore my claim about measurable performance difference.

>> Sorry, I don't know the context.  What's the bug here?
> 
> $ valgrind src/temacs 
> [...]
> Segmentation fault

Is that a segmentation fault in valgrind itself, or in temacs?

I don't see why running temacs under valgrind would affect
VIRT_ADDR_VARIES; surely valgrind doesn't change the pure array's
address.

But you're right: if setting VIRT_ADDR_VARIES has no measurable
performance difference and if it lets people debug better, that's
a pretty strong argument for having it always on.



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: VIRT_ADDR_VARIES
  2011-11-08 17:34         ` VIRT_ADDR_VARIES Paul Eggert
@ 2011-11-08 18:38           ` Andreas Schwab
  2011-11-08 21:17             ` VIRT_ADDR_VARIES Paul Eggert
  0 siblings, 1 reply; 25+ messages in thread
From: Andreas Schwab @ 2011-11-08 18:38 UTC (permalink / raw)
  To: Paul Eggert; +Cc: emacs-devel

Paul Eggert <eggert@cs.ucla.edu> writes:

> Is that a segmentation fault in valgrind itself, or in temacs?

It causes PURE_P to always return true.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: VIRT_ADDR_VARIES
  2011-11-08 18:38           ` VIRT_ADDR_VARIES Andreas Schwab
@ 2011-11-08 21:17             ` Paul Eggert
  2011-11-08 22:06               ` VIRT_ADDR_VARIES Andreas Schwab
  0 siblings, 1 reply; 25+ messages in thread
From: Paul Eggert @ 2011-11-08 21:17 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: emacs-devel

On 11/08/11 10:38, Andreas Schwab wrote:
> It causes PURE_P to always return true.

OK, so Emacs is dumping core, not valgrind.
But how can valgrind affect the address of my_edata?
Valgrind supplies its own allocator, but I didn't
think it could move initialized data around.

I'm asking partly because I'm worried that if
valgrind can move my_edata, then there are more-serious
issues that we also would need to address, i.e. that
the PURE_P core dump is just the tip of an iceberg.

I assume this is on x86-64 GNU/Linux?  I'm not observing
this behavior on my host, which is that flavor.
I'm running Fedora 15, which has valgrind 3.6.1.



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: VIRT_ADDR_VARIES
  2011-11-08 21:17             ` VIRT_ADDR_VARIES Paul Eggert
@ 2011-11-08 22:06               ` Andreas Schwab
  2011-11-09 17:44                 ` VIRT_ADDR_VARIES Paul Eggert
  0 siblings, 1 reply; 25+ messages in thread
From: Andreas Schwab @ 2011-11-08 22:06 UTC (permalink / raw)
  To: Paul Eggert; +Cc: emacs-devel

Paul Eggert <eggert@cs.ucla.edu> writes:

> But how can valgrind affect the address of my_edata?

It doesn't of course.

> Valgrind supplies its own allocator, but I didn't
> think it could move initialized data around.

But the heap.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: VIRT_ADDR_VARIES
  2011-11-08 22:06               ` VIRT_ADDR_VARIES Andreas Schwab
@ 2011-11-09 17:44                 ` Paul Eggert
  2011-11-09 21:32                   ` VIRT_ADDR_VARIES Andreas Schwab
  2011-11-10  4:05                   ` VIRT_ADDR_VARIES Stephen J. Turnbull
  0 siblings, 2 replies; 25+ messages in thread
From: Paul Eggert @ 2011-11-09 17:44 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: emacs-devel

On 11/08/11 14:06, Andreas Schwab wrote:
>> Valgrind supplies its own allocator, but I didn't
>> > think it could move initialized data around.
> But the heap.

Sorry, I'm lost.  As I understand it, the
executable in a traditional Unix-like system has
storage laid out in this order:

  text (programs and read-only data)
  data (read-write initialized static data)
  bss (read-write zeroed static data)
  everything else

Valgrind has some control over the layout in
"everything else", but it can't affect the addresses
in text, data, and bss.  my_edata lives in "data",
so how can valgrind affect whether a pointer compares
less than my_edata?



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: VIRT_ADDR_VARIES
  2011-11-09 17:44                 ` VIRT_ADDR_VARIES Paul Eggert
@ 2011-11-09 21:32                   ` Andreas Schwab
  2011-11-10  8:17                     ` VIRT_ADDR_VARIES Paul Eggert
  2011-11-10  4:05                   ` VIRT_ADDR_VARIES Stephen J. Turnbull
  1 sibling, 1 reply; 25+ messages in thread
From: Andreas Schwab @ 2011-11-09 21:32 UTC (permalink / raw)
  To: Paul Eggert; +Cc: emacs-devel

Paul Eggert <eggert@cs.ucla.edu> writes:

> Valgrind has some control over the layout in
> "everything else",

Everything else can be anywhere.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: VIRT_ADDR_VARIES
  2011-11-09 17:44                 ` VIRT_ADDR_VARIES Paul Eggert
  2011-11-09 21:32                   ` VIRT_ADDR_VARIES Andreas Schwab
@ 2011-11-10  4:05                   ` Stephen J. Turnbull
  2011-11-10  5:40                     ` VIRT_ADDR_VARIES Paul Eggert
  1 sibling, 1 reply; 25+ messages in thread
From: Stephen J. Turnbull @ 2011-11-10  4:05 UTC (permalink / raw)
  To: Paul Eggert; +Cc: Andreas Schwab, emacs-devel

Paul Eggert writes:

 > Sorry, I'm lost.  As I understand it, the
 > executable in a traditional Unix-like system has
 > storage laid out in this order:

We don't live in a traditional world anymore.  We have SELinux,
randomized base addresses, and the Hannibal Lector "Slice 'Em & Dice
'Em" version of GNU ld to deal with, as well as a few "legacy"
environments that don't deliberately try to mess with the heads of
people who used to use BASIC to "poke" 8080 instructions into memory.




^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: VIRT_ADDR_VARIES
  2011-11-10  4:05                   ` VIRT_ADDR_VARIES Stephen J. Turnbull
@ 2011-11-10  5:40                     ` Paul Eggert
  2011-11-10 17:05                       ` VIRT_ADDR_VARIES Andreas Schwab
  0 siblings, 1 reply; 25+ messages in thread
From: Paul Eggert @ 2011-11-10  5:40 UTC (permalink / raw)
  To: Stephen J. Turnbull; +Cc: Andreas Schwab, emacs-devel

On 11/09/11 20:05, Stephen J. Turnbull wrote:
> We don't live in a traditional world anymore.  We have SELinux,...

Yes, of course, but the question isn't whether we
have non-traditional layouts -- of course we have them.
The question is: on what type of platform is VIRT_ADDR_VARIES
correctly unset (i.e., the traditional model is good enough
for Emacs), but this approach breaks down when running under
valgrind?

Let me try to state the issue more concretely.
I don't observe the problem on my host (Fedora 15 x86-64),
even though it has address space randomization.
If I run these two shell commands:

         ./temacs --batch --eval '(progn (insert-file "/proc/self/maps") (message "%s\n" (buffer-string)))'

valgrind ./temacs --batch --eval '(progn (insert-file "/proc/self/maps") (message "%s\n" (buffer-string)))'

I get quite different outputs, but the text, data, and bss
segments are laid out identically in both cases, and they're
always laid out before the heap.  That is, the output looks
like this both times:

  00400000-00634000 r-xp 00000000 09:02 106972830                          .../src/temacs
  00834000-00adc000 rw-p 00234000 09:02 106972830                          .../src/temacs
  00adc000-00b63000 rw-p 00000000 00:00 0
  [...everything else...]

Here the first line is text, the second data, the third bss.
Because the heap is after the data in both cases,
Emacs works in both cases, and there's no need to define
VIRT_ADDR_VARIES, regardless of whether valgrind is used.

Can someone run those two commands on a GNU/Linux host where valgrind
breaks things, and use the output to explain why the breakage
occurs?  That might help explain the issue better.



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: VIRT_ADDR_VARIES
  2011-11-09 21:32                   ` VIRT_ADDR_VARIES Andreas Schwab
@ 2011-11-10  8:17                     ` Paul Eggert
  2011-11-10  9:29                       ` VIRT_ADDR_VARIES Andreas Schwab
                                         ` (2 more replies)
  0 siblings, 3 replies; 25+ messages in thread
From: Paul Eggert @ 2011-11-10  8:17 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: emacs-devel

How about this patch?  Does it fix your problem with valgrind?

=== modified file 'src/ChangeLog'
--- src/ChangeLog	2011-11-09 14:29:23 +0000
+++ src/ChangeLog	2011-11-10 08:14:27 +0000
@@ -1,3 +1,22 @@
+2011-11-10  Paul Eggert  <eggert@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-09  Chong Yidong  <cyd@gnu.org>
 
 	* window.c (Fwindow_inside_edges, Fwindow_inside_pixel_edges)

=== 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
 \f
 /* 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 @@
 \f
 /* 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-02-16 01:35:20 +0000
+++ src/s/hpux10-20.h	2011-11-10 08:14:27 +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
 \f
 /* The data segment on this machine always starts at address 0x40000000.  */
 #define DATA_SEG_BITS 0x40000000



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: VIRT_ADDR_VARIES
  2011-11-10  8:17                     ` VIRT_ADDR_VARIES Paul Eggert
@ 2011-11-10  9:29                       ` Andreas Schwab
  2011-11-10 16:19                         ` VIRT_ADDR_VARIES Paul Eggert
  2011-11-10 11:06                       ` VIRT_ADDR_VARIES Eli Zaretskii
  2011-11-14  4:57                       ` VIRT_ADDR_VARIES Paul Eggert
  2 siblings, 1 reply; 25+ messages in thread
From: Andreas Schwab @ 2011-11-10  9:29 UTC (permalink / raw)
  To: Paul Eggert; +Cc: emacs-devel

Paul Eggert <eggert@cs.ucla.edu> writes:

> +  ((uintptr_t) XPNTR (obj) - (uintptr_t) pure <= PURESIZE)

There is no need for obfuscation, the compiler should find that out by
itself.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: VIRT_ADDR_VARIES
  2011-11-10  8:17                     ` VIRT_ADDR_VARIES Paul Eggert
  2011-11-10  9:29                       ` VIRT_ADDR_VARIES Andreas Schwab
@ 2011-11-10 11:06                       ` Eli Zaretskii
  2011-11-10 11:20                         ` VIRT_ADDR_VARIES Andreas Schwab
  2011-11-10 16:29                         ` VIRT_ADDR_VARIES Paul Eggert
  2011-11-14  4:57                       ` VIRT_ADDR_VARIES Paul Eggert
  2 siblings, 2 replies; 25+ messages in thread
From: Eli Zaretskii @ 2011-11-10 11:06 UTC (permalink / raw)
  To: Paul Eggert; +Cc: schwab, emacs-devel

> Date: Thu, 10 Nov 2011 00:17:06 -0800
> From: Paul Eggert <eggert@cs.ucla.edu>
> Cc: emacs-devel@gnu.org
> 
> --- src/puresize.h	2011-06-09 19:08:29 +0000
> +++ src/puresize.h	2011-11-10 08:14:27 +0000
> @@ -75,21 +75,7 @@
>  \f
>  /* 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)

What is the exact definition of "a pure object"?  Your changes assume
that it is something allocated off pure[], but the original macro,
viz.

  extern char my_edata[];

  #define PURE_P(obj) \
    ((PNTR_COMPARISON_TYPE) XPNTR (obj) < (PNTR_COMPARISON_TYPE) my_edata)

tests something different.  It could, for example, return non-zero for
an object that was not dynamically allocated at all.  It could be that
a test against my_edata was just a means, but we should be sure it
was, before we effectively define VIRT_ADDRESS_VARIES on all
platforms.

Also, isn't it dangerous to have an externally visible symbol with a
name such as `pure'?  What if some system library somewhere also has
such a symbol?



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: VIRT_ADDR_VARIES
  2011-11-10 11:06                       ` VIRT_ADDR_VARIES Eli Zaretskii
@ 2011-11-10 11:20                         ` Andreas Schwab
  2011-11-10 16:29                         ` VIRT_ADDR_VARIES Paul Eggert
  1 sibling, 0 replies; 25+ messages in thread
From: Andreas Schwab @ 2011-11-10 11:20 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Paul Eggert, emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

> What is the exact definition of "a pure object"?

Everything in the pure space.

>   #define PURE_P(obj) \
>     ((PNTR_COMPARISON_TYPE) XPNTR (obj) < (PNTR_COMPARISON_TYPE) my_edata)
>
> tests something different.

That's the very bug.

> It could, for example, return non-zero for an object that was not
> dynamically allocated at all.

That doesn't exist.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: VIRT_ADDR_VARIES
  2011-11-10  9:29                       ` VIRT_ADDR_VARIES Andreas Schwab
@ 2011-11-10 16:19                         ` Paul Eggert
  0 siblings, 0 replies; 25+ messages in thread
From: Paul Eggert @ 2011-11-10 16:19 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: emacs-devel

On 11/10/11 01:29, Andreas Schwab wrote:
>> +  ((uintptr_t) XPNTR (obj) - (uintptr_t) pure <= PURESIZE)
> 
> There is no need for obfuscation, the compiler should find that out by
> itself.

Perhaps it should, but it typically doesn't.  For example, given this:

	#include <inttypes.h>
	#include <stddef.h>
	#define TYPEMASK 7
	#define XPNTR(a) ((a) & ~TYPEMASK)
	#define PURESIZE 1000000

	extern long pure[];

	#define PURE_P1(obj) \
	  ((char *) pure <= (char *) XPNTR (obj) \
	   && (char *) XPNTR (obj) < (char *) pure + PURESIZE)

	#define PURE_P2(obj) \
	  ((uintptr_t) XPNTR (obj) - (uintptr_t) pure <= PURESIZE)

	int pure1 (intptr_t obj) { return PURE_P1 (obj); }
	int pure2 (intptr_t obj) { return PURE_P2 (obj); }

gcc -O2 (x86-64, GCC 4.6.2) generates this for the comparisons of PURE_P1:

		xorl	%eax, %eax
		cmpq	$pure, %rdi
		jb	.L2
		xorl	%eax, %eax
		cmpq	$pure+1000000, %rdi
		setb	%al
	.L2:	rep

and this for PURE_P2:

		xorl	%eax, %eax
		subq	$pure, %rdi
		cmpq	$1000000, %rdi
		setbe	%al

The PURE_P1 version has more instructions, and has a conditional jump.
The PURE_P2 version is nearly as fast as the old code without
VIRT_ADDR_VARIES.  I observed similar savings with the other
compilers I tried (Sun C 5.11 sparcv9 cc -xO4, clang 2.8 x86-64 -O2).
So we should go with PURE_P2.




^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: VIRT_ADDR_VARIES
  2011-11-10 11:06                       ` VIRT_ADDR_VARIES Eli Zaretskii
  2011-11-10 11:20                         ` VIRT_ADDR_VARIES Andreas Schwab
@ 2011-11-10 16:29                         ` Paul Eggert
  2011-11-10 16:43                           ` VIRT_ADDR_VARIES Eli Zaretskii
  1 sibling, 1 reply; 25+ messages in thread
From: Paul Eggert @ 2011-11-10 16:29 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

On 11/10/11 03:06, Eli Zaretskii wrote:

> It could be that a test against my_edata was just a means,
> but we should be sure it was

Yes, I'm sure it was a means.  I've read the code carefully, and
remember this part well.  That's why the VIRT_ADDR_VARIES implementation
is also a valid one.

> isn't it dangerous to have an externally visible symbol with a
> name such as `pure'?

Emacs has been built with an extern symbol 'pure'
for decades, on many platforms.  And platform implementers
would be unlikely to usurp that particular name anyway.
So we should be safe.



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: VIRT_ADDR_VARIES
  2011-11-10 16:29                         ` VIRT_ADDR_VARIES Paul Eggert
@ 2011-11-10 16:43                           ` Eli Zaretskii
  2011-11-10 16:50                             ` VIRT_ADDR_VARIES Paul Eggert
  0 siblings, 1 reply; 25+ messages in thread
From: Eli Zaretskii @ 2011-11-10 16:43 UTC (permalink / raw)
  To: Paul Eggert; +Cc: emacs-devel

> Date: Thu, 10 Nov 2011 08:29:43 -0800
> From: Paul Eggert <eggert@cs.ucla.edu>
> CC: emacs-devel@gnu.org
> 
> On 11/10/11 03:06, Eli Zaretskii wrote:
> 
> > It could be that a test against my_edata was just a means,
> > but we should be sure it was
> 
> Yes, I'm sure it was a means.

But why would someone do such a thing? why not test against the start
and end of pure[], like the code conditioned on VIRT_ADDR_VARIES does?
Was all that just to keep pure[] private and not expose it as an
external symbol?  Or was there some other reason?



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: VIRT_ADDR_VARIES
  2011-11-10 16:43                           ` VIRT_ADDR_VARIES Eli Zaretskii
@ 2011-11-10 16:50                             ` Paul Eggert
  2011-11-10 18:06                               ` VIRT_ADDR_VARIES Eli Zaretskii
  0 siblings, 1 reply; 25+ messages in thread
From: Paul Eggert @ 2011-11-10 16:50 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

On 11/10/11 08:43, Eli Zaretskii wrote:
> But why would someone do such a thing?

Performance.

Comparing against my_edata takes just one instruction.
Comparing against both the start and the end of 'pure'
takes two comparisons and some glue instructions and
typically includes a conditional branch.  The optimization
I've suggested in this thread prunes away most of this
underbrush, but it's still one more instruction
(a 'subtract') over the my_edata approach.



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: VIRT_ADDR_VARIES
  2011-11-10  5:40                     ` VIRT_ADDR_VARIES Paul Eggert
@ 2011-11-10 17:05                       ` Andreas Schwab
  2011-11-10 17:15                         ` VIRT_ADDR_VARIES Paul Eggert
  0 siblings, 1 reply; 25+ messages in thread
From: Andreas Schwab @ 2011-11-10 17:05 UTC (permalink / raw)
  To: Paul Eggert; +Cc: Stephen J. Turnbull, emacs-devel

Paul Eggert <eggert@cs.ucla.edu> writes:

> Let me try to state the issue more concretely.
> I don't observe the problem on my host (Fedora 15 x86-64),
> even though it has address space randomization.

You are trapped in your small world.  There is a lot of life beyond
that.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: VIRT_ADDR_VARIES
  2011-11-10 17:05                       ` VIRT_ADDR_VARIES Andreas Schwab
@ 2011-11-10 17:15                         ` Paul Eggert
  0 siblings, 0 replies; 25+ messages in thread
From: Paul Eggert @ 2011-11-10 17:15 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: Stephen J. Turnbull, emacs-devel

On 11/10/11 09:05, Andreas Schwab wrote:
> You are trapped in your small world.

Perhaps, but that's why I'm asking: to find out what the rest
of the universe actually looks like.

Did you try running the command that I suggested?
Does its output match your understanding of why 'valgrind'
breaks things for you?  Here's the command again:

./temacs --batch --eval '(progn (insert-file "/proc/self/maps") (message "%s\n" (buffer-string)))'

to be run once by itself, and once under valgrind on your
platform.



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: VIRT_ADDR_VARIES
  2011-11-10 16:50                             ` VIRT_ADDR_VARIES Paul Eggert
@ 2011-11-10 18:06                               ` Eli Zaretskii
  0 siblings, 0 replies; 25+ messages in thread
From: Eli Zaretskii @ 2011-11-10 18:06 UTC (permalink / raw)
  To: Paul Eggert; +Cc: emacs-devel

> Date: Thu, 10 Nov 2011 08:50:39 -0800
> From: Paul Eggert <eggert@cs.ucla.edu>
> CC: emacs-devel@gnu.org
> 
> On 11/10/11 08:43, Eli Zaretskii wrote:
> > But why would someone do such a thing?
> 
> Performance.
> 
> Comparing against my_edata takes just one instruction.
> Comparing against both the start and the end of 'pure'
> takes two comparisons and some glue instructions and
> typically includes a conditional branch.

I'm surprised.  Saving a few instructions in something that is never
called inside any time-critical code would be more than just premature
optimization in my book.



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: VIRT_ADDR_VARIES
  2011-11-10  8:17                     ` VIRT_ADDR_VARIES Paul Eggert
  2011-11-10  9:29                       ` VIRT_ADDR_VARIES Andreas Schwab
  2011-11-10 11:06                       ` VIRT_ADDR_VARIES Eli Zaretskii
@ 2011-11-14  4:57                       ` Paul Eggert
  2 siblings, 0 replies; 25+ messages in thread
From: Paul Eggert @ 2011-11-14  4:57 UTC (permalink / raw)
  To: emacs-devel

No further comment on the patch itself,
so I've just now filed it as part of a bug report at
<http://debbugs.gnu.org/cgi/bugreport.cgi?bug=10042>.



^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2011-11-14  4:57 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-11-06 17:18 VIRT_ADDR_VARIES Andreas Schwab
2011-11-07  1:50 ` VIRT_ADDR_VARIES Paul Eggert
2011-11-07  8:38   ` VIRT_ADDR_VARIES Andreas Schwab
2011-11-07 23:23     ` VIRT_ADDR_VARIES Paul Eggert
2011-11-08  8:56       ` VIRT_ADDR_VARIES Andreas Schwab
2011-11-08 17:34         ` VIRT_ADDR_VARIES Paul Eggert
2011-11-08 18:38           ` VIRT_ADDR_VARIES Andreas Schwab
2011-11-08 21:17             ` VIRT_ADDR_VARIES Paul Eggert
2011-11-08 22:06               ` VIRT_ADDR_VARIES Andreas Schwab
2011-11-09 17:44                 ` VIRT_ADDR_VARIES Paul Eggert
2011-11-09 21:32                   ` VIRT_ADDR_VARIES Andreas Schwab
2011-11-10  8:17                     ` VIRT_ADDR_VARIES Paul Eggert
2011-11-10  9:29                       ` VIRT_ADDR_VARIES Andreas Schwab
2011-11-10 16:19                         ` VIRT_ADDR_VARIES Paul Eggert
2011-11-10 11:06                       ` VIRT_ADDR_VARIES Eli Zaretskii
2011-11-10 11:20                         ` VIRT_ADDR_VARIES Andreas Schwab
2011-11-10 16:29                         ` VIRT_ADDR_VARIES Paul Eggert
2011-11-10 16:43                           ` VIRT_ADDR_VARIES Eli Zaretskii
2011-11-10 16:50                             ` VIRT_ADDR_VARIES Paul Eggert
2011-11-10 18:06                               ` VIRT_ADDR_VARIES Eli Zaretskii
2011-11-14  4:57                       ` VIRT_ADDR_VARIES Paul Eggert
2011-11-10  4:05                   ` VIRT_ADDR_VARIES Stephen J. Turnbull
2011-11-10  5:40                     ` VIRT_ADDR_VARIES Paul Eggert
2011-11-10 17:05                       ` VIRT_ADDR_VARIES Andreas Schwab
2011-11-10 17:15                         ` VIRT_ADDR_VARIES Paul Eggert

Code repositories for project(s) associated with this public inbox

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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).