bug#9273: 23.3; malloc initialization should (sometimes) happen at runtime

bug#9273: 23.3; malloc initialization should (sometimes) happen at runtime

[Ken Brown]

The code in src/gmalloc.c makes assumptions about how a system maintains 
its memory that are not necessarily valid.  In particular, they will not 
be valid on Cygwin starting with version 1.7.10 (which will almost 
certainly be released before emacs 24.1).  The problem is that malloc 
initialization is done by temacs, and the results are dumped into emacs. 
  This includes the setting __malloc_initialized = 1, so no malloc 
initialization is done when emacs is run.  But the dumped value of 
_heapbase, while appropriate for temacs, may not point to the beginning 
of the runtime heap for emacs.  This causes all code that uses the BLOCK 
and ADDRESS macros to be invalid.

Here's what happens on Cygwin.  temacs (on Cygwin) uses a static buffer 
as its heap and a function bss_sbrk that simulates sbrk.  (See 
src/sheap.c.)  The data in this buffer, including malloc information, 
are then dumped into emacs.exe as initialized data.  But when the dumped 
emacs is run, it uses Cygwin's sbrk, which allocates memory on a heap 
that won't (as of Cygwin 1.7.10) be contiguous with the static heap. 
The saved value of _heapbase, which points into the static heap, is 
never changed, but it will mess up later calculations as soon as sbrk is 
called for the first time.

All of this is described in detail on the Cygwin mailing list in the 
thread starting at

   http://cygwin.com/ml/cygwin/2011-08/msg00153.html

See especially

   http://cygwin.com/ml/cygwin/2011-08/msg00193.html

which contains a gdb session illustrating the problem.  The context for 
that session is that, as a result of the problem I'm reporting, 
morecore_nolock went into an infinite loop.  I attached gdb to that 
looping process.

Maybe the solution is for emacs to do malloc initialization, including 
the assignment of _heapbase, every time it starts, at least on systems 
that use gmalloc.c.  I made one naive attempt to do this, but it didn't 
work (and it was Cygwin specific).  Namely, I made unexec (for Cygwin) 
set _malloc_initialized = 0 before dumping.  The resulting emacs aborted 
as soon as it was started.  I haven't figured out what went wrong, but 
I'm not sure that's the right answer anyway.

Ken

bug#9273: 23.3; malloc initialization should (sometimes) happen at runtime

[Richard Stallman]

If Cygwin does something very strange, there are limits to how far
we would want to change Emacs to cope with that.

-- 
Dr Richard Stallman
President, Free Software Foundation
51 Franklin St
Boston MA 02110
USA
www.fsf.org  www.gnu.org
Skype: No way! That's nonfree (freedom-denying) software.
  Use free telephony http://directory.fsf.org/category/tel/

bug#9273: 23.3; malloc initialization should (sometimes) happen at runtime

[Eli Zaretskii]

> Date: Tue, 09 Aug 2011 16:09:48 -0400
> From: Ken Brown <kbrown@cornell.edu>
> 
> The code in src/gmalloc.c makes assumptions about how a system maintains 
> its memory that are not necessarily valid.  In particular, they will not 
> be valid on Cygwin starting with version 1.7.10 (which will almost 
> certainly be released before emacs 24.1).  The problem is that malloc 
> initialization is done by temacs, and the results are dumped into emacs. 
>   This includes the setting __malloc_initialized = 1, so no malloc 
> initialization is done when emacs is run.  But the dumped value of 
> _heapbase, while appropriate for temacs, may not point to the beginning 
> of the runtime heap for emacs.  This causes all code that uses the BLOCK 
> and ADDRESS macros to be invalid.

If Cygwin developers cannot or won't add to the Cygwin memory
allocation enough features and knobs to cater to the special needs of
Emacs dumping, then your only hope is to make Cygwin-specific changes
to Emacs.

You will see that 2 other ports that need to live with Windows memory
allocation either have such knobs and features (DJGPP, used to build
the DOS port; see the beginning of src/msdos.c) or use their own
emulation of sbrk that upholds the contract expected by gmalloc.c and
ralloc.c (w32heap.c, for the native Windows build).

> But when the dumped emacs is run, it uses Cygwin's sbrk, which
> allocates memory on a heap that won't (as of Cygwin 1.7.10) be
> contiguous with the static heap.  The saved value of _heapbase,
> which points into the static heap, is never changed, but it will
> mess up later calculations as soon as sbrk is called for the first
> time.

Are you sure this is all that's at work here?  AFAIR, gmalloc does
have code to cope with non-contiguous memory regions returned by sbrk.

> All of this is described in detail on the Cygwin mailing list in the 
> thread starting at
> 
>    http://cygwin.com/ml/cygwin/2011-08/msg00153.html
> 
> See especially
> 
>    http://cygwin.com/ml/cygwin/2011-08/msg00193.html

I have read all the discussion there, but I'm sorry to say that I
cannot figure out what you are talking about: there's too much
Cygwin-isms in that thread that I couldn't penetrate.

