[PATCH21.5] regex_malloc_disallowed fix

PATCH 21.5 JJ> From the backtrace below, it looks like the problem is triggered like JJ> this: JJ> (1) re_match_2_internal sets regex_malloc_disallowed, then calls JJ> (2) offset_to_charxpos, which calls JJ> (3) bytebpos_to_charbpos_func, at which point JJ> (4) a signal arrives, throwing us into JJ> (5) vdb_fault_handler, which mallocs Jerry, thank you for providing detailed diagnosis. Here is my solution to the problem: My approach is to fully avoid allocation in the write barrier's signal handler. Basically this means to make the page_fault_table large enough beforehand. Luckily the upper bound of the page_fault_table's size can be determined prior to each write-barrier cycle. The upper bound corresponds to the number of pages that are watched by the write barrier. These are all the pages that contain BLACK objects. This patch includes minimal modifications to determine the number of processed pages, and the call to resize the page_fault_table. Please test. I don't know if this is the best way to deal with it. I read through the comments in regex.c that talk about the connection of REL_ALLOC and regex_malloc_disallowed. I was relieved to read that even Ben had trouble understanding it all. Is somebody around who understands it and can shed light on the regular expressions engine? Would it be better to "be prepared for malloc()", as the comments in regex.c state? Does anybody know how to "prepare"? Can you do this with RE_MATCH_RELOCATE_MOVEABLE_DATA_POINTERS and/or RE_SEARCH_RELOCATE_MOVEABLE_DATA_POINTERS? How expensive is this? Is it more elegant than avoiding malloc in the signal handler and causing memory overhead? Comments welcome! If nobody objects to the patch as it is right now (and comes up with a better solution), I'll commit it in a couple of days. src/ChangeLog addition: 2006-03-21 Marcus Crestani <crestani(a)xemacs.org&gt; * mc-alloc.c (visit_all_used_page_headers): * mc-alloc.c (finalize_page_for_disksave): * mc-alloc.c (mc_finalize_for_disksave): * mc-alloc.c (sweep_page): * mc-alloc.c (mc_sweep): * mc-alloc.c (protect_heap_page): * mc-alloc.c (protect_heap_pages): * mc-alloc.c (unprotect_heap_page): * mc-alloc.c (unprotect_heap_pages): * mc-alloc.h: Return number of pages processed. * vdb.c (vdb_start_dirty_bits_recording): Adjust size of page_fault_table to its upper bound (= number of pages that contain BLACK objects) in advance, to avoid malloc in the signal handler. xemacs-21.5 source patch: Diff command: cvs -q diff -u Files affected: src/vdb.c src/mc-alloc.h src/mc-alloc.c Index: src/mc-alloc.c =================================================================== RCS file: /pack/xemacscvs/XEmacs/xemacs/src/mc-alloc.c,v retrieving revision 1.8 diff -u -r1.8 mc-alloc.c --- src/mc-alloc.c 27 Feb 2006 16:29:27 -0000 1.8 +++ src/mc-alloc.c 21 Mar 2006 16:39:39 -0000 ＠＠ -430,10 +430,11 ＠＠ /* Visits all pages (page_headers) hooked into the used heap pages list and executes f with the current page header as - argument. Needed for sweep. */ -static void -visit_all_used_page_headers (void (*f) (page_header *ph)) + argument. Needed for sweep. Returns number of processed pages. */ +static EMACS_INT +visit_all_used_page_headers (EMACS_INT (*f) (page_header *ph)) { + EMACS_INT number_of_pages_processed = 0; EMACS_INT i; for (i = 0; i < N_USED_PAGE_LISTS; i++) if (PLH_FIRST (USED_HEAP_PAGES (i))) ＠＠ -442,11 +443,12 ＠＠ while (PH_NEXT (ph)) { page_header *next = PH_NEXT (ph); /* in case f removes the page */ - f (ph); + number_of_pages_processed += f (ph); ph = next; } - f (ph); + number_of_pages_processed += f (ph); } + return number_of_pages_processed; } ＠＠ -1485,8 +1487,9 ＠＠ object's finalizer for disksave. Therefore, you have to define the macro MC_ALLOC_CALL_FINALIZER_FOR_DISKSAVE(ptr). This macro should do nothing else then test if there is a finalizer and call it on - the given argument, which is the heap address of the object. */ -static void + the given argument, which is the heap address of the object. + Returns number of processed pages. */ +static EMACS_INT finalize_page_for_disksave (page_header *ph) { EMACS_INT heap_space = (EMACS_INT) PH_HEAP_SPACE (ph); ＠＠ -1499,21 +1502,24 ＠＠ EMACS_INT ptr = (heap_space + (heap_space_step * mark_bit)); MC_ALLOC_CALL_FINALIZER_FOR_DISKSAVE ((void *) ptr); } + return 1; } -/* Finalizes the heap for disksave. */ -void +/* Finalizes the heap for disksave. Returns number of processed + pages. */ +EMACS_INT mc_finalize_for_disksave (void) { - visit_all_used_page_headers (finalize_page_for_disksave); + return visit_all_used_page_headers (finalize_page_for_disksave); } -/* Sweeps a page: all the non-marked cells are freed. If the page is empty - in the end, it is removed. If some cells are free, it is moved to the - front of its page header list. Full pages stay where they are. */ -static void +/* Sweeps a page: all the non-marked cells are freed. If the page is + empty in the end, it is removed. If some cells are free, it is + moved to the front of its page header list. Full pages stay where + they are. Returns number of processed pages.*/ +static EMACS_INT sweep_page (page_header *ph) { Rawbyte *heap_space = (Rawbyte *) PH_HEAP_SPACE (ph); ＠＠ -1533,7 +1539,7 ＠＠ { zero_mark_bits (ph); PH_BLACK_BIT (ph) = 0; - return; + return 1; } } ＠＠ -1552,14 +1558,16 ＠＠ remove_page_from_used_list (ph); else if (PH_CELLS_USED (ph) < PH_CELLS_ON_PAGE (ph)) move_page_header_to_front (ph); + + return 1; } -/* Sweeps the heap. */ -void +/* Sweeps the heap. Returns number of processed pages. */ +EMACS_INT mc_sweep (void) { - visit_all_used_page_headers (sweep_page); + return visit_all_used_page_headers (sweep_page); } ＠＠ -1860,8 +1868,8 ＠＠ /* Protect the heap page of given page header ph if black objects are - on the page. */ -static void + on the page. Returns number of processed pages. */ +static EMACS_INT protect_heap_page (page_header *ph) { if (PH_BLACK_BIT (ph)) ＠＠ -1870,20 +1878,23 ＠＠ EMACS_INT heap_space_size = PH_N_PAGES (ph) * PAGE_SIZE; vdb_protect ((void *) heap_space, heap_space_size); PH_PROTECTION_BIT (ph) = 1; + return 1; } + return 0; } -/* Protect all heap pages with black objects. */ -void +/* Protect all heap pages with black objects. Returns number of + processed pages.*/ +EMACS_INT protect_heap_pages (void) { - visit_all_used_page_headers (protect_heap_page); + return visit_all_used_page_headers (protect_heap_page); } /* Remove protection (if there) of heap page of given page header - ph. */ -static void + ph. Returns number of processed pages. */ +static EMACS_INT unprotect_heap_page (page_header *ph) { if (PH_PROTECTION_BIT (ph)) ＠＠ -1892,14 +1903,17 ＠＠ EMACS_INT heap_space_size = PH_N_PAGES (ph) * PAGE_SIZE; vdb_unprotect (heap_space, heap_space_size); PH_PROTECTION_BIT (ph) = 0; + return 1; } + return 0; } -/* Remove protection for all heap pages which are protected. */ -void +/* Remove protection for all heap pages which are protected. Returns + number of processed pages. */ +EMACS_INT unprotect_heap_pages (void) { - visit_all_used_page_headers (unprotect_heap_page); + return visit_all_used_page_headers (unprotect_heap_page); } /* Remove protection and mark page dirty. */ Index: src/mc-alloc.h =================================================================== RCS file: /pack/xemacscvs/XEmacs/xemacs/src/mc-alloc.h,v retrieving revision 1.4 diff -u -r1.4 mc-alloc.h --- src/mc-alloc.h 27 Feb 2006 16:29:27 -0000 1.4 +++ src/mc-alloc.h 21 Mar 2006 16:39:39 -0000 ＠＠ -105,11 +105,12 ＠＠ /* The finalizer of every not marked object is called. The macro MC_ALLOC_CALL_FINALIZER has to be defined and call the finalizer of - the object. */ -void mc_finalize (void); + the object. Returns number of processed pages. */ +EMACS_INT mc_finalize (void); -/* All not marked objects of the used heap are freed. */ -void mc_sweep (void); +/* All not marked objects of the used heap are freed. Returns number + of processed pages. */ +EMACS_INT mc_sweep (void); ＠＠ -117,8 +118,9 ＠＠ /* The finalizer for disksave of every object is called to shrink the dump image. The macro MC_ALLOC_CALL_FINALIZER_FOR_DISKSAVE has to - be defined and call the finalizer for disksave of the object. */ -void mc_finalize_for_disksave (void); + be defined and call the finalizer for disksave of the object. + Returns number of processed pages. */ +EMACS_INT mc_finalize_for_disksave (void); ＠＠ -141,12 +143,13 ＠＠ /* Is the fault at ptr on a protected page? */ EMACS_INT fault_on_protected_page (void *ptr); -/* Remove protection (if there) of heap page of given page header - ph. */ -void protect_heap_pages (void); - -/* Remove protection for all heap pages which are protected. */ -void unprotect_heap_pages (void); +/* Remove protection (if there) of heap page of given page header ph. + Returns number of processed pages. */ +EMACS_INT protect_heap_pages (void); + +/* Remove protection for all heap pages which are protected. Returns + number of processed pages. */ +EMACS_INT unprotect_heap_pages (void); /* Remove protection and mark page dirty. */ void unprotect_page_and_mark_dirty (void *ptr); Index: src/vdb.c =================================================================== RCS file: /pack/xemacscvs/XEmacs/xemacs/src/vdb.c,v retrieving revision 1.1 diff -u -r1.1 vdb.c --- src/vdb.c 25 Nov 2005 01:42:08 -0000 1.1 +++ src/vdb.c 21 Mar 2006 16:39:39 -0000 ＠＠ -38,8 +38,9 ＠＠ void vdb_start_dirty_bits_recording (void) { + Elemcount protected_pages = (Elemcount) protect_heap_pages (); page_fault_table = Dynarr_new2 (void_ptr_dynarr, void *); - protect_heap_pages (); + Dynarr_resize (page_fault_table, protected_pages); } /* Remove heap protection. */ -- Marcus