>From d0145537fa511a44e2a4af01da3947e92f0b8331 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Sat, 15 Aug 2020 10:48:36 -0700 Subject: [PATCH 1/2] Fix GC bugs related to uninitialized vectors Avoid problems if GC occurs while initializing a vector. Problem with Fdelete reported by Pip Cet in: https://lists.gnu.org/r/emacs-devel/2020-08/msg00313.html I looked for similar problems elsewhere and found quite a few. * src/coding.c (make_subsidiaries): * src/composite.c (syms_of_composite): * src/font.c (build_style_table, Ffont_get_glyphs): * src/nsselect.m (clean_local_selection_data): * src/nsxwidget.m (js_to_lisp): * src/syntax.c (init_syntax_once): * src/window.c (Fcurrent_window_configuration): * src/xselect.c (selection_data_to_lisp_data) (clean_local_selection_data): Use make_nil_vector instead of make_uninit_vector. * src/fns.c (Fdelete): * src/xwidget.c (webkit_js_to_lisp): Use allocate_nil_vector instead of allocate_vector. * src/search.c (Fnewline_cache_check): Use make_vector instead of make_uninit_vector. --- src/coding.c | 9 +++------ src/composite.c | 2 +- src/fns.c | 2 +- src/font.c | 6 +++--- src/lisp.h | 7 +++++-- src/nsselect.m | 2 +- src/nsxwidget.m | 4 ++-- src/search.c | 9 ++------- src/syntax.c | 4 ++-- src/window.c | 2 +- src/xselect.c | 6 +++--- src/xwidget.c | 4 ++-- 12 files changed, 26 insertions(+), 31 deletions(-) diff --git a/src/coding.c b/src/coding.c index 1d79c703a3..51bd441de9 100644 --- a/src/coding.c +++ b/src/coding.c @@ -10856,20 +10856,17 @@ DEFUN ("coding-system-priority-list", Fcoding_system_priority_list, return Fnreverse (val); } -static const char *const suffixes[] = { "-unix", "-dos", "-mac" }; - static Lisp_Object make_subsidiaries (Lisp_Object base) { - Lisp_Object subsidiaries; + static char const suffixes[][8] = { "-unix", "-dos", "-mac" }; ptrdiff_t base_name_len = SBYTES (SYMBOL_NAME (base)); USE_SAFE_ALLOCA; char *buf = SAFE_ALLOCA (base_name_len + 6); - int i; memcpy (buf, SDATA (SYMBOL_NAME (base)), base_name_len); - subsidiaries = make_uninit_vector (3); - for (i = 0; i < 3; i++) + Lisp_Object subsidiaries = make_nil_vector (3); + for (int i = 0; i < 3; i++) { strcpy (buf + base_name_len, suffixes[i]); ASET (subsidiaries, i, intern (buf)); diff --git a/src/composite.c b/src/composite.c index ec2b8328f7..396d456f8c 100644 --- a/src/composite.c +++ b/src/composite.c @@ -1939,7 +1939,7 @@ syms_of_composite (void) staticpro (&gstring_hash_table); staticpro (&gstring_work_headers); - gstring_work_headers = make_uninit_vector (8); + gstring_work_headers = make_nil_vector (8); for (i = 0; i < 8; i++) ASET (gstring_work_headers, i, make_nil_vector (i + 2)); staticpro (&gstring_work); diff --git a/src/fns.c b/src/fns.c index 9199178212..ded6f344aa 100644 --- a/src/fns.c +++ b/src/fns.c @@ -1755,7 +1755,7 @@ DEFUN ("delete", Fdelete, Sdelete, 2, 2, 0, if (n != ASIZE (seq)) { - struct Lisp_Vector *p = allocate_vector (n); + struct Lisp_Vector *p = allocate_nil_vector (n); for (i = n = 0; i < ASIZE (seq); ++i) if (NILP (Fequal (AREF (seq, i), elt))) diff --git a/src/font.c b/src/font.c index ab00402b40..ccbd3fc9ce 100644 --- a/src/font.c +++ b/src/font.c @@ -4889,7 +4889,7 @@ DEFUN ("font-get-glyphs", Ffont_get_glyphs, Sfont_get_glyphs, 3, 4, 0, { struct font *font = CHECK_FONT_GET_OBJECT (font_object); ptrdiff_t len; - Lisp_Object *chars, vec; + Lisp_Object *chars; USE_SAFE_ALLOCA; if (NILP (object)) @@ -4957,7 +4957,7 @@ DEFUN ("font-get-glyphs", Ffont_get_glyphs, Sfont_get_glyphs, 3, 4, 0, else wrong_type_argument (Qarrayp, object); - vec = make_uninit_vector (len); + Lisp_Object vec = make_nil_vector (len); for (ptrdiff_t i = 0; i < len; i++) { Lisp_Object g; @@ -5203,7 +5203,7 @@ #define BUILD_STYLE_TABLE(TBL) build_style_table (TBL, ARRAYELTS (TBL)) static Lisp_Object build_style_table (const struct table_entry *entry, int nelement) { - Lisp_Object table = make_uninit_vector (nelement); + Lisp_Object table = make_nil_vector (nelement); for (int i = 0; i < nelement; i++) { int j; diff --git a/src/lisp.h b/src/lisp.h index eaf1c6ce6d..7983339ac5 100644 --- a/src/lisp.h +++ b/src/lisp.h @@ -3916,7 +3916,6 @@ build_string (const char *str) extern Lisp_Object pure_cons (Lisp_Object, Lisp_Object); extern Lisp_Object make_vector (ptrdiff_t, Lisp_Object); -extern struct Lisp_Vector *allocate_vector (ptrdiff_t); extern struct Lisp_Vector *allocate_nil_vector (ptrdiff_t); /* Make an uninitialized vector for SIZE objects. NOTE: you must @@ -3926,7 +3925,11 @@ build_string (const char *str) v = make_uninit_vector (3); ASET (v, 0, obj0); ASET (v, 1, Ffunction_can_gc ()); - ASET (v, 2, obj1); */ + ASET (v, 2, obj1); + + allocate_vector has a similar problem. */ + +extern struct Lisp_Vector *allocate_vector (ptrdiff_t); INLINE Lisp_Object make_uninit_vector (ptrdiff_t size) diff --git a/src/nsselect.m b/src/nsselect.m index 38ac66e9c7..7b1937f5d9 100644 --- a/src/nsselect.m +++ b/src/nsselect.m @@ -114,7 +114,7 @@ Updated by Christian Limpach (chris@nice.ch) if (size == 1) return clean_local_selection_data (AREF (obj, 0)); - copy = make_uninit_vector (size); + copy = make_nil_vector (size); for (i = 0; i < size; i++) ASET (copy, i, clean_local_selection_data (AREF (obj, i))); return copy; diff --git a/src/nsxwidget.m b/src/nsxwidget.m index 370abee395..e81ca7fc0c 100644 --- a/src/nsxwidget.m +++ b/src/nsxwidget.m @@ -388,7 +388,7 @@ - (void)userContentController:(WKUserContentController *)userContentController NSArray *nsarr = (NSArray *) value; EMACS_INT n = nsarr.count; Lisp_Object obj; - struct Lisp_Vector *p = allocate_vector (n); + struct Lisp_Vector *p = allocate_nil_vector (n); for (ptrdiff_t i = 0; i < n; ++i) p->contents[i] = js_to_lisp ([nsarr objectAtIndex:i]); @@ -401,7 +401,7 @@ - (void)userContentController:(WKUserContentController *)userContentController NSArray *keys = nsdict.allKeys; ptrdiff_t n = keys.count; Lisp_Object obj; - struct Lisp_Vector *p = allocate_vector (n); + struct Lisp_Vector *p = allocate_nil_vector (n); for (ptrdiff_t i = 0; i < n; ++i) { diff --git a/src/search.c b/src/search.c index 38c64caf7c..23b31d9281 100644 --- a/src/search.c +++ b/src/search.c @@ -3271,7 +3271,7 @@ DEFUN ("newline-cache-check", Fnewline_cache_check, Snewline_cache_check, TYPE_MAXIMUM (ptrdiff_t), &nl_count_cache, NULL, true); /* Create vector and populate it. */ - cache_newlines = make_uninit_vector (nl_count_cache); + cache_newlines = make_vector (nl_count_cache, make_fixnum (-1)); if (nl_count_cache) { @@ -3285,15 +3285,12 @@ DEFUN ("newline-cache-check", Fnewline_cache_check, Snewline_cache_check, break; ASET (cache_newlines, i, make_fixnum (found - 1)); } - /* Fill the rest of slots with an invalid position. */ - for ( ; i < nl_count_cache; i++) - ASET (cache_newlines, i, make_fixnum (-1)); } /* Now do the same, but without using the cache. */ find_newline1 (BEGV, BEGV_BYTE, ZV, ZV_BYTE, TYPE_MAXIMUM (ptrdiff_t), &nl_count_buf, NULL, true); - buf_newlines = make_uninit_vector (nl_count_buf); + buf_newlines = make_vector (nl_count_buf, make_fixnum (-1)); if (nl_count_buf) { for (from = BEGV, found = from, i = 0; from < ZV; from = found, i++) @@ -3306,8 +3303,6 @@ DEFUN ("newline-cache-check", Fnewline_cache_check, Snewline_cache_check, break; ASET (buf_newlines, i, make_fixnum (found - 1)); } - for ( ; i < nl_count_buf; i++) - ASET (buf_newlines, i, make_fixnum (-1)); } /* Construct the value and return it. */ diff --git a/src/syntax.c b/src/syntax.c index a03202d386..9f77ea5f9b 100644 --- a/src/syntax.c +++ b/src/syntax.c @@ -3617,9 +3617,9 @@ init_syntax_once (void) DEFSYM (Qsyntax_table, "syntax-table"); /* Create objects which can be shared among syntax tables. */ - Vsyntax_code_object = make_uninit_vector (Smax); + Vsyntax_code_object = make_nil_vector (Smax); for (i = 0; i < Smax; i++) - ASET (Vsyntax_code_object, i, Fcons (make_fixnum (i), Qnil)); + ASET (Vsyntax_code_object, i, list1 (make_fixnum (i))); /* Now we are ready to set up this property, so we can create syntax tables. */ diff --git a/src/window.c b/src/window.c index e2dea8b70e..ef58f43a0b 100644 --- a/src/window.c +++ b/src/window.c @@ -7465,7 +7465,7 @@ redirection (see `redirect-frame-focus'). The variable data->minibuf_selected_window = minibuf_level > 0 ? minibuf_selected_window : Qnil; data->root_window = FRAME_ROOT_WINDOW (f); data->focus_frame = FRAME_FOCUS_FRAME (f); - Lisp_Object tem = make_uninit_vector (n_windows); + Lisp_Object tem = make_nil_vector (n_windows); data->saved_windows = tem; for (ptrdiff_t i = 0; i < n_windows; i++) ASET (tem, i, make_nil_vector (VECSIZE (struct saved_window))); diff --git a/src/xselect.c b/src/xselect.c index 48d6215a7b..bf50c598b2 100644 --- a/src/xselect.c +++ b/src/xselect.c @@ -1594,7 +1594,7 @@ selection_data_to_lisp_data (struct x_display_info *dpyinfo, return x_atom_to_symbol (dpyinfo, (Atom) idata[0]); else { - Lisp_Object v = make_uninit_vector (size / sizeof (int)); + Lisp_Object v = make_nil_vector (size / sizeof (int)); for (i = 0; i < size / sizeof (int); i++) ASET (v, i, x_atom_to_symbol (dpyinfo, (Atom) idata[i])); @@ -1653,7 +1653,7 @@ selection_data_to_lisp_data (struct x_display_info *dpyinfo, else { ptrdiff_t i; - Lisp_Object v = make_uninit_vector (size / X_LONG_SIZE); + Lisp_Object v = make_nil_vector (size / X_LONG_SIZE); if (type == XA_INTEGER) { @@ -1860,7 +1860,7 @@ clean_local_selection_data (Lisp_Object obj) Lisp_Object copy; if (size == 1) return clean_local_selection_data (AREF (obj, 0)); - copy = make_uninit_vector (size); + copy = make_nil_vector (size); for (i = 0; i < size; i++) ASET (copy, i, clean_local_selection_data (AREF (obj, i))); return copy; diff --git a/src/xwidget.c b/src/xwidget.c index c61f5bef88..154b3e9c82 100644 --- a/src/xwidget.c +++ b/src/xwidget.c @@ -343,7 +343,7 @@ webkit_js_to_lisp (JSCValue *value) memory_full (SIZE_MAX); ptrdiff_t n = dlen; - struct Lisp_Vector *p = allocate_vector (n); + struct Lisp_Vector *p = allocate_nil_vector (n); for (ptrdiff_t i = 0; i < n; ++i) { @@ -361,7 +361,7 @@ webkit_js_to_lisp (JSCValue *value) Lisp_Object obj; if (PTRDIFF_MAX < n) memory_full (n); - struct Lisp_Vector *p = allocate_vector (n); + struct Lisp_Vector *p = allocate_nil_vector (n); for (ptrdiff_t i = 0; i < n; ++i) { -- 2.17.1