From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.bugs Subject: bug#22526: 25.0.90; Crash starting gnus Date: Sun, 14 Feb 2016 00:11:58 +0200 Message-ID: <83r3ggz2dt.fsf@gnu.org> References: <56AFD88B.5040904@gmail.com> <87pow9cc0c.fsf@gnus.org> <83h9hkse78.fsf@gnu.org> <864mdk44q6.fsf@gmail.com> <83mvrcqli1.fsf@gnu.org> <86twlg2e69.fsf@gmail.com> <8360xv9ems.fsf@gnu.org> <8637sz7xmh.fsf@gmail.com> <83io1v7xcd.fsf@gnu.org> <83fuwx7vkv.fsf@gnu.org> <86fuwxk1l1.fsf@gmail.com> <837fi96mkq.fsf@gnu.org> <83vb5s6gas.fsf@gnu.org> <83d1s05zov.fsf@gnu.org> Reply-To: Eli Zaretskii NNTP-Posting-Host: plane.gmane.org X-Trace: ger.gmane.org 1455401605 9432 80.91.229.3 (13 Feb 2016 22:13:25 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Sat, 13 Feb 2016 22:13:25 +0000 (UTC) Cc: 22526@debbugs.gnu.org, andrewjmoreton@gmail.com To: Fabrice Popineau Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Sat Feb 13 23:13:14 2016 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1aUiRE-0002pn-RX for geb-bug-gnu-emacs@m.gmane.org; Sat, 13 Feb 2016 23:13:13 +0100 Original-Received: from localhost ([::1]:45269 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aUiRE-0003YE-30 for geb-bug-gnu-emacs@m.gmane.org; Sat, 13 Feb 2016 17:13:12 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:53115) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aUiR9-0003XC-8F for bug-gnu-emacs@gnu.org; Sat, 13 Feb 2016 17:13:08 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aUiR4-0001U1-8G for bug-gnu-emacs@gnu.org; Sat, 13 Feb 2016 17:13:07 -0500 Original-Received: from debbugs.gnu.org ([208.118.235.43]:47618) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aUiR4-0001Tx-4Q for bug-gnu-emacs@gnu.org; Sat, 13 Feb 2016 17:13:02 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84) (envelope-from ) id 1aUiR4-0006TQ-03 for bug-gnu-emacs@gnu.org; Sat, 13 Feb 2016 17:13:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Eli Zaretskii Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sat, 13 Feb 2016 22:13:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 22526 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: Original-Received: via spool by 22526-submit@debbugs.gnu.org id=B22526.145540152924819 (code B ref 22526); Sat, 13 Feb 2016 22:13:01 +0000 Original-Received: (at 22526) by debbugs.gnu.org; 13 Feb 2016 22:12:09 +0000 Original-Received: from localhost ([127.0.0.1]:38393 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84) (envelope-from ) id 1aUiQC-0006SF-TF for submit@debbugs.gnu.org; Sat, 13 Feb 2016 17:12:09 -0500 Original-Received: from eggs.gnu.org ([208.118.235.92]:33676) by debbugs.gnu.org with esmtp (Exim 4.84) (envelope-from ) id 1aUiQA-0006Rn-Qz for 22526@debbugs.gnu.org; Sat, 13 Feb 2016 17:12:07 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aUiQ0-00015f-Kk for 22526@debbugs.gnu.org; Sat, 13 Feb 2016 17:12:01 -0500 Original-Received: from fencepost.gnu.org ([2001:4830:134:3::e]:33416) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aUiQ0-00015a-H1; Sat, 13 Feb 2016 17:11:56 -0500 Original-Received: from 84.94.185.246.cable.012.net.il ([84.94.185.246]:1919 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_128_CBC_SHA1:128) (Exim 4.82) (envelope-from ) id 1aUiPz-0005v1-SS; Sat, 13 Feb 2016 17:11:56 -0500 In-reply-to: (message from Fabrice Popineau on Sat, 13 Feb 2016 22:35:57 +0100) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 208.118.235.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.bugs:112998 Archived-At: > From: Fabrice Popineau > Date: Sat, 13 Feb 2016 22:35:57 +0100 > Cc: andrewjmoreton@gmail.com, 22526@debbugs.gnu.org > > I think we need the DebPrint() trace of the problem to conclude. I think the patch I propose below will help in that. > About w32heap.c, the very minimum that we need is : > > diff --git a/src/w32heap.c b/src/w32heap.c > index 00da86a..91167cd 100644 > --- a/src/w32heap.c > +++ b/src/w32heap.c > @@ -654,7 +654,7 @@ mmap_alloc (void **var, size_t nbytes) > *var = VirtualAlloc (p, nbytes, MEM_COMMIT, PAGE_READWRITE); > } > > - if (!p) > + if (p == NULL || *var == NULL) > { > if (GetLastError () == ERROR_NOT_ENOUGH_MEMORY) > errno = ENOMEM; > @@ -718,6 +718,7 @@ mmap_realloc (void **var, size_t nbytes) > DebPrint (("realloc enlarge: VirtualAlloc error %ld\n", > GetLastError ())); > errno = ENOMEM; > + return NULL; > } > return *var; > } See below, I think the patch I propose handles that as well. > About mmap_realloc(), I'm not sure a second attempt at reallocating the buffer at a different address has a > better chance to succeed > (but who knows ? Maybe you are right to try, but I would avoid the jump between the branches of the > conditional). > > Anyway, there may be some other interference : > > /* If there is enough room in the current reserved area, then > commit more pages as needed. */ > if (m2.State == MEM_RESERVE > && nbytes <= memInfo.RegionSize + m2.RegionSize) > { > > If the address sent to mmap_realloc() has been messed up by another part of the code, we won't know it, > VirtualQuery will return > a wrong value and we will keep taking wrong decisions. For example, if m2.State is not MEM_RESERVE, > what does that mean? It means the region after the one we are trying to enlarge was not reserved by us, and we should call mmap_alloc (which we do). No? What I'm worried about is something else: the code is written under the assumption that *var is the base address of the allocation, which is why we use *var + memInfo.RegionSize to get to the next region. But if *var is not the base address, this is wrong, and we should use memInfo.BaseAddress instead, I think. WDYT? > Every block that we try to reallocate should have been allocated first, so reserved first. Are there other cases > that could happen? > > The error codes from VirtualAlloc() here are crucial. The error is ERROR_INVALID_PARAMETER (87), as Andy just reported. > Admittedly, if there were problems of this nature, they would probably have been observed on other platforms. I agree that it's strange we only see this now on a single machine. But maybe Andy does have memory content he doesn't know about. Anyway, here's the patch I propose. Andy, please apply this and then run Emacs under GDB again, so that the error messages will be seen. The patch includes the previous one. diff --git a/src/w32heap.c b/src/w32heap.c index 00da86a..a05c7f2 100644 --- a/src/w32heap.c +++ b/src/w32heap.c @@ -652,15 +652,19 @@ mmap_alloc (void **var, size_t nbytes) { /* Now, commit pages for NBYTES. */ *var = VirtualAlloc (p, nbytes, MEM_COMMIT, PAGE_READWRITE); + if (*var == NULL) + p = *var; } if (!p) { - if (GetLastError () == ERROR_NOT_ENOUGH_MEMORY) + DWORD e = GetLastError (); + + if (e == ERROR_NOT_ENOUGH_MEMORY) errno = ENOMEM; else { - DebPrint (("mmap_alloc: error %ld\n", GetLastError ())); + DebPrint (("mmap_alloc: error %ld\n", e)); errno = EINVAL; } } @@ -700,6 +704,8 @@ mmap_realloc (void **var, size_t nbytes) /* We need to enlarge the block. */ if (memInfo.RegionSize < nbytes) { + void *old_ptr; + if (VirtualQuery (*var + memInfo.RegionSize, &m2, sizeof(m2)) == 0) DebPrint (("mmap_realloc: VirtualQuery error = %ld\n", GetLastError ())); @@ -715,9 +721,11 @@ mmap_realloc (void **var, size_t nbytes) MEM_COMMIT, PAGE_READWRITE); if (!p /* && GetLastError() != ERROR_NOT_ENOUGH_MEMORY */) { - DebPrint (("realloc enlarge: VirtualAlloc error %ld\n", + DebPrint (("realloc enlarge: VirtualAlloc (%p + %I64x, %I64x) error %ld\n", + *var, (uint64_t)memInfo.RegionSize, + (uint64_t)(nbytes - memInfo.RegionSize), GetLastError ())); - errno = ENOMEM; + goto enlarge_block; } return *var; } @@ -726,7 +734,8 @@ mmap_realloc (void **var, size_t nbytes) /* Else we must actually enlarge the block by allocating a new one and copying previous contents from the old to the new one. */ - void *old_ptr = *var; + enlarge_block: + old_ptr = *var; if (mmap_alloc (var, nbytes)) {