bug#716: Bug in buffer-swap-text

bug#716: Bug in buffer-swap-text

[Geoff Gole]

> I cannot seem to reproduce it here, tho I'm not 100% sure how you run
> the above code.  Could you give a more detailed recipe, starting from
> "emacs -Q" and showing whether you use M-: or C-x C-e, ... ?

As I recall I started from an emacs -Q, pasted the code into
*scratch*, and hit C-j.

Currently I'm on a different version of emacs to the one in which
I observed the crashes:

  GNU Emacs 23.0.60.1 (i486-pc-linux-gnu, GTK+ Version 2.12.11) of
2008-11-22 on elegiac, modified by Debian

And with this emacs I can't reproduce the bug.

> Not sure what font backend the user on NetBSD/powerpc was using, but there might
> be something that the xft font backend does that buffer-swap-text is relying on
> which the w32 and one of the x font backends are not doing correctly.

If it's the font backend, then perhaps I won't see the crash with
an emacs -nw -Q. I'll try and test this.

bug#716: Bug in buffer-swap-text

[Jason Rumney]

Geoff Gole wrote:
> If it's the font backend, then perhaps I won't see the crash with
> an emacs -nw -Q. I'll try and test this.
>   

Good point. I can reproduce it with emacs -nw -Q, so it is not the font 
backend.

bug#716: Bug in buffer-swap-text

[Geoff Gole]

>
> Good point. I can reproduce it with emacs -nw -Q, so it is not the font
> backend.
>

Yes, I can reproduce it with -nw -Q as well (under Windows).

bug#716: Bug in buffer-swap-text

[Stefan Monnier]

>> Good point. I can reproduce it with emacs -nw -Q, so it is not the font
>> backend.
> Yes, I can reproduce it with -nw -Q as well (under Windows).

Still no "luck" for me,


        Stefan

bug#716: Bug in buffer-swap-text

[Geoff Gole]

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

I can reliably get a crash without changing coding systems:

(progn
  (buffer-swap-text (generate-new-buffer "test"))
  (insert (make-string 128 ?a)))

(progn
  (buffer-swap-text (generate-new-buffer "test"))
  (garbage-collect))

All the crashes I've seen with tar-mode or
buffer-swap-text end up hitting the same call to abort in
r_re_alloc().

Backtraces for the above lisp fragments (same order):

(gdb) bt
#0  w32_abort () at w32fns.c:7279
#1  0x01137df8 in r_re_alloc (ptr=0x2a27008, size=2129) at ralloc.c:1028
#2  0x0106ed4e in enlarge_buffer_text (b=0x2a27000, delta=2108) at
buffer.c:5065
#3  0x010fe25e in make_gap_larger (nbytes_added=2108) at insdel.c:526
#4  0x010ff949 in insert_from_string_1 (string=47277539, pos=0, pos_byte=0,
    nchars=128, nbytes=128, inherit=0, before_markers=44199944) at
insdel.c:1107
#5  0x01100da6 in insert_from_string (string=47277539, pos=0, pos_byte=0,
length=128,
    length_byte=0, inherit=44199944) at insdel.c:1048
#6  0x0108d79c in general_insert_function (insert_func=0x1100fac <insert>,
    insert_from_string_func=0x1100d3b <insert_from_string>, inherit=0,
nargs=1,
    args=0x82f3e0) at editfns.c:2184
#7  0x0108d86c in Finsert (nargs=0, args=0x0) at editfns.c:2228
#8  0x0100b364 in Feval (form=19643352) at eval.c:2378
#9  0x0100b632 in Fprogn (args=0) at eval.c:449
#10 0x0100b42b in Feval (form=18325836) at eval.c:2322
#11 0x0100be38 in Ffuncall (nargs=2, args=0x1179ef0) at eval.c:3044
#12 0x01111e63 in Fbyte_code (bytestr=0, vector=8582708, maxdepth=1) at
bytecode.c:678
#13 0x0100b722 in funcall_lambda (fun=18972820, nargs=1,
arg_vector=0x82f774)
    at eval.c:3231