> Maybe the solution is for emacs to do malloc initialization, including 
> the assignment of _heapbase, every time it starts, at least on systems 
> that use gmalloc.c.

Most supported systems don't need that.  The native Windows build
indeed does, see w32heap.c.  Perhaps you could reuse some or even most
of it for Cygwin.  (What is so special about the Cygwin sbrk that is
worth sticking to it?)

> I made one naive attempt to do this, but it didn't work (and it was
> Cygwin specific).  Namely, I made unexec (for Cygwin) set
> _malloc_initialized = 0 before dumping.  The resulting emacs aborted
> as soon as it was started.  I haven't figured out what went wrong,
> but I'm not sure that's the right answer anyway.

One more evidence that something else is at work here.  I would
suggest to walk through the session that reinitializes the heap after
unexec and see what goes wrong there.

bug#9273: 23.3; malloc initialization should (sometimes) happen at runtime

[Ken Brown]

On 8/10/2011 11:56 AM, Eli Zaretskii wrote:
>> Date: Tue, 09 Aug 2011 16:09:48 -0400
>> From: Ken Brown<kbrown@cornell.edu>
>
>> But when the dumped emacs is run, it uses Cygwin's sbrk, which
>> allocates memory on a heap that won't (as of Cygwin 1.7.10) be
>> contiguous with the static heap.  The saved value of _heapbase,
>> which points into the static heap, is never changed, but it will
>> mess up later calculations as soon as sbrk is called for the first
>> time.
>
> Are you sure this is all that's at work here?  AFAIR, gmalloc does
> have code to cope with non-contiguous memory regions returned by sbrk.

The issue isn't that sbrk returns non-contiguous regions.  The issue is 
that two different of sbrk are used.  One is used when temacs is 
running, and a different one is used when the dumped emacs.exe is 
running.  This is controlled by the Cygwin-specific code in gmalloc.c, 
and it has to do with the way unexec works in the Cygwin build of emacs.

>> All of this is described in detail on the Cygwin mailing list in the
>> thread starting at
>>
>>     http://cygwin.com/ml/cygwin/2011-08/msg00153.html
>>
>> See especially
>>
>>     http://cygwin.com/ml/cygwin/2011-08/msg00193.html
>
> I have read all the discussion there, but I'm sorry to say that I
> cannot figure out what you are talking about: there's too much
> Cygwin-isms in that thread that I couldn't penetrate.

Thanks for trying.

>> Maybe the solution is for emacs to do malloc initialization, including
>> the assignment of _heapbase, every time it starts, at least on systems
>> that use gmalloc.c.
>
> Most supported systems don't need that.  The native Windows build
> indeed does, see w32heap.c.  Perhaps you could reuse some or even most
> of it for Cygwin.  (What is so special about the Cygwin sbrk that is
> worth sticking to it?)

Thanks for the suggestions.

Ken

bug#9273: 23.3; malloc initialization should (sometimes) happen at runtime

[Eli Zaretskii]

> Date: Wed, 10 Aug 2011 13:52:39 -0400
> From: Ken Brown <kbrown@cornell.edu>
> CC: "9273@debbugs.gnu.org" <9273@debbugs.gnu.org>
> 
> On 8/10/2011 11:56 AM, Eli Zaretskii wrote:
> >> Date: Tue, 09 Aug 2011 16:09:48 -0400
> >> From: Ken Brown<kbrown@cornell.edu>
> >
> >> But when the dumped emacs is run, it uses Cygwin's sbrk, which
> >> allocates memory on a heap that won't (as of Cygwin 1.7.10) be
> >> contiguous with the static heap.  The saved value of _heapbase,
> >> which points into the static heap, is never changed, but it will
> >> mess up later calculations as soon as sbrk is called for the first
> >> time.
> >
> > Are you sure this is all that's at work here?  AFAIR, gmalloc does
> > have code to cope with non-contiguous memory regions returned by sbrk.
> 
> The issue isn't that sbrk returns non-contiguous regions.  The issue is 
> that two different of sbrk are used.  One is used when temacs is 
> running, and a different one is used when the dumped emacs.exe is 
> running.

I still don't see the problem: the memory sbrk'ed before dumping is
frozen in the dumped Emacs, so I don't see how that could matter.
Perhaps I'm missing something.

bug#9273: 23.3; malloc initialization should (sometimes) happen at runtime

[Ken Brown]

