From 8afaa1321f8088bfb877fe4b6676e8517adb0bb7 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Sat, 21 Nov 2015 10:38:19 -0800 Subject: [PATCH] Add a few safety checks when ENABLE_CHECKING This was motivated by the recent addition of module code, which added some ENABLE_CHECKING-enabled checks that are useful elsewhere too. * src/alloc.c (compact_font_cache_entry): * src/fns.c (sweep_weak_table): * src/lread.c (oblookup): Use gc_asize rather than doing it by hand. * src/emacs-module.c (module_make_global_ref) (module_free_global_ref, module_vec_size): Omit assertions that lisp.h now checks. * src/lisp.h (XFASTINT, ASIZE): In functional implementations, check that the result is nonnegative. Use eassume, as this info can help a bit when optimizing production code. (XSYMBOL) [!USE_LSB_TAG]: Assert that argument is a symbol, to be consistent with the USE_LSB_TAG case. (gc_asize): New function, when ASIZE is needed in the gc. (gc_aset): Use it. (HASH_TABLE_P): Move definition up, so that it can be used ... (XHASH_TABLE): ... here, to assert that the arg is a hash table. --- src/alloc.c | 2 +- src/emacs-module.c | 12 ++---------- src/fns.c | 9 +++------ src/lisp.h | 33 +++++++++++++++++++++++---------- src/lread.c | 4 +--- 5 files changed, 30 insertions(+), 30 deletions(-) diff --git a/src/alloc.c b/src/alloc.c index ad3f5235480..99f5cdcdea2 100644 --- a/src/alloc.c +++ b/src/alloc.c @@ -5339,7 +5339,7 @@ compact_font_cache_entry (Lisp_Object entry) && !VECTOR_MARKED_P (XFONT_SPEC (XCAR (obj))) && VECTORP (XCDR (obj))) { - ptrdiff_t i, size = ASIZE (XCDR (obj)) & ~ARRAY_MARK_FLAG; + ptrdiff_t i, size = gc_asize (XCDR (obj)); /* If font-spec is not marked, most likely all font-entities are not marked too. But we must be sure that nothing is diff --git a/src/emacs-module.c b/src/emacs-module.c index 011cc7be914..c8a0d89492a 100644 --- a/src/emacs-module.c +++ b/src/emacs-module.c @@ -257,7 +257,6 @@ module_make_global_ref (emacs_env *env, emacs_value ref) check_main_thread (); eassert (module_non_local_exit_check (env) == emacs_funcall_exit_return); MODULE_HANDLE_SIGNALS; - eassert (HASH_TABLE_P (Vmodule_refs_hash)); struct Lisp_Hash_Table *h = XHASH_TABLE (Vmodule_refs_hash); Lisp_Object new_obj = value_to_lisp (ref); EMACS_UINT hashcode; @@ -266,7 +265,6 @@ module_make_global_ref (emacs_env *env, emacs_value ref) if (i >= 0) { Lisp_Object value = HASH_VALUE (h, i); - eassert (NATNUMP (value)); EMACS_INT refcount = XFASTINT (value) + 1; if (refcount > MOST_POSITIVE_FIXNUM) { @@ -293,7 +291,6 @@ module_free_global_ref (emacs_env *env, emacs_value ref) /* FIXME: Wait a minute. Shouldn't this function report an error if the hash lookup fails? */ MODULE_HANDLE_SIGNALS_VOID; - eassert (HASH_TABLE_P (Vmodule_refs_hash)); struct Lisp_Hash_Table *h = XHASH_TABLE (Vmodule_refs_hash); Lisp_Object obj = value_to_lisp (ref); EMACS_UINT hashcode; @@ -302,7 +299,6 @@ module_free_global_ref (emacs_env *env, emacs_value ref) if (i >= 0) { Lisp_Object value = HASH_VALUE (h, i); - eassert (NATNUMP (value)); EMACS_INT refcount = XFASTINT (value) - 1; if (refcount > 0) { @@ -310,10 +306,7 @@ module_free_global_ref (emacs_env *env, emacs_value ref) set_hash_value_slot (h, i, value); } else - { - eassert (refcount == 0); - hash_remove_from_table (h, value); - } + hash_remove_from_table (h, value); } } @@ -670,7 +663,6 @@ module_vec_size (emacs_env *env, emacs_value vec) module_wrong_type (env, Qvectorp, lvec); return 0; } - eassert (ASIZE (lvec) >= 0); return ASIZE (lvec); } @@ -894,7 +886,7 @@ finalize_storage (struct emacs_value_storage *storage) } /* Allocate a new value from STORAGE and stores OBJ in it. Return - NULL if allocations fails and use ENV for non local exit reporting. */ + NULL if allocation fails and use ENV for non local exit reporting. */ static emacs_value allocate_emacs_value (emacs_env *env, struct emacs_value_storage *storage, Lisp_Object obj) diff --git a/src/fns.c b/src/fns.c index 824d96980a0..4c13290158a 100644 --- a/src/fns.c +++ b/src/fns.c @@ -4079,13 +4079,10 @@ hash_clear (struct Lisp_Hash_Table *h) static bool sweep_weak_table (struct Lisp_Hash_Table *h, bool remove_entries_p) { - ptrdiff_t bucket, n; - bool marked; - - n = ASIZE (h->index) & ~ARRAY_MARK_FLAG; - marked = 0; + ptrdiff_t n = gc_asize (h->index); + bool marked = false; - for (bucket = 0; bucket < n; ++bucket) + for (ptrdiff_t bucket = 0; bucket < n; ++bucket) { Lisp_Object idx, next, prev; diff --git a/src/lisp.h b/src/lisp.h index 71dca7201d0..9af13a85557 100644 --- a/src/lisp.h +++ b/src/lisp.h @@ -847,7 +847,9 @@ INLINE EMACS_INT INLINE EMACS_INT (XFASTINT) (Lisp_Object a) { - return lisp_h_XFASTINT (a); + EMACS_INT n = lisp_h_XFASTINT (a); + eassume (0 <= n); + return n; } INLINE struct Lisp_Symbol * @@ -915,7 +917,7 @@ XFASTINT (Lisp_Object a) { EMACS_INT int0 = Lisp_Int0; EMACS_INT n = USE_LSB_TAG ? XINT (a) : XLI (a) - (int0 << VALBITS); - eassert (0 <= n); + eassume (0 <= n); return n; } @@ -923,6 +925,7 @@ XFASTINT (Lisp_Object a) INLINE struct Lisp_Symbol * XSYMBOL (Lisp_Object a) { + eassert (SYMBOLP (a)); uintptr_t i = (uintptr_t) XUNTAG (a, Lisp_Symbol); void *p = (char *) lispsym + i; return p; @@ -1536,7 +1539,16 @@ aref_addr (Lisp_Object array, ptrdiff_t idx) INLINE ptrdiff_t ASIZE (Lisp_Object array) { - return XVECTOR (array)->header.size; + ptrdiff_t size = XVECTOR (array)->header.size; + eassume (0 <= size); + return size; +} + +INLINE ptrdiff_t +gc_asize (Lisp_Object array) +{ + /* Like ASIZE, but also can be used in the garbage collector. */ + return XVECTOR (array)->header.size & ~ARRAY_MARK_FLAG; } INLINE void @@ -1551,7 +1563,7 @@ gc_aset (Lisp_Object array, ptrdiff_t idx, Lisp_Object val) { /* Like ASET, but also can be used in the garbage collector: sweep_weak_table calls set_hash_key etc. while the table is marked. */ - eassert (0 <= idx && idx < (ASIZE (array) & ~ARRAY_MARK_FLAG)); + eassert (0 <= idx && idx < gc_asize (array)); XVECTOR (array)->contents[idx] = val; } @@ -1933,21 +1945,22 @@ struct Lisp_Hash_Table }; +INLINE bool +HASH_TABLE_P (Lisp_Object a) +{ + return PSEUDOVECTORP (a, PVEC_HASH_TABLE); +} + INLINE struct Lisp_Hash_Table * XHASH_TABLE (Lisp_Object a) { + eassert (HASH_TABLE_P (a)); return XUNTAG (a, Lisp_Vectorlike); } #define XSET_HASH_TABLE(VAR, PTR) \ (XSETPSEUDOVECTOR (VAR, PTR, PVEC_HASH_TABLE)) -INLINE bool -HASH_TABLE_P (Lisp_Object a) -{ - return PSEUDOVECTORP (a, PVEC_HASH_TABLE); -} - /* Value is the key part of entry IDX in hash table H. */ INLINE Lisp_Object HASH_KEY (struct Lisp_Hash_Table *h, ptrdiff_t idx) diff --git a/src/lread.c b/src/lread.c index a62a42ad1a9..7f0f1d1f6a8 100644 --- a/src/lread.c +++ b/src/lread.c @@ -3941,10 +3941,8 @@ oblookup (Lisp_Object obarray, register const char *ptr, ptrdiff_t size, ptrdiff Lisp_Object bucket, tem; obarray = check_obarray (obarray); - obsize = ASIZE (obarray); - /* This is sometimes needed in the middle of GC. */ - obsize &= ~ARRAY_MARK_FLAG; + obsize = gc_asize (obarray); hash = hash_string (ptr, size_byte) % obsize; bucket = AREF (obarray, hash); oblookup_last_bucket_number = hash; -- 2.39.2