#14 0x0100bc17 in Ffuncall (nargs=2, args=0x1218094) at eval.c:3101
#15 0x01111e63 in Fbyte_code (bytestr=0, vector=8583024, maxdepth=1) at
bytecode.c:678
#16 0x0100b722 in funcall_lambda (fun=18973068, nargs=1,
arg_vector=0x82f8b4)
    at eval.c:3231
#17 0x0100bc17 in Ffuncall (nargs=2, args=0x121818c) at eval.c:3101
#18 0x01111e63 in Fbyte_code (bytestr=0, vector=8583344, maxdepth=1) at
bytecode.c:678
#19 0x0100b722 in funcall_lambda (fun=18971276, nargs=0,
arg_vector=0x82fa24)
    at eval.c:3231
#20 0x0100bc17 in Ffuncall (nargs=1, args=0x1217a8c) at eval.c:3101
#21 0x0100d37f in apply1 (fn=53934297, arg=0) at eval.c:2785
#22 0x0110fdc5 in Fcall_interactively (function=53934297,
record_flag=44161025,
    keys=44194564) at callint.c:389
#23 0x0100be11 in Ffuncall (nargs=4, args=0x12bf728) at eval.c:3050
#24 0x0100bfe9 in call3 (fn=0, arg1=0, arg2=0, arg3=0) at eval.c:2870
#25 0x01056cb9 in Fcommand_execute (cmd=53934297, record_flag=44161025,
keys=0,
    special=44161025) at keyboard.c:10333
#26 0x0105e4e5 in command_loop_1 () at keyboard.c:1880
#27 0x01009f9e in internal_condition_case (bfun=0x105e180 <command_loop_1>,
    handlers=44224777, hfun=0x105772c <cmd_error>) at eval.c:1511
#28 0x01051cba in command_loop_2 () at keyboard.c:1338
#29 0x01009ed3 in internal_catch (tag=0, func=0x1051c97 <command_loop_2>,
arg=44161025)
    at eval.c:1247
#30 0x01051ac7 in command_loop () at keyboard.c:1317
#31 0x01051b60 in recursive_edit_1 () at keyboard.c:942
#32 0x01051c81 in Frecursive_edit () at keyboard.c:1004
#33 0x01002e36 in main (argc=1, argv=0xa427b8) at emacs.c:1777

Lisp Backtrace:
"insert" (0x82f3e0)
"progn" (0x82f518)
"eval" (0x82f638)
"eval-last-sexp-1" (0x82f774)
"eval-last-sexp" (0x82f8b4)
"eval-print-last-sexp" (0x82fa24)
"call-interactively" (0x82fc04)
(gdb)


(gdb) bt
#0  w32_abort () at w32fns.c:7279
#1  0x01137df8 in r_re_alloc (ptr=0x2d56008, size=100) at ralloc.c:1028
#2  0x0106ed4e in enlarge_buffer_text (b=0x2d56000, delta=-1979) at
buffer.c:5065
#3  0x010fe38a in make_gap_smaller (nbytes_removed=-1979) at insdel.c:600
#4  0x01065cab in Fgarbage_collect () at alloc.c:5022
#5  0x0100b383 in Feval (form=18331376) at eval.c:2372
#6  0x0100b632 in Fprogn (args=0) at eval.c:449
#7  0x0100b42b in Feval (form=18325836) at eval.c:2322
#8  0x0100be38 in Ffuncall (nargs=2, args=0x1179ef0) at eval.c:3044
#9  0x01111e63 in Fbyte_code (bytestr=0, vector=8582708, maxdepth=1) at
bytecode.c:678
#10 0x0100b722 in funcall_lambda (fun=18972820, nargs=1,
arg_vector=0x82f774)
    at eval.c:3231
#11 0x0100bc17 in Ffuncall (nargs=2, args=0x1218094) at eval.c:3101
#12 0x01111e63 in Fbyte_code (bytestr=0, vector=8583024, maxdepth=1) at
bytecode.c:678
#13 0x0100b722 in funcall_lambda (fun=18973068, nargs=1,
arg_vector=0x82f8b4)
    at eval.c:3231