On 8/10/2011 2:10 PM, Eli Zaretskii wrote:
>> Date: Wed, 10 Aug 2011 13:52:39 -0400
>> From: Ken Brown<kbrown@cornell.edu>
>> CC: "9273@debbugs.gnu.org"<9273@debbugs.gnu.org>
>>
>> On 8/10/2011 11:56 AM, Eli Zaretskii wrote:
>>>> Date: Tue, 09 Aug 2011 16:09:48 -0400
>>>> From: Ken Brown<kbrown@cornell.edu>
>>>
>>>> But when the dumped emacs is run, it uses Cygwin's sbrk, which
>>>> allocates memory on a heap that won't (as of Cygwin 1.7.10) be
>>>> contiguous with the static heap.  The saved value of _heapbase,
>>>> which points into the static heap, is never changed, but it will
>>>> mess up later calculations as soon as sbrk is called for the first
>>>> time.
>>>
>>> Are you sure this is all that's at work here?  AFAIR, gmalloc does
>>> have code to cope with non-contiguous memory regions returned by sbrk.
>>
>> The issue isn't that sbrk returns non-contiguous regions.  The issue is
>> that two different of sbrk are used.  One is used when temacs is
>> running, and a different one is used when the dumped emacs.exe is
>> running.
>
> I still don't see the problem: the memory sbrk'ed before dumping is
> frozen in the dumped Emacs, so I don't see how that could matter.
> Perhaps I'm missing something.

The memory sbrk'ed before dumping is in the static heap, which is 
somewhere in relatively low memory.  All the variables that malloc uses 
for keeping track of this involve these low addresses.  But when the 
dumped emacs is run, Cygwin's sbrk is called, and it returns addresses 
starting at wherever Cygwin decides to put the heap (which will be 
either 0x20000000 or 0x80000000 in Cygwin 1.7.10, depending on whether 
or not large address awareness is enabled for emacs.exe and is supported 
by the underlying Windows system).

The calculations done in gmalloc.c are based on the assumption that the 
heap starts in the same place in the dumped executable as it did before 
dumping.  See especially the BLOCK and ADDRESS macros, which use the 
_heapbase variable.  But _heapbase was set before dumping, and it points 
somewhere in the static heap; this is now much lower than the beginning 
of the runtime heap.

The specific problem that led me to notice this was that under some 
circumstances emacs went into an infinite loop when executing the 
following (from morecore_nolock in gmalloc.c):

newsize = heapsize;
do
   newsize *= 2;
while ((__malloc_size_t) BLOCK ((char *) result + size) > newsize);

