From mboxrd@z Thu Jan 1 00:00:00 1970 Path: main.gmane.org!not-for-mail From: storm@cua.dk (Kim F. Storm) Newsgroups: gmane.emacs.devel Subject: Re: Fix to long-standing crashes in GC Date: 29 May 2004 01:40:15 +0200 Sender: emacs-devel-bounces+emacs-devel=quimby.gnus.org@gnu.org Message-ID: References: <40A3BC23.8060000@math.ku.dk> <200405180013.i4I0Ddl15818@raven.dms.auburn.edu> NNTP-Posting-Host: deer.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: sea.gmane.org 1085808443 32279 80.91.224.253 (29 May 2004 05:27:23 GMT) X-Complaints-To: usenet@sea.gmane.org NNTP-Posting-Date: Sat, 29 May 2004 05:27:23 +0000 (UTC) Cc: Luc Teirlinck , rms@gnu.org, emacs-devel@gnu.org Original-X-From: emacs-devel-bounces+emacs-devel=quimby.gnus.org@gnu.org Sat May 29 07:27:15 2004 Return-path: Original-Received: from quimby.gnus.org ([80.91.224.244]) by deer.gmane.org with esmtp (Exim 3.35 #1 (Debian)) id 1BTwNH-0000Pm-00 for ; Sat, 29 May 2004 07:27:15 +0200 Original-Received: from lists.gnu.org ([199.232.76.165]) by quimby.gnus.org with esmtp (Exim 3.35 #1 (Debian)) id 1BTwNG-0003uB-00 for ; Sat, 29 May 2004 07:27:15 +0200 Original-Received: from [127.0.0.1] (helo=mailman.gnu.org) by lists.gnu.org with esmtp (Exim 4.33) id 1BTwNL-00004s-0K for emacs-devel@quimby.gnus.org; Sat, 29 May 2004 01:27:19 -0400 Original-Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.33) id 1BTwND-0008WJ-TW for emacs-devel@gnu.org; Sat, 29 May 2004 01:27:12 -0400 Original-Received: from exim by lists.gnu.org with spam-scanned (Exim 4.33) id 1BTwND-0008W7-AW for emacs-devel@gnu.org; Sat, 29 May 2004 01:27:11 -0400 Original-Received: from [199.232.76.173] (helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.33) id 1BTwND-0008W4-63 for emacs-devel@gnu.org; Sat, 29 May 2004 01:27:11 -0400 Original-Received: from [199.232.41.8] (helo=mx20.gnu.org) by monty-python.gnu.org with esmtp (TLSv1:DES-CBC3-SHA:168) (Exim 4.34) id 1BTw59-0001jh-Om; Sat, 29 May 2004 01:08:31 -0400 Original-Received: from [195.41.46.237] (helo=pfepc.post.tele.dk) by mx20.gnu.org with esmtp (Exim 4.34) id 1BTr00-0006cn-GX; Fri, 28 May 2004 19:42:52 -0400 Original-Received: from kfs-l.imdomain.dk.cua.dk (0x503e2644.bynxx3.adsl-dhcp.tele.dk [80.62.38.68]) by pfepc.post.tele.dk (Postfix) with SMTP id 9A783262826; Sat, 29 May 2004 01:40:07 +0200 (CEST) Original-To: Stefan Monnier In-Reply-To: Original-Lines: 94 User-Agent: Gnus/5.09 (Gnus v5.9.0) Emacs/21.3.50 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.4 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+emacs-devel=quimby.gnus.org@gnu.org Xref: main.gmane.org gmane.emacs.devel:24133 X-Report-Spam: http://spam.gmane.org/gmane.emacs.devel:24133 Stefan Monnier writes: > > I have now installed some further changes to the GC code to > > fix problems with invalid Lisp_Misc objects. > > > The main problem was that marker blocks could be freed while > > still being pointed to by buffer undo lists, but I made other > > minor changes to make the code more robust. > > I still don't understand how this could happen. In retrospect, the problem was/is caused by the method used to mark the undo-list (the step that your patch removes). The problem with the current undo-list marking is that we still mark the cons cells which points to the markers that we might later free and remove from the undo-list. This meant that we would not free those cons cells which thus ends up pointing to invalid marker objects. Now, when we on next gc calls mark_stack, and thus mark_maybe_object / mark_maybe_pointer, we may accidentally have a pointer address that leads to a (valid) cons, which is checked with mark_maybe_object... that's ok, you see that it points to a valid cons block, and it is not marked Vdead. So you do mark_object on that cons, meaning that you unconditionally does mark_object on the car and cdr. The cdr of that cons is another cons, so you do mark_object (unconditionally) on the car of that cons -- but it happened to be a cons cell which used to be on an undo-list, so its car points to a lisp_free_marker -- or worse into a freed marker block. My changes focused on fixing the effects of the erroneous marking of those cons cells, so part of my fix was to expclicitly clear (Qnil) the car and cdr of the two cons cells that are removed from the undo-list; that was necessary as the removal was done AFTER gc_sweep. Now, looking at your alternative patch, it elegantly eliminates the bogus marking of the cons cells that are removed from the undo-list, so I think your patch will also fix the problem. Your change is indeed cleaner than my fixes, so let's install your patch instead. BTW, I think that clearing the cons cells before strings is still a proper sequence in gc_sweep, as the old sequence (that your patch reinstalls) implies that after cleaning the strings, there are cons cells (unmarked though) that points into freed memory. I.e. I suggest that the following part of the patch is omitted: @@ -5202,6 +5152,16 @@ static void gc_sweep () { + /* Remove or mark entries in weak hash tables. + This must be done before any object is unmarked. */ + sweep_weak_hash_tables (); + + sweep_strings (); +#ifdef GC_CHECK_STRING_BYTES + if (!noninteractive) + check_string_bytes (1); +#endif + /* Put all unmarked conses on free list */ { register struct cons_block *cblk; @@ -5252,16 +5212,6 @@ total_free_conses = num_free; } - /* Remove or mark entries in weak hash tables. - This must be done before any object is unmarked. */ - sweep_weak_hash_tables (); - - sweep_strings (); -#ifdef GC_CHECK_STRING_BYTES - if (!noninteractive) - check_string_bytes (1); -#endif - /* Put all unmarked floats on free list */ { register struct float_block *fblk; -- Kim F. Storm http://www.cua.dk