#14 0x0100bc17 in Ffuncall (nargs=2, args=0x121818c) at eval.c:3101
#15 0x01111e63 in Fbyte_code (bytestr=0, vector=8583344, maxdepth=1) at
bytecode.c:678
#16 0x0100b722 in funcall_lambda (fun=18971276, nargs=0,
arg_vector=0x82fa24)
    at eval.c:3231
#17 0x0100bc17 in Ffuncall (nargs=1, args=0x1217a8c) at eval.c:3101
#18 0x0100d37f in apply1 (fn=53934297, arg=0) at eval.c:2785
#19 0x0110fdc5 in Fcall_interactively (function=53934297,
record_flag=44161025,
    keys=44194564) at callint.c:389
#20 0x0100be11 in Ffuncall (nargs=4, args=0x12bf728) at eval.c:3050
#21 0x0100bfe9 in call3 (fn=0, arg1=0, arg2=0, arg3=0) at eval.c:2870
#22 0x01056cb9 in Fcommand_execute (cmd=53934297, record_flag=44161025,
keys=0,
    special=44161025) at keyboard.c:10333
#23 0x0105e4e5 in command_loop_1 () at keyboard.c:1880
#24 0x01009f9e in internal_condition_case (bfun=0x105e180 <command_loop_1>,
    handlers=44224777, hfun=0x105772c <cmd_error>) at eval.c:1511
#25 0x01051cba in command_loop_2 () at keyboard.c:1338
#26 0x01009ed3 in internal_catch (tag=0, func=0x1051c97 <command_loop_2>,
arg=44161025)
    at eval.c:1247
#27 0x01051ac7 in command_loop () at keyboard.c:1317
#28 0x01051b60 in recursive_edit_1 () at keyboard.c:942
#29 0x01051c81 in Frecursive_edit () at keyboard.c:1004
#30 0x01002e36 in main (argc=1, argv=0xa427b8) at emacs.c:1777

Lisp Backtrace:
"garbage-collect" (0x82f420)
"progn" (0x82f518)
"eval" (0x82f638)
"eval-last-sexp-1" (0x82f774)
"eval-last-sexp" (0x82f8b4)
"eval-print-last-sexp" (0x82fa24)
"call-interactively" (0x82fc04)
(gdb)