Here `result' is very large, and BLOCK returns a large number because 
it's using a small _heapbase.  So the test is always true, newsize 
becomes 0 because of overflow, and the loop never terminates.

Aside from the infinite loop, however, BLOCK and ADDRESS simply yield 
results that don't make sense when the heap starts in high memory but 
_heapbase points to low memory.

Surprisingly, I haven't yet run into any problems when Cygwin's heap 
starts at 0x20000000.  (It was 0x80000000 in the situation above.)  I 
don't know if there's a good reason for this or if it's just luck.  I 
think what happens is that malloc behaves as if it's allowed to allocate 
memory ranging all the way from the static heap to 0x20000000 and 
beyond.  If for some reason it really is legal for malloc to use the 
memory between the static heap and 0x20000000, then I guess there's no 
harm done as long as the large addresses don't lead to overflow.

Ken

bug#9273: 23.3; malloc initialization should (sometimes) happen at runtime

[Ken Brown]

On 8/10/2011 11:56 AM, Eli Zaretskii wrote:
>> Date: Tue, 09 Aug 2011 16:09:48 -0400
>> From: Ken Brown<kbrown@cornell.edu>
>> I made one naive attempt to do this, but it didn't work (and it was
>> Cygwin specific).  Namely, I made unexec (for Cygwin) set
>> _malloc_initialized = 0 before dumping.  The resulting emacs aborted
>> as soon as it was started.  I haven't figured out what went wrong,
>> but I'm not sure that's the right answer anyway.

The problem was that realloc got called on memory that had been 
allocated prior to dumping, and the malloc information that was used 
then had disappeared.  I think there's an obvious solution to this.  At 
the time of reinitialization, we save the previous malloc state.  Then 
if realloc is called on a pointer to something in the static heap, we 
temporarily restore the old state and let realloc proceed as it did in 
temacs prior to dumping.

Unless I've (again) missed something obvious, it shouldn't be too hard
to do this.  I'm about to go on vacation, but I should have a patch 
ready within a couple weeks.

Ken

bug#9273: 23.3; malloc initialization should (sometimes) happen at runtime

[Eli Zaretskii]

> Date: Thu, 11 Aug 2011 17:45:41 -0400
> From: Ken Brown <kbrown@cornell.edu>
> CC: "9273@debbugs.gnu.org" <9273@debbugs.gnu.org>
> 
> The problem was that realloc got called on memory that had been 
> allocated prior to dumping, and the malloc information that was used 
> then had disappeared.

Can you show the code which called realloc on that memory?  I'm
surprised that Emacs does that, but perhaps I'm missing something.

bug#9273: 23.3; malloc initialization should (sometimes) happen at runtime

[Ken Brown]

On 8/12/2011 2:54 AM, Eli Zaretskii wrote:
>> Date: Thu, 11 Aug 2011 17:45:41 -0400
>> From: Ken Brown<kbrown@cornell.edu>
>> CC: "9273@debbugs.gnu.org"<9273@debbugs.gnu.org>
>>
>> The problem was that realloc got called on memory that had been
>> allocated prior to dumping, and the malloc information that was used
>> then had disappeared.
>
> Can you show the code which called realloc on that memory?  I'm
> surprised that Emacs does that, but perhaps I'm missing something.

Here's the code that I stumbled across (as a result of a SEGV).  I 
haven't checked to see if there are other examples.  From terminal.c:

/* Deletes the bootstrap terminal device.
    Called through delete_terminal_hook. */

static void
delete_initial_terminal (struct terminal *terminal)
{
   if (terminal != initial_terminal)
     abort ();

   delete_terminal (terminal);
   initial_terminal = NULL;
}

bug#9273: 23.3; malloc initialization should (sometimes) happen at runtime

[Eli Zaretskii]

> Date: Fri, 12 Aug 2011 06:10:35 -0400
> From: Ken Brown <kbrown@cornell.edu>
> CC: "9273@debbugs.gnu.org" <9273@debbugs.gnu.org>
> 
> On 8/12/2011 2:54 AM, Eli Zaretskii wrote:
> >> Date: Thu, 11 Aug 2011 17:45:41 -0400
> >> From: Ken Brown<kbrown@cornell.edu>
> >> CC: "9273@debbugs.gnu.org"<9273@debbugs.gnu.org>
> >>
> >> The problem was that realloc got called on memory that had been
> >> allocated prior to dumping, and the malloc information that was used
> >> then had disappeared.
> >
> > Can you show the code which called realloc on that memory?  I'm
> > surprised that Emacs does that, but perhaps I'm missing something.
> 
> Here's the code that I stumbled across (as a result of a SEGV).  I 
> haven't checked to see if there are other examples.  From terminal.c:
> 
> /* Deletes the bootstrap terminal device.
>     Called through delete_terminal_hook. */
> 
> static void
> delete_initial_terminal (struct terminal *terminal)
> {
>    if (terminal != initial_terminal)
>      abort ();
> 
>    delete_terminal (terminal);
>    initial_terminal = NULL;
> }

delete_terminal doesn't call realloc, it just calls xfree.

Do the problems with the Cygwin build go away if the call to
delete_terminal is commented out?  That is, does the infloop still
happen or not?

bug#9273: 23.3; malloc initialization should (sometimes) happen at runtime

[Ken Brown]

On 8/12/2011 7:33 AM, Eli Zaretskii wrote:
>> Date: Fri, 12 Aug 2011 06:10:35 -0400
>> From: Ken Brown<kbrown@cornell.edu>
>> CC: "9273@debbugs.gnu.org"<9273@debbugs.gnu.org>
>>
>> On 8/12/2011 2:54 AM, Eli Zaretskii wrote:
>>>> Date: Thu, 11 Aug 2011 17:45:41 -0400
>>>> From: Ken Brown<kbrown@cornell.edu>
>>>> CC: "9273@debbugs.gnu.org"<9273@debbugs.gnu.org>
>>>>
>>>> The problem was that realloc got called on memory that had been
>>>> allocated prior to dumping, and the malloc information that was used
>>>> then had disappeared.
>>>
>>> Can you show the code which called realloc on that memory?  I'm
>>> surprised that Emacs does that, but perhaps I'm missing something.
>>
>> Here's the code that I stumbled across (as a result of a SEGV).  I
>> haven't checked to see if there are other examples.  From terminal.c:
>>
>> /* Deletes the bootstrap terminal device.
>>      Called through delete_terminal_hook. */
>>
>> static void
>> delete_initial_terminal (struct terminal *terminal)
>> {
>>     if (terminal != initial_terminal)
>>       abort ();
>>
>>     delete_terminal (terminal);
>>     initial_terminal = NULL;
>> }
>
> delete_terminal doesn't call realloc, it just calls xfree.

Maybe I mis-remembered where the call to realloc is.  I'll reproduce it 
later and let you know.  (I don't have time at the moment.)  But I 
assure you that I did a backtrace showing that realloc was called on 
something related to terminals.

> Do the problems with the Cygwin build go away if the call to
> delete_terminal is commented out?

No.  At the very least, I have to force reinitialization of malloc. 
Otherwise the BLOCK macro yields wrong results that lead to infinite 
looping or crashing.  After reinitialization, I have to be able to 
handle calls to free() on memory allocated prior to dumping.  Probably 
it's OK to just ignore such calls.  If I can also take care of calls to 
realloc too, everything will be OK.

Ken

bug#9273: 23.3; malloc initialization should (sometimes) happen at runtime

[Ken Brown]

On 8/12/2011 8:18 AM, Ken Brown wrote:
> On 8/12/2011 7:33 AM, Eli Zaretskii wrote:
>>> Date: Fri, 12 Aug 2011 06:10:35 -0400
>>> From: Ken Brown<kbrown@cornell.edu>
>>> CC: "9273@debbugs.gnu.org"<9273@debbugs.gnu.org>
>>>
>>> On 8/12/2011 2:54 AM, Eli Zaretskii wrote:
>>>>> Date: Thu, 11 Aug 2011 17:45:41 -0400
>>>>> From: Ken Brown<kbrown@cornell.edu>
>>>>> CC: "9273@debbugs.gnu.org"<9273@debbugs.gnu.org>
>>>>>
>>>>> The problem was that realloc got called on memory that had been
>>>>> allocated prior to dumping, and the malloc information that was used
>>>>> then had disappeared.
>>>>
>>>> Can you show the code which called realloc on that memory?  I'm
>>>> surprised that Emacs does that, but perhaps I'm missing something.
>>>
>>> Here's the code that I stumbled across (as a result of a SEGV).  I
>>> haven't checked to see if there are other examples.  From terminal.c:
>>>
>>> /* Deletes the bootstrap terminal device.
>>>       Called through delete_terminal_hook. */
>>>
>>> static void
>>> delete_initial_terminal (struct terminal *terminal)
>>> {
>>>      if (terminal != initial_terminal)
>>>        abort ();
>>>
>>>      delete_terminal (terminal);
>>>      initial_terminal = NULL;
>>> }
>>
>> delete_terminal doesn't call realloc, it just calls xfree.
> 
> Maybe I mis-remembered where the call to realloc is.  I'll reproduce it
> later and let you know.  (I don't have time at the moment.)  But I
> assure you that I did a backtrace showing that realloc was called on
> something related to terminals.
> 
>> Do the problems with the Cygwin build go away if the call to
>> delete_terminal is commented out?
> 
> No.  At the very least, I have to force reinitialization of malloc.
> Otherwise the BLOCK macro yields wrong results that lead to infinite
> looping or crashing.  After reinitialization, I have to be able to
> handle calls to free() on memory allocated prior to dumping.  Probably
> it's OK to just ignore such calls.  If I can also take care of calls to
> realloc too, everything will be OK.

OK, here's a backtrace showing realloc being called on memory in the static heap (at 0x897040).  This is after applying the patch appended at the end of this message.  (I think it's self-explanatory, but I'll be glad to explain further.)

(gdb) r -Q
Starting program: /home/kbrown/src/emacs/test/src/emacs.exe -Q
[New Thread 4756.0x1144]
warning: cYgFFFFFFFF 611857C0
[New Thread 4756.0xd80]
warning: cYgstd 28ccf5 d 3

Program received signal SIGSEGV, Segmentation fault.
0x006368f5 in _realloc_internal_nolock (ptr=0x897040, size=28)
    at gmalloc.c:1394
1394      type = _heapinfo[block].busy.type;
(gdb) p block
$1 = 4294838425
(gdb) bt
#0  0x006368f5 in _realloc_internal_nolock (ptr=0x897040, size=28)
    at gmalloc.c:1394
#1  0x00636bd7 in _realloc_internal (ptr=0x897040, size=28) at gmalloc.c:1499
#2  0x00636c42 in realloc (ptr=0x897040, size=28) at gmalloc.c:1516
#3  0x00596856 in xrealloc (block=0x897040, size=28) at alloc.c:711
#4  0x00589648 in regex_compile (pattern=0xa7ec60 "site-lisp", size=9, 
    syntax=3408388, bufp=0x846258) at regex.c:3684
#5  0x0059556d in re_compile_pattern (pattern=0xa7ec60 "site-lisp", length=9, 
    bufp=0x846258) at regex.c:6361
#6  0x005768d0 in compile_pattern_1 (cp=0x846248, pattern=9810241, 
    translate=8930309, posix=0) at search.c:150
#7  0x00576b32 in compile_pattern (pattern=9810241, regp=0x8475d8, 
    translate=8930309, posix=0, multibyte=0) at search.c:245
#8  0x005771b8 in string_match_1 (regexp=9810241, string=9810337, 
    start=8968218, posix=0) at search.c:401
#9  0x005773ab in Fstring_match (regexp=9810241, string=9810337, start=8968218)
    at search.c:451
#10 0x005e4f91 in init_lread () at lread.c:4111
#11 0x0052866c in main (argc=2, argv=0x2001cc00) at emacs.c:1467

(gdb) p _heapbase
$3 = 0x20000000 ""
(gdb) p block
$1 = 4294838425

The SEGV comes from the ridiculous value of block, which was calculated by the BLOCK macro.

=== modified file 'src/gmalloc.c'
--- src/gmalloc.c       2011-08-04 17:04:39 +0000
+++ src/gmalloc.c       2011-08-12 19:47:21 +0000
@@ -584,6 +584,12 @@
   mcheck (NULL);
 #endif

+#ifdef CYGWIN
+  if (bss_sbrk_did_unexec)
+    /* we're reinitializing the dumped emacs. */
+    memset (_fraghead, 0, BLOCKLOG * sizeof (struct list));
+#endif
+
   if (__malloc_initialize_hook)
     (*__malloc_initialize_hook) ();

@@ -1054,6 +1060,12 @@
   if (ptr == NULL)
     return;

+#ifdef CYGWIN
+  if (ptr < _heapbase)
+    /* we're being asked to free something in the static heap */
+    return;
+#endif
+
   PROTECT_MALLOC_STATE (0);

   LOCK_ALIGNED_BLOCKS ();

=== modified file 'src/unexcw.c'
--- src/unexcw.c        2011-03-17 20:18:59 +0000
+++ src/unexcw.c        2011-08-12 15:37:47 +0000
@@ -33,6 +33,8 @@

 extern int bss_sbrk_did_unexec;

+extern int __malloc_initialized;
+
 /* emacs symbols that indicate where bss and data end for emacs internals */
 extern char my_endbss[];
 extern char my_edata[];
@@ -210,9 +212,12 @@
            lseek (fd, (long) (exe_header->section_header[i].s_scnptr),
                   SEEK_SET);
          assert (ret != -1);
+         /* force the dumped emacs to reinitialize malloc */
+         __malloc_initialized = 0;
          ret =
            write (fd, (char *) start_address,
                   my_endbss - (char *) start_address);
+         __malloc_initialized = 1;
          assert (ret == (my_endbss - (char *) start_address));
          if (debug_unexcw)
            printf ("         .bss, mem start 0x%08x mem length %d\n",

bug#9273: 23.3; malloc initialization should (sometimes) happen at runtime

[Eli Zaretskii]

> Date: Fri, 12 Aug 2011 16:24:20 -0400
> From: Ken Brown <kbrown@cornell.edu>
> CC: "9273@debbugs.gnu.org" <9273@debbugs.gnu.org>
> 
> Program received signal SIGSEGV, Segmentation fault.
> 0x006368f5 in _realloc_internal_nolock (ptr=0x897040, size=28)
>     at gmalloc.c:1394
> 1394      type = _heapinfo[block].busy.type;
> (gdb) p block
> $1 = 4294838425

I'm confused: since you patched unexecw.c to set __malloc_initialized
to zero, the dumped Emacs should have called malloc_initialize_1,
which should have allocated a new copy of _heapinfo, that was supposed
to be consistent with the current heap.  Why isn't that working? why
`block' still gets a value that is relative to the "old" _heapinfo?