[-- Attachment #2: Type: text/html, Size: 6981 bytes --]

bug#716: Bug in buffer-swap-text

[Magnus Henoch]

"Geoff Gole" <geoffgole@gmail.com> writes:

> I can reliably get a crash without changing coding systems:
>
> (progn
>   (buffer-swap-text (generate-new-buffer "test"))
>   (insert (make-string 128 ?a)))
>
> (progn
>   (buffer-swap-text (generate-new-buffer "test"))
>   (garbage-collect))

I can confirm that either of these give a crash on NetBSD/powerpc, so
the problem might not be Windows-specific.

Magnus

bug#716: Bug in buffer-swap-text

[Stefan Monnier]

> I can reliably get a crash without changing coding systems:
> (progn
>   (buffer-swap-text (generate-new-buffer "test"))
>   (insert (make-string 128 ?a)))

> (progn
>   (buffer-swap-text (generate-new-buffer "test"))
>   (garbage-collect))

I cannot seem to reproduce it here, tho I'm not 100% sure how you run
the above code.  Could you give a more detailed recipe, starting from
"emacs -Q" and showing whether you use M-: or C-x C-e, ... ?


        Stefan

bug#716: Bug in buffer-swap-text

[jasonr]

Quoting Stefan Monnier <monnier@IRO.UMontreal.CA>:

> > I can reliably get a crash without changing coding systems:
> > (progn
> >   (buffer-swap-text (generate-new-buffer "test"))
> >   (insert (make-string 128 ?a)))

I get a crash at this point using C-x C-e on this line as the first activity in
emacs -Q, after pasting the above into *scratch* and removing the > quotations.

> > (progn
> >   (buffer-swap-text (generate-new-buffer "test"))
> >   (garbage-collect))
>
> I cannot seem to reproduce it here, tho I'm not 100% sure how you run
> the above code.  Could you give a more detailed recipe, starting from
> "emacs -Q" and showing whether you use M-: or C-x C-e, ... ?

Can you reproduce it by evaluating the following first:

(set-default-coding-systems 'iso-latin-1-dos)

Possibly not, as setting the coding systems to ...-unix does not make the
problem go away for me, and the user who saw the bug on NetBSD/powerpc did not
report doing anything special like this to trigger the bug.

bug#716: Bug in buffer-swap-text

[Stefan Monnier]

>> > I can reliably get a crash without changing coding systems:
>> > (progn
>> >   (buffer-swap-text (generate-new-buffer "test"))
>> >   (insert (make-string 128 ?a)))
> I get a crash at this point using C-x C-e on this line as the first
> activity in emacs -Q, after pasting the above into *scratch* and
> removing the > quotations.
> Can you reproduce it by evaluating the following first:
> (set-default-coding-systems 'iso-latin-1-dos)

No, that doesn't help.  I'm still not able to reproduce it.


        Stefan

bug#716: Bug in buffer-swap-text

[jasonr]

Quoting Stefan Monnier <monnier@iro.umontreal.ca>:

> >> > I can reliably get a crash without changing coding systems:
> >> > (progn
> >> >   (buffer-swap-text (generate-new-buffer "test"))
> >> >   (insert (make-string 128 ?a)))
> > I get a crash at this point using C-x C-e on this line as the first
> > activity in emacs -Q, after pasting the above into *scratch* and
> > removing the > quotations.
> > Can you reproduce it by evaluating the following first:
> > (set-default-coding-systems 'iso-latin-1-dos)
>
> No, that doesn't help.  I'm still not able to reproduce it.

Since the crashes seem to be happening when the swapped buffers are subsequently
displayed, how about trying different font backends, for example:
  emacs -Q -xrm Emacs.fontBackend:x

Not sure what font backend the user on NetBSD/powerpc was using, but there might
be something that the xft font backend does that buffer-swap-text is relying on
which the w32 and one of the x font backends are not doing correctly.

bug#716: Bug in buffer-swap-text

[Jason Rumney]

Stefan Monnier wrote:
>> I can reliably get a crash without changing coding systems:
>> (progn
>>   (buffer-swap-text (generate-new-buffer "test"))
>>   (insert (make-string 128 ?a)))
>>     
>> (progn
>>   (buffer-swap-text (generate-new-buffer "test"))
>>   (garbage-collect))
>>     
>
> I cannot seem to reproduce it here, tho I'm not 100% sure how you run
> the above code.  Could you give a more detailed recipe, starting from
> "emacs -Q" and showing whether you use M-: or C-x C-e, ... ?
>   

One possible variable here is the way that buffer space is allocated. On 
Windows, it seems REL_ALLOC is defined. I presume GNU/Linux defines 
USE_MMAP_FOR_BUFFERS which would cause it to take a different code path 
around the point where we see a crash on Windows, and as Magnus Henoch 
saw on NetBSD/powerpc also (though we don't have a stack trace for that 
crash, so can't tell for sure it is crashing in the same place).

Magnus, does src/config.h have either of these constants defined on your 
NetBSD/powerpc machine? A third possibility if neither of those are 
defined is that xmalloc/xfree/xrealloc are used.

bug#716: Bug in buffer-swap-text

[Jason Rumney]

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

Jason Rumney wrote:
> One possible variable here is the way that buffer space is allocated. 
> On Windows, it seems REL_ALLOC is defined. I presume GNU/Linux defines 
> USE_MMAP_FOR_BUFFERS which would cause it to take a different code 
> path around the point where we see a crash on Windows, and as Magnus 
> Henoch saw on NetBSD/powerpc also (though we don't have a stack trace 
> for that crash, so can't tell for sure it is crashing in the same place).
The following patch seems to fix the problem, does it look correct to 
others who might understand ralloc.c and buffer_swap_text better than I do?



[-- Attachment #2: bug716.diff --]
[-- Type: text/plain, Size: 1546 bytes --]

Index: buffer.c
===================================================================
RCS file: /sources/emacs/emacs/src/buffer.c,v
retrieving revision 1.575
diff -r1.575 buffer.c
2184a2185,2189
> #ifdef REL_ALLOC
> extern void r_alloc_prepare_to_swap_pointers P_ ((POINTER_TYPE **,
> 						  POINTER_TYPE **));
> #endif
> 
2222a2228,2232
> #ifdef REL_ALLOC
>   r_alloc_prepare_to_swap_pointers (&current_buffer->own_text.beg,
> 				    &other_buffer->own_text.beg);
> #endif
> 
Index: ralloc.c
===================================================================
RCS file: /sources/emacs/emacs/src/ralloc.c,v
retrieving revision 1.69
diff -r1.69 ralloc.c
1225a1226,1248
> /* Swap relocatable data between two pointers.
>    This is used by buffer_swap_text.  Since buffer_swap_text swaps the
>    whole text structure in one go, this function has been written to only
>    update the internal pointers back to the variables, ready for when the
>    swap is actually done.  It must be called before the pointers are
>    swapped so that the state is consistent when find_bloc is called.  */
> void
> r_alloc_prepare_to_swap_pointers (p1, p2)
>      POINTER *p1, *p2;
> {
>   bloc_ptr bloc1, bloc2;
>   bloc1 = find_bloc (p1);
>   bloc2 = find_bloc (p2);
>   if (bloc1 == NIL_BLOC || bloc2 == NIL_BLOC)
>     abort ();
> 
>   /* Swap internal pointers back to the variables.  */
>   bloc1->variable = p2;
>   bloc2->variable = p1;
> 
>   /* It would be cleaner to do the actual swap here too, but it would
>      complicate buffer_swap_text.  */
> }

bug#716: Bug in buffer-swap-text

[Jason Rumney]

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

Jason Rumney wrote:
> Jason Rumney wrote:
>> One possible variable here is the way that buffer space is allocated. 
>> On Windows, it seems REL_ALLOC is defined. I presume GNU/Linux 
>> defines USE_MMAP_FOR_BUFFERS which would cause it to take a different 
>> code path around the point where we see a crash on Windows, and as 
>> Magnus Henoch saw on NetBSD/powerpc also (though we don't have a 
>> stack trace for that crash, so can't tell for sure it is crashing in 
>> the same place).
> The following patch seems to fix the problem, does it look correct to 
> others who might understand ralloc.c and buffer_swap_text better than 
> I do?

Sorry, once more with context:



[-- Attachment #2: bug716.diff --]
[-- Type: text/plain, Size: 2369 bytes --]

Index: buffer.c
===================================================================
RCS file: /sources/emacs/emacs/src/buffer.c,v
retrieving revision 1.575
diff -c -r1.575 buffer.c
*** buffer.c	9 Dec 2008 23:08:05 -0000	1.575
--- buffer.c	23 Dec 2008 14:37:33 -0000
***************
*** 2182,2187 ****
--- 2182,2192 ----
    return byte_pos;
  }
  
+ #ifdef REL_ALLOC
+ extern void r_alloc_prepare_to_swap_pointers P_ ((POINTER_TYPE **,
+ 						  POINTER_TYPE **));
+ #endif
+ 
  DEFUN ("buffer-swap-text", Fbuffer_swap_text, Sbuffer_swap_text,
         1, 1, 0,
         doc: /* Swap the text between current buffer and BUFFER.  */)
***************
*** 2220,2225 ****
--- 2225,2235 ----
      current_buffer->field = tmp##field;			\
    } while (0)
  
+ #ifdef REL_ALLOC
+   r_alloc_prepare_to_swap_pointers (&current_buffer->own_text.beg,
+ 				    &other_buffer->own_text.beg);
+ #endif
+ 
    swapfield (own_text, struct buffer_text);
    eassert (current_buffer->text == &current_buffer->own_text);
    eassert (other_buffer->text == &other_buffer->own_text);
Index: ralloc.c
===================================================================
RCS file: /sources/emacs/emacs/src/ralloc.c,v
retrieving revision 1.69
diff -c -r1.69 ralloc.c
*** ralloc.c	21 Nov 2008 12:14:07 -0000	1.69
--- ralloc.c	23 Dec 2008 14:40:52 -0000
***************
*** 1223,1228 ****
--- 1223,1251 ----
  
  #endif /* DEBUG */
  
+ /* Swap relocatable data between two pointers.
+    This is used by buffer_swap_text.  Since buffer_swap_text swaps the
+    whole text structure in one go, this function has been written to only
+    update the internal pointers back to the variables, ready for when the
+    swap is actually done.  It must be called before the pointers are
+    swapped so that the state is consistent when find_bloc is called.  */
+ void
+ r_alloc_prepare_to_swap_pointers (p1, p2)
+      POINTER *p1, *p2;
+ {
+   bloc_ptr bloc1, bloc2;
+   bloc1 = find_bloc (p1);
+   bloc2 = find_bloc (p2);
+   if (bloc1 == NIL_BLOC || bloc2 == NIL_BLOC)
+     abort ();
+ 
+   /* Swap internal pointers back to the variables.  */
+   bloc1->variable = p2;
+   bloc2->variable = p1;
+ 
+   /* It would be cleaner to do the actual swap here too, but it would
+      complicate buffer_swap_text.  */
+ }
  
  \f
  /***********************************************************************

bug#716: Bug in buffer-swap-text

[Stefan Monnier]

>>> One possible variable here is the way that buffer space is allocated. On
>>> Windows, it seems REL_ALLOC is defined. I presume GNU/Linux defines
>>> USE_MMAP_FOR_BUFFERS which would cause it to take a different code path
>>> around the point where we see a crash on Windows, and as Magnus Henoch
>>> saw on NetBSD/powerpc also (though we don't have a stack trace for that
>>> crash, so can't tell for sure it is crashing in the same place).
>> The following patch seems to fix the problem, does it look correct to
>> others who might understand ralloc.c and buffer_swap_text better than
>> I do?
> Sorry, once more with context:

Your analysis sounds right, thank you.  I'd suggest to use another
r_alloc primitve, something like r_alloc_reset_variable, so you could do

  r_alloc_reset_variable(&current_buffer->own_text.beg);
  r_alloc_reset_variable(&other_buffer->own_text.beg);

after the swap.  It could use the untested patch below.  WDYT?


        Stefan


=== modified file 'src/ralloc.c'
--- src/ralloc.c	2008-11-21 19:14:07 +0000
+++ src/ralloc.c	2008-12-23 17:23:02 +0000
@@ -402,7 +402,7 @@
 
   while (p != NIL_BLOC)
     {
-      if (p->variable == ptr && p->data == *ptr)
+      if (p->data == *ptr)
 	return p;
 
       p = p->next;
@@ -1223,6 +1223,15 @@
 
 #endif /* DEBUG */
 
+/* Change the variable of the bloc pointed from `p' to be `p'.  */
+void r_alloc_reset_variable (POINTER *p)
+{
+  bloc_ptr bloc = find_bloc (p);
+  if (bloc == NIL_BLOC)
+    abort ();
+  bloc->variable = p;
+}
+
 
 \f
 /***********************************************************************

bug#716: Bug in buffer-swap-text

[Jason Rumney]

Stefan Monnier wrote:
> Your analysis sounds right, thank you.  I'd suggest to use another
> r_alloc primitve, something like r_alloc_reset_variable, so you could do
>
>   r_alloc_reset_variable(&current_buffer->own_text.beg);
>   r_alloc_reset_variable(&other_buffer->own_text.beg);
>
> after the swap.  It could use the untested patch below.  WDYT?
>
>
> === modified file 'src/ralloc.c'
> --- src/ralloc.c	2008-11-21 19:14:07 +0000
> +++ src/ralloc.c	2008-12-23 17:23:02 +0000
> @@ -402,7 +402,7 @@
>  
>    while (p != NIL_BLOC)
>      {
> -      if (p->variable == ptr && p->data == *ptr)
> +      if (p->data == *ptr)
>  	return p;
>  
>        p = p->next;
>   
This removes the consistency check, without which we would have taken 
much longer to find this problem, as Emacs would not have aborted when 
no bloc could be found, and the problem would have been memory 
corruption later when that bloc of memory was moved and the wrong 
pointer updated.

bug#716: Bug in buffer-swap-text

[jasonr]

Quoting Stefan Monnier <monnier@iro.umontreal.ca>:

> Your analysis sounds right, thank you.  I'd suggest to use another
> r_alloc primitve, something like r_alloc_reset_variable, so you could do

I've adapted your suggestion so it does not remove the consistency check in
find_bloc (instead it does a non-checking find inline):

*** buffer.c.~1.575.~	2008-12-10 21:52:40.562500000 +0800
--- buffer.c	2008-12-24 09:12:15.536522600 +0800
***************
*** 2182,2187 ****
--- 2182,2191 ----
    return byte_pos;
  }

+ #ifdef REL_ALLOC
+ extern void r_alloc_reset_variable P_ ((PTR *));
+ #endif
+
  DEFUN ("buffer-swap-text", Fbuffer_swap_text, Sbuffer_swap_text,
         1, 1, 0,
         doc: /* Swap the text between current buffer and BUFFER.  */)
***************
*** 2223,2228 ****
--- 2227,2237 ----
    swapfield (own_text, struct buffer_text);
    eassert (current_buffer->text == &current_buffer->own_text);
    eassert (other_buffer->text == &other_buffer->own_text);
+ #ifdef REL_ALLOC
+   r_alloc_reset_variable ((PTR *) &current_buffer->own_text.beg);
+   r_alloc_reset_variable ((PTR *) &other_buffer->own_text.beg);
+ #endif
+
    swapfield (pt, EMACS_INT);
    swapfield (pt_byte, EMACS_INT);
    swapfield (begv, EMACS_INT);
*** ralloc.c.~1.69.~	2008-11-23 22:34:01.250000000 +0800
--- ralloc.c	2008-12-24 09:12:35.643967100 +0800
***************
*** 1223,1228 ****
--- 1223,1254 ----

  #endif /* DEBUG */

+ /* Update the internal record of which variable points to some data.
+    Used by buffer-swap-text in Emacs to restore consistency after it
+    swaps the buffer text between two buffer objects.  */
+ void
+ r_alloc_reset_variable (ptr)
+      POINTER *ptr;
+ {
+   bloc_ptr bloc = first_bloc;
+
+   /* Find the bloc that corresponds to the data pointed to by pointer.
+      find_bloc cannot be used, as it has internal consistency checks
+      which fail when the variable needs reseting.  */
+   while (bloc != NIL_BLOC)
+     {
+       if (bloc->data == *ptr)
+ 	break;
+
+       bloc = bloc->next;
+     }
+
+   if (bloc == NIL_BLOC)
+     abort ();
+
+   /* Update variable to point to the new location.  */
+   bloc->variable = ptr;
+ }

  \f
  /***********************************************************************

bug#716: Bug in buffer-swap-text

[Stefan Monnier]

>> Your analysis sounds right, thank you.  I'd suggest to use another
>> r_alloc primitve, something like r_alloc_reset_variable, so you could do

> I've adapted your suggestion so it does not remove the consistency check in
> find_bloc (instead it does a non-checking find inline):

Thanks.  After your email, I thought we could just change
r_alloc_reset_variable to take 2 arguments: the old ptr and the
new ptr.  This way, you get more consistency checking.

BTW, if the double check (p->variable == ptr && p->data == *ptr) is
there for consistency, it would be good to add a comment about it.

Better yet: make it a real consistency check, along the lines of

  assert (*p->variable == p->data);

so it's not just checked for `ptr', but for all blocs visited in the loop.


        Stefan