bug#9273: 23.3; malloc initialization should (sometimes) happen at runtime

[Ken Brown]

On 8/13/2011 4:05 AM, Eli Zaretskii wrote:
>> Date: Fri, 12 Aug 2011 16:24:20 -0400
>> From: Ken Brown<kbrown@cornell.edu>
>> CC: "9273@debbugs.gnu.org"<9273@debbugs.gnu.org>
>>
>> Program received signal SIGSEGV, Segmentation fault.
>> 0x006368f5 in _realloc_internal_nolock (ptr=0x897040, size=28)
>>      at gmalloc.c:1394
>> 1394      type = _heapinfo[block].busy.type;
>> (gdb) p block
>> $1 = 4294838425
>
> I'm confused: since you patched unexecw.c to set __malloc_initialized
> to zero, the dumped Emacs should have called malloc_initialize_1,
> which should have allocated a new copy of _heapinfo, that was supposed
> to be consistent with the current heap.  Why isn't that working? why
> `block' still gets a value that is relative to the "old" _heapinfo?

_heapinfo is indeed consistent with the current heap.  But the pointer 
that was passed to realloc points into the old heap.  So applying BLOCK 
to that pointer yields an absurd result.  I can easily catch such cases 
by testing for ptr < _heapbase, as in my patch to _free_internal_nolock, 
but I have to figure out the best way to handle them once I've caught 
them.  I have work in progress that tries to keep track of both heaps, 
but I haven't got it working yet.

An alternative would be to have realloc return NULL (or some other 
special value) in these cases, but then I would have to find all 
possible callers of realloc (with pointers to the old heap) and make 
sure they know how to deal with that return value.  I'm guessing my 
first approach is safer and easier to implement.

Ken

bug#9273: 23.3; malloc initialization should (sometimes) happen at runtime

[Eli Zaretskii]

> Date: Sat, 13 Aug 2011 09:48:52 -0400
> From: Ken Brown <kbrown@cornell.edu>
> CC: "9273@debbugs.gnu.org" <9273@debbugs.gnu.org>
> 
> _heapinfo is indeed consistent with the current heap.  But the pointer 
> that was passed to realloc points into the old heap.  So applying BLOCK 
> to that pointer yields an absurd result.  I can easily catch such cases 
> by testing for ptr < _heapbase, as in my patch to _free_internal_nolock, 
> but I have to figure out the best way to handle them once I've caught 
> them.

malloc a buffer, then copy the contents of the old one to the new
one.  You will have to know the size of the old block, which means you
will have to access the old copy of _heapinfo.

bug#9273: 23.3; malloc initialization should (sometimes) happen at runtime

[Ken Brown]

On 8/13/2011 10:41 AM, Eli Zaretskii wrote:
>> Date: Sat, 13 Aug 2011 09:48:52 -0400
>> From: Ken Brown<kbrown@cornell.edu>
>> CC: "9273@debbugs.gnu.org"<9273@debbugs.gnu.org>
>>
>> _heapinfo is indeed consistent with the current heap.  But the pointer
>> that was passed to realloc points into the old heap.  So applying BLOCK
>> to that pointer yields an absurd result.  I can easily catch such cases
>> by testing for ptr<  _heapbase, as in my patch to _free_internal_nolock,
>> but I have to figure out the best way to handle them once I've caught
>> them.
>
> malloc a buffer, then copy the contents of the old one to the new
> one.  You will have to know the size of the old block, which means you
> will have to access the old copy of _heapinfo.

Thanks!  That's much easier than what I was trying to do.

Ken

bug#9273: 23.3; malloc initialization should (sometimes) happen at runtime

[Stefan Monnier]

>>> _heapinfo is indeed consistent with the current heap.  But the pointer
>>> that was passed to realloc points into the old heap.  So applying BLOCK
>>> to that pointer yields an absurd result.  I can easily catch such cases
>>> by testing for ptr<  _heapbase, as in my patch to _free_internal_nolock,
>>> but I have to figure out the best way to handle them once I've caught
>>> them.
>> malloc a buffer, then copy the contents of the old one to the new
>> one.  You will have to know the size of the old block, which means you
>> will have to access the old copy of _heapinfo.
> Thanks!  That's much easier than what I was trying to do.

Wouldn't it be easier to try and convince malloc to keep using the old
_heapinfo?


        Stefan

bug#9273: 23.3; malloc initialization should (sometimes) happen at runtime

[Ken Brown]

On 8/13/2011 11:07 AM, Stefan Monnier wrote:
>>>> _heapinfo is indeed consistent with the current heap.  But the pointer
>>>> that was passed to realloc points into the old heap.  So applying BLOCK
>>>> to that pointer yields an absurd result.  I can easily catch such cases
>>>> by testing for ptr<   _heapbase, as in my patch to _free_internal_nolock,
>>>> but I have to figure out the best way to handle them once I've caught
>>>> them.
>>> malloc a buffer, then copy the contents of the old one to the new
>>> one.  You will have to know the size of the old block, which means you
>>> will have to access the old copy of _heapinfo.
>> Thanks!  That's much easier than what I was trying to do.
>
> Wouldn't it be easier to try and convince malloc to keep using the old
> _heapinfo?

Probably.  That would keep me from having to get involved in the details 
of how _heapinfo works, and it would probably be less error prone.  So I 
would do something like the following:

1. Temporarily restore the pre-dump malloc state.
2. Call xrealloc, getting storage of the new size in the old heap.
3. Copy the contents into a temporary buffer, and then free the recently 
allocated storage in the old heap.
4. Restore the malloc state, call malloc, and then copy the contents of 
the temporary buffer into the just-acquired storage in the new heap.

Is that what you had in mind?

Ken

bug#9273: 23.3; malloc initialization should (sometimes) happen at runtime

[Stefan Monnier]

> Probably.  That would keep me from having to get involved in the details of
> how _heapinfo works, and it would probably be less error prone.  So I would
> do something like the following:

> 1. Temporarily restore the pre-dump malloc state.
> 2. Call xrealloc, getting storage of the new size in the old heap.
> 3. Copy the contents into a temporary buffer, and then free the recently
> allocated storage in the old heap.
> 4. Restore the malloc state, call malloc, and then copy the contents of the
> temporary buffer into the just-acquired storage in the new heap.

> Is that what you had in mind?

No, I meant, during initialization of Emacs, make sure we get the old
_heapinfo state, so that malloc/realloc/... just works.


        Stefan

bug#9273: 23.3; malloc initialization should (sometimes) happen at runtime

[Ken Brown]

On 8/13/2011 3:19 PM, Stefan Monnier wrote:
>> Probably.  That would keep me from having to get involved in the details of
>> how _heapinfo works, and it would probably be less error prone.  So I would
>> do something like the following:
> 
>> 1. Temporarily restore the pre-dump malloc state.
>> 2. Call xrealloc, getting storage of the new size in the old heap.
>> 3. Copy the contents into a temporary buffer, and then free the recently
>> allocated storage in the old heap.
>> 4. Restore the malloc state, call malloc, and then copy the contents of the
>> temporary buffer into the just-acquired storage in the new heap.
> 
>> Is that what you had in mind?
> 
> No, I meant, during initialization of Emacs, make sure we get the old
> _heapinfo state, so that malloc/realloc/... just works.

No, that wouldn't be good because the old _heapinfo state uses the static heap, which is very small.  It's big enough for temacs, but not for a usable emacs.  In any case, Eli's suggestion turned out to be very easy to implement.  My revised patch is appended below, and it seems to solve all the problems I encountered.  I still want to test it further, but I think it's OK.  Eli, thanks for your help.

Ken

=== modified file 'src/gmalloc.c'
--- src/gmalloc.c       2011-08-04 17:04:39 +0000
+++ src/gmalloc.c       2011-08-13 22:32:45 +0000
@@ -351,11 +351,19 @@
 #endif
 #include <errno.h>

-/* How to really get more memory.  */
-#if defined(CYGWIN)
+/* On Cygwin there are two heaps.  temacs uses the static heap
+   (defined in sheap.c and managed with bss_sbrk), and the dumped
+   emacs uses the Cygwin heap (managed with sbrk).  When emacs starts
+   on Cygwin, it reinitializes malloc, and we save the old info for
+   use by free and realloc if they're called with a pointer into the
+   static heap. */
+#ifdef CYGWIN
 extern __ptr_t bss_sbrk PP ((ptrdiff_t __size));
 extern int bss_sbrk_did_unexec;
+char *bss_sbrk_heapbase;
+malloc_info *bss_sbrk_heapinfo;
 #endif
+
 __ptr_t (*__morecore) PP ((__malloc_ptrdiff_t __size)) = __default_morecore;

 /* Debugging hook for `malloc'.  */
@@ -584,6 +592,16 @@
   mcheck (NULL);
 #endif

+#ifdef CYGWIN
+  if (bss_sbrk_did_unexec)
+    /* we're reinitializing the dumped emacs */
+    {
+      bss_sbrk_heapbase = _heapbase;
+      bss_sbrk_heapinfo = _heapinfo;
+      memset (_fraghead, 0, BLOCKLOG * sizeof (struct list));
+    }
+#endif
+
   if (__malloc_initialize_hook)
     (*__malloc_initialize_hook) ();

@@ -1054,6 +1072,12 @@
   if (ptr == NULL)
     return;

+#ifdef CYGWIN
+  if (ptr < _heapbase)
+    /* We're being asked to free something in the static heap. */
+    return;
+#endif
+
   PROTECT_MALLOC_STATE (0);

   LOCK_ALIGNED_BLOCKS ();
@@ -1346,9 +1370,33 @@
 #include <malloc.h>
 #endif

-
 #define min(A, B) ((A) < (B) ? (A) : (B))

+/* On Cygwin the dumped emacs may try to realloc storage allocated in
+   the static heap.  We just malloc space in the new heap and copy the
+   data.  */
+#ifdef CYGWIN
+__ptr_t
+special_realloc (ptr, size)
+     __ptr_t ptr;
+     __malloc_size_t size;
+{
+  __ptr_t result;
+  int type;
+  __malloc_size_t block, oldsize;
+
+  block = ((char *) ptr - bss_sbrk_heapbase) / BLOCKSIZE + 1;
+  type = bss_sbrk_heapinfo[block].busy.type;
+  oldsize =
+    type == 0 ? bss_sbrk_heapinfo[block].busy.info.size * BLOCKSIZE
+    : (__malloc_size_t) 1 << type;
+  result = _malloc_internal_nolock (size);
+  if (result != NULL)
+    memcpy (result, ptr, min (oldsize, size));
+  return result;
+}
+#endif
+
 /* Debugging hook for realloc.  */
 __ptr_t (*__realloc_hook) PP ((__ptr_t __ptr, __malloc_size_t __size));

@@ -1375,6 +1423,12 @@
   else if (ptr == NULL)
     return _malloc_internal_nolock (size);

+#ifdef CYGWIN
+  if (ptr < _heapbase)
+    /* ptr points into the static heap */
+    return special_realloc (ptr, size);
+#endif
+
   block = BLOCK (ptr);

   PROTECT_MALLOC_STATE (0);

=== modified file 'src/unexcw.c'
--- src/unexcw.c        2011-03-17 20:18:59 +0000
+++ src/unexcw.c        2011-08-12 20:10:03 +0000
@@ -33,6 +33,8 @@

 extern int bss_sbrk_did_unexec;

+extern int __malloc_initialized;
+
 /* emacs symbols that indicate where bss and data end for emacs internals */
 extern char my_endbss[];
 extern char my_edata[];
@@ -210,9 +212,12 @@
            lseek (fd, (long) (exe_header->section_header[i].s_scnptr),
                   SEEK_SET);
          assert (ret != -1);
+         /* force the dumped emacs to reinitialize malloc */
+         __malloc_initialized = 0;
          ret =
            write (fd, (char *) start_address,
                   my_endbss - (char *) start_address);
+         __malloc_initialized = 1;
          assert (ret == (my_endbss - (char *) start_address));
          if (debug_unexcw)
            printf ("         .bss, mem start 0x%08x mem length %d\n",

bug#9273: 23.3; malloc initialization should (sometimes) happen at runtime

[Ken Brown]

I've committed the changes and am closing the bug.

Ken

bug#9273: 23.3; malloc initialization should (sometimes) happen at runtime

[grischka]

> No. At the very least, I have to force reinitialization of malloc. 
> Otherwise the BLOCK macro yields wrong results that lead to infinite 
> looping or crashing. 

I don't think the results from the BLOCK macro are wrong.  I think
your analysis of the problem is wrong. ;)

--- grischka