From 8ecca2f09f6bc387412f258c4fc4e3ddf807b2b3 Mon Sep 17 00:00:00 2001 From: Philipp Stephani Date: Thu, 23 Jul 2020 13:48:43 +0200 Subject: Backport: Fix memory leak for global module objects (Bug#42482). Instead of storing the global values in a global 'emacs_value_storage' object, store them as hash values alongside the reference counts. That way the garbage collector takes care of cleaning them up. * src/emacs-module.c (global_storage): Remove. (struct module_global_reference): New pseudovector type. (XMODULE_GLOBAL_REFERENCE): New helper function. (module_make_global_ref, module_free_global_ref): Use 'module_global_reference' struct for global reference values. (value_to_lisp, module_handle_nonlocal_exit): Adapt to deletion of 'global_storage'. (cherry picked from commit 5c5eb9790898e4ab10bcbbdb6871947ed3018569) --- src/emacs-module.c | 82 +++++++++++++++++++++++++++++++++++------------------- 1 file changed, 54 insertions(+), 28 deletions(-) (limited to 'src') diff --git a/src/emacs-module.c b/src/emacs-module.c index 911b82b8a1a..4269b0ba2ac 100644 --- a/src/emacs-module.c +++ b/src/emacs-module.c @@ -159,11 +159,11 @@ struct emacs_value_frame /* A structure that holds an initial frame (so that the first local values require no dynamic allocation) and keeps track of the current frame. */ -static struct emacs_value_storage +struct emacs_value_storage { struct emacs_value_frame initial; struct emacs_value_frame *current; -} global_storage; +}; /* Private runtime and environment members. */ @@ -351,10 +351,35 @@ module_get_environment (struct emacs_runtime *ert) } /* To make global refs (GC-protected global values) keep a hash that - maps global Lisp objects to reference counts. */ + maps global Lisp objects to 'struct module_global_reference' + objects. We store the 'emacs_value' in the hash table so that it + is automatically garbage-collected (Bug#42482). */ static Lisp_Object Vmodule_refs_hash; +/* Pseudovector type for global references. The pseudovector tag is + PVEC_OTHER since these values are never printed and don't need to + be special-cased for garbage collection. */ + +struct module_global_reference { + /* Pseudovector header, must come first. */ + union vectorlike_header header; + + /* Holds the emacs_value for the object. The Lisp_Object stored + therein must be the same as the hash key. */ + struct emacs_value_tag value; + + /* Reference count, always positive. */ + ptrdiff_t refcount; +}; + +static struct module_global_reference * +XMODULE_GLOBAL_REFERENCE (Lisp_Object o) +{ + eassert (PSEUDOVECTORP (o, PVEC_OTHER)); + return XUNTAG (o, Lisp_Vectorlike, struct module_global_reference); +} + static emacs_value module_make_global_ref (emacs_env *env, emacs_value ref) { @@ -363,21 +388,30 @@ module_make_global_ref (emacs_env *env, emacs_value ref) Lisp_Object new_obj = value_to_lisp (ref), hashcode; ptrdiff_t i = hash_lookup (h, new_obj, &hashcode); + /* Note: This approach requires the garbage collector to never move + objects. */ + if (i >= 0) { Lisp_Object value = HASH_VALUE (h, i); - EMACS_INT refcount = XFIXNAT (value) + 1; - if (MOST_POSITIVE_FIXNUM < refcount) + struct module_global_reference *ref = XMODULE_GLOBAL_REFERENCE (value); + bool overflow = INT_ADD_WRAPV (ref->refcount, 1, &ref->refcount); + if (overflow) overflow_error (); - value = make_fixed_natnum (refcount); - set_hash_value_slot (h, i, value); + return &ref->value; } else { - hash_put (h, new_obj, make_fixed_natnum (1), hashcode); + struct module_global_reference *ref + = ALLOCATE_PLAIN_PSEUDOVECTOR (struct module_global_reference, + PVEC_OTHER); + ref->value.v = new_obj; + ref->refcount = 1; + Lisp_Object value; + XSETPSEUDOVECTOR (value, ref, PVEC_OTHER); + hash_put (h, new_obj, value, hashcode); + return &ref->value; } - - return allocate_emacs_value (env, &global_storage, new_obj); } static void @@ -393,23 +427,16 @@ module_free_global_ref (emacs_env *env, emacs_value ref) if (i >= 0) { - EMACS_INT refcount = XFIXNAT (HASH_VALUE (h, i)) - 1; - if (refcount > 0) - set_hash_value_slot (h, i, make_fixed_natnum (refcount)); - else - { - eassert (refcount == 0); - hash_remove_from_table (h, obj); - } + Lisp_Object value = HASH_VALUE (h, i); + struct module_global_reference *ref = XMODULE_GLOBAL_REFERENCE (value); + eassert (0 < ref->refcount); + if (--ref->refcount == 0) + hash_remove_from_table (h, obj); } - - if (module_assertions) + else if (module_assertions) { - ptrdiff_t count = 0; - if (value_storage_contains_p (&global_storage, ref, &count)) - return; module_abort ("Global value was not found in list of %"pD"d globals", - count); + h->count); } } @@ -1190,8 +1217,10 @@ value_to_lisp (emacs_value v) ++num_environments; } /* Also check global values. */ - if (value_storage_contains_p (&global_storage, v, &num_values)) + struct Lisp_Hash_Table *h = XHASH_TABLE (Vmodule_refs_hash); + if (hash_lookup (h, v->v, NULL) != -1) goto ok; + INT_ADD_WRAPV (num_values, h->count, &num_values); module_abort (("Emacs value not found in %"pD"d values " "of %"pD"d environments"), num_values, num_environments); @@ -1404,10 +1433,7 @@ module_handle_nonlocal_exit (emacs_env *env, enum nonlocal_exit type, void init_module_assertions (bool enable) { - /* If enabling module assertions, use a hidden environment for - storing the globals. This environment is never freed. */ module_assertions = enable; - initialize_storage (&global_storage); } /* Return whether STORAGE contains VALUE. Used to check module -- cgit v1.2.1 From 8c94ca94dc2772e5c651de6cf46bfffc388234d5 Mon Sep 17 00:00:00 2001 From: Philipp Stephani Date: Sat, 25 Jul 2020 23:04:05 +0200 Subject: Backport: Fix subtle bug when checking liveness of module values. We can't simply look up the Lisp object in the global reference table because an invalid local and a valid global reference might refer to the same object. Instead, we have to test the address of the global reference against the stored references. * src/emacs-module.c (module_global_reference_p): New helper function. (value_to_lisp): Use it. (cherry picked from commit 6355a3ec62f43c9b99d483982ff851d32dd78891) --- src/emacs-module.c | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/emacs-module.c b/src/emacs-module.c index 4269b0ba2ac..099a6a3cf25 100644 --- a/src/emacs-module.c +++ b/src/emacs-module.c @@ -78,6 +78,7 @@ To add a new module function, proceed as follows: #include "emacs-module.h" #include +#include #include #include #include @@ -380,6 +381,28 @@ XMODULE_GLOBAL_REFERENCE (Lisp_Object o) return XUNTAG (o, Lisp_Vectorlike, struct module_global_reference); } +/* Returns whether V is a global reference. Only used to check module + assertions. If V is not a global reference, increment *N by the + number of global references (for debugging output). */ + +static bool +module_global_reference_p (emacs_value v, ptrdiff_t *n) +{ + struct Lisp_Hash_Table *h = XHASH_TABLE (Vmodule_refs_hash); + /* Note that we can't use `hash_lookup' because V might be a local + reference that's identical to some global reference. */ + for (ptrdiff_t i = 0; i < HASH_TABLE_SIZE (h); ++i) + { + if (!EQ (HASH_KEY (h, i), Qunbound) + && &XMODULE_GLOBAL_REFERENCE (HASH_VALUE (h, i))->value == v) + return true; + } + /* Only used for debugging, so we don't care about overflow, just + make sure the operation is defined. */ + INT_ADD_WRAPV (*n, h->count, n); + return false; +} + static emacs_value module_make_global_ref (emacs_env *env, emacs_value ref) { @@ -1217,10 +1240,8 @@ value_to_lisp (emacs_value v) ++num_environments; } /* Also check global values. */ - struct Lisp_Hash_Table *h = XHASH_TABLE (Vmodule_refs_hash); - if (hash_lookup (h, v->v, NULL) != -1) + if (module_global_reference_p (v, &num_values)) goto ok; - INT_ADD_WRAPV (num_values, h->count, &num_values); module_abort (("Emacs value not found in %"pD"d values " "of %"pD"d environments"), num_values, num_environments); -- cgit v1.2.1 From d767418b76818e4e83bf19cc08307c1329144c13 Mon Sep 17 00:00:00 2001 From: Philipp Stephani Date: Sat, 25 Jul 2020 23:23:19 +0200 Subject: Backport: Make checking for liveness of global values more precise. We can't just use a hash lookup because a global and a local reference might refer to the same Lisp object. * src/emacs-module.c (module_free_global_ref): More precise check for global liveness. (cherry picked from commit 9f01ce6327af886f26399924a9aadf16cdd4fd9f) --- src/emacs-module.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) (limited to 'src') diff --git a/src/emacs-module.c b/src/emacs-module.c index 099a6a3cf25..a90a9765dbf 100644 --- a/src/emacs-module.c +++ b/src/emacs-module.c @@ -448,6 +448,14 @@ module_free_global_ref (emacs_env *env, emacs_value ref) Lisp_Object obj = value_to_lisp (ref); ptrdiff_t i = hash_lookup (h, obj, NULL); + if (module_assertions) + { + ptrdiff_t n = 0; + if (! module_global_reference_p (ref, &n)) + module_abort ("Global value was not found in list of %"pD"d globals", + n); + } + if (i >= 0) { Lisp_Object value = HASH_VALUE (h, i); @@ -456,11 +464,6 @@ module_free_global_ref (emacs_env *env, emacs_value ref) if (--ref->refcount == 0) hash_remove_from_table (h, obj); } - else if (module_assertions) - { - module_abort ("Global value was not found in list of %"pD"d globals", - h->count); - } } static enum emacs_funcall_exit -- cgit v1.2.1 From 418ea25bbf306c448516ea79c9eaf25b904e62e4 Mon Sep 17 00:00:00 2001 From: Eli Zaretskii Date: Sun, 2 Aug 2020 17:05:00 +0300 Subject: Fix last change in alloc.c. * src/alloc.c (mark_maybe_object) [WIDE_EMACS_INT]: Avoid compiler warning about 'overflow' being unused. --- src/alloc.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'src') diff --git a/src/alloc.c b/src/alloc.c index be293cca54a..da11426075b 100644 --- a/src/alloc.c +++ b/src/alloc.c @@ -4649,6 +4649,8 @@ mark_maybe_object (Lisp_Object obj) significant bits as tag bits, the tag is small enough to not overflow either. */ eassert (!overflow); +#else + (void) overflow; #endif void *po = (char *) ((intptr_t) (char *) XLP (obj) + offset); -- cgit v1.2.1 From 2e9d1f4d44036e7c0605cfeac091368e013e3ed9 Mon Sep 17 00:00:00 2001 From: Philipp Stephani Date: Sun, 2 Aug 2020 16:05:44 +0200 Subject: * src/alloc.c (mark_maybe_object): Avoid signed integer overflow --- src/alloc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/alloc.c b/src/alloc.c index da11426075b..5220ef84783 100644 --- a/src/alloc.c +++ b/src/alloc.c @@ -4652,7 +4652,8 @@ mark_maybe_object (Lisp_Object obj) #else (void) overflow; #endif - void *po = (char *) ((intptr_t) (char *) XLP (obj) + offset); + INT_ADD_WRAPV (offset, (intptr_t) (char *) XLP (obj), &offset); + void *po = (char *) offset; /* If the pointer is in the dump image and the dump has a record of the object starting at the place where the pointer points, we -- cgit v1.2.1 From a07ec21bf24b8d1dc41808f997dd0fb78cad3870 Mon Sep 17 00:00:00 2001 From: Eli Zaretskii Date: Sun, 2 Aug 2020 18:27:33 +0300 Subject: Re-enable scroll-margin when cursor-motion optimization is disabled * src/xdisp.c (try_window): Fix logic of disabling margins when cursor is close to BOB or EOB. Account for header-line, if any, when computing the scroll margin in pixels. (Bug#42653) --- src/xdisp.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/xdisp.c b/src/xdisp.c index fc17014c029..a8cd4dc853c 100644 --- a/src/xdisp.c +++ b/src/xdisp.c @@ -19223,18 +19223,20 @@ try_window (Lisp_Object window, struct text_pos pos, int flags) && !MINI_WINDOW_P (w)) { int this_scroll_margin = window_scroll_margin (w, MARGIN_IN_PIXELS); + if (window_wants_header_line (w)) + this_scroll_margin += CURRENT_HEADER_LINE_HEIGHT (w); start_display (&it, w, pos); if ((w->cursor.y >= 0 /* not vscrolled */ && w->cursor.y < this_scroll_margin - && CHARPOS (pos) > BEGV - && it_charpos < ZV) + && CHARPOS (pos) > BEGV) /* rms: considering make_cursor_line_fully_visible_p here seems to give wrong results. We don't want to recenter when the last line is partly visible, we want to allow that case to be handled in the usual way. */ - || w->cursor.y > (it.last_visible_y - partial_line_height (&it) - - this_scroll_margin - 1)) + || (it_charpos < ZV /* if EOB is visible, disable bottom margin */ + && w->cursor.y > (it.last_visible_y - partial_line_height (&it) + - this_scroll_margin - 1))) { w->cursor.vpos = -1; clear_glyph_matrix (w->desired_matrix); -- cgit v1.2.1 From 72c5f71cd45c860299950cd058d8e13b87375741 Mon Sep 17 00:00:00 2001 From: Grégory Mounié Date: Sun, 2 Aug 2020 15:56:33 +0200 Subject: Avoid segfaults if XIM is set but not xim_styles Emacs segfaults at the X11 initialization if XIM is set and xim_styles is NULL. This patch avoids the crash. * src/xfns.c: Check also if FRAME_X_XIM_STYLES(f) is NULL. (Bug#42676) (Bug#42673) (Bug#42677) Copyright-paperwork-exempt: yes --- src/xfns.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/xfns.c b/src/xfns.c index b89fac1cdac..f9a00a6dafd 100644 --- a/src/xfns.c +++ b/src/xfns.c @@ -2658,7 +2658,7 @@ create_frame_xic (struct frame *f) goto out; xim = FRAME_X_XIM (f); - if (!xim) + if (!xim || ! FRAME_X_XIM_STYLES(f)) goto out; /* Determine XIC style. */ -- cgit v1.2.1 From f921feceb8cd8c52f281447c984d0b67a738a33c Mon Sep 17 00:00:00 2001 From: Derek Zhou Date: Mon, 3 Aug 2020 07:56:22 +0200 Subject: Fix problem where TLS connections would sometimes hang * src/process.c (wait_reading_process_output): Before the select, check every interesting gnutls stream for available data in the buffer. If some of them hit, and either there is no wait_proc or the wait_proc is one of the gnutls streams with new data, set the select timeout to 0 after the select, and merge the gnutls buffer status into the select returns (bug#40665). This fixes a problem where TLS connections would sometimes hang. --- src/process.c | 97 ++++++++++++++++++++++++++++------------------------------- 1 file changed, 46 insertions(+), 51 deletions(-) (limited to 'src') diff --git a/src/process.c b/src/process.c index 6e5bcf307ab..15634e4a8b0 100644 --- a/src/process.c +++ b/src/process.c @@ -5491,6 +5491,10 @@ wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd, } else { +#ifdef HAVE_GNUTLS + int tls_nfds; + fd_set tls_available; +#endif /* Set the timeout for adaptive read buffering if any process has non-zero read_output_skip and non-zero read_output_delay, and we are not reading output for a @@ -5560,7 +5564,36 @@ wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd, } #endif -/* Non-macOS HAVE_GLIB builds call thread_select in xgselect.c. */ +#ifdef HAVE_GNUTLS + /* GnuTLS buffers data internally. We need to check if some + data is available in the buffers manually before the select. + And if so, we need to skip the select which could block. */ + FD_ZERO (&tls_available); + tls_nfds = 0; + for (channel = 0; channel < FD_SETSIZE; ++channel) + if (! NILP (chan_process[channel]) + && FD_ISSET (channel, &Available)) + { + struct Lisp_Process *p = XPROCESS (chan_process[channel]); + if (p + && p->gnutls_p && p->gnutls_state + && emacs_gnutls_record_check_pending (p->gnutls_state) > 0) + { + tls_nfds++; + eassert (p->infd == channel); + FD_SET (p->infd, &tls_available); + } + } + /* If wait_proc is somebody else, we have to wait in select + as usual. Otherwise, clobber the timeout. */ + if (tls_nfds > 0 + && (!wait_proc || + (wait_proc->infd >= 0 + && FD_ISSET (wait_proc->infd, &tls_available)))) + timeout = make_timespec (0, 0); +#endif + + /* Non-macOS HAVE_GLIB builds call thread_select in xgselect.c. */ #if defined HAVE_GLIB && !defined HAVE_NS nfds = xg_select (max_desc + 1, &Available, (check_write ? &Writeok : 0), @@ -5578,59 +5611,21 @@ wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd, #endif /* !HAVE_GLIB */ #ifdef HAVE_GNUTLS - /* GnuTLS buffers data internally. In lowat mode it leaves - some data in the TCP buffers so that select works, but - with custom pull/push functions we need to check if some - data is available in the buffers manually. */ - if (nfds == 0) + /* Merge tls_available into Available. */ + if (tls_nfds > 0) { - fd_set tls_available; - int set = 0; - - FD_ZERO (&tls_available); - if (! wait_proc) + if (nfds == 0 || (nfds < 0 && errno == EINTR)) { - /* We're not waiting on a specific process, so loop - through all the channels and check for data. - This is a workaround needed for some versions of - the gnutls library -- 2.12.14 has been confirmed - to need it. */ - for (channel = 0; channel < FD_SETSIZE; ++channel) - if (! NILP (chan_process[channel])) - { - struct Lisp_Process *p = - XPROCESS (chan_process[channel]); - if (p && p->gnutls_p && p->gnutls_state - && ((emacs_gnutls_record_check_pending - (p->gnutls_state)) - > 0)) - { - nfds++; - eassert (p->infd == channel); - FD_SET (p->infd, &tls_available); - set++; - } - } - } - else - { - /* Check this specific channel. */ - if (wait_proc->gnutls_p /* Check for valid process. */ - && wait_proc->gnutls_state - /* Do we have pending data? */ - && ((emacs_gnutls_record_check_pending - (wait_proc->gnutls_state)) - > 0)) - { - nfds = 1; - eassert (0 <= wait_proc->infd); - /* Set to Available. */ - FD_SET (wait_proc->infd, &tls_available); - set++; - } + /* Fast path, just copy. */ + nfds = tls_nfds; + Available = tls_available; } - if (set) - Available = tls_available; + else if (nfds > 0) + /* Slow path, merge one by one. Note: nfds does not need + to be accurate, just positive is enough. */ + for (channel = 0; channel < FD_SETSIZE; ++channel) + if (FD_ISSET(channel, &tls_available)) + FD_SET(channel, &Available); } #endif } -- cgit v1.2.1 From 99275822c6c36ac308a7b77b5271066df5f38dfb Mon Sep 17 00:00:00 2001 From: Eli Zaretskii Date: Mon, 3 Aug 2020 17:56:40 +0300 Subject: Fix last change in 'try_window' * src/xdisp.c (try_window): Don't modify the logic when EOB is in the viewport. (Bug#42653) --- src/xdisp.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/xdisp.c b/src/xdisp.c index a8cd4dc853c..9f07361d48b 100644 --- a/src/xdisp.c +++ b/src/xdisp.c @@ -19234,9 +19234,8 @@ try_window (Lisp_Object window, struct text_pos pos, int flags) seems to give wrong results. We don't want to recenter when the last line is partly visible, we want to allow that case to be handled in the usual way. */ - || (it_charpos < ZV /* if EOB is visible, disable bottom margin */ - && w->cursor.y > (it.last_visible_y - partial_line_height (&it) - - this_scroll_margin - 1))) + || w->cursor.y > (it.last_visible_y - partial_line_height (&it) + - this_scroll_margin - 1)) { w->cursor.vpos = -1; clear_glyph_matrix (w->desired_matrix); -- cgit v1.2.1 From a4ed198e8f3754a59cabbb03ab6bae8a49597ee0 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Mon, 3 Aug 2020 15:21:58 -0700 Subject: Simplify pointer computation in mark_maybe_object * src/alloc.c (mark_maybe_object): Use simpler way to avoid -fsanitize=undefined false alarms, by converting the word tag to intptr_t first. Omit now-unnecessary runtime overflow check. (mark_memory): Work even if UINTPTR_MAX <= INT_MAX (!). --- src/alloc.c | 24 +++++++----------------- 1 file changed, 7 insertions(+), 17 deletions(-) (limited to 'src') diff --git a/src/alloc.c b/src/alloc.c index 5220ef84783..3a02ef3f8c4 100644 --- a/src/alloc.c +++ b/src/alloc.c @@ -4625,7 +4625,7 @@ mark_maybe_object (Lisp_Object obj) #endif int type_tag = XTYPE (obj); - intptr_t offset; + intptr_t pointer_word_tag = LISP_WORD_TAG (type_tag), offset, ipo; switch (type_tag) { @@ -4641,19 +4641,8 @@ mark_maybe_object (Lisp_Object obj) break; } - bool overflow - = INT_SUBTRACT_WRAPV (offset, LISP_WORD_TAG (type_tag), &offset); -#if !defined WIDE_EMACS_INT || USE_LSB_TAG - /* If we don't use wide integers, then `intptr_t' should always be - large enough to not overflow. Furthermore, when using the least - significant bits as tag bits, the tag is small enough to not - overflow either. */ - eassert (!overflow); -#else - (void) overflow; -#endif - INT_ADD_WRAPV (offset, (intptr_t) (char *) XLP (obj), &offset); - void *po = (char *) offset; + INT_ADD_WRAPV ((intptr_t) XLP (obj), offset - pointer_word_tag, &ipo); + void *po = (void *) ipo; /* If the pointer is in the dump image and the dump has a record of the object starting at the place where the pointer points, we @@ -4856,7 +4845,7 @@ mark_memory (void const *start, void const *end) for (pp = start; (void const *) pp < end; pp += GC_POINTER_ALIGNMENT) { - char *p = *(char *const *) pp; + void *p = *(void *const *) pp; mark_maybe_pointer (p); /* Unmask any struct Lisp_Symbol pointer that make_lisp_symbol @@ -4864,8 +4853,9 @@ mark_memory (void const *start, void const *end) On a host with 32-bit pointers and 64-bit Lisp_Objects, a Lisp_Object might be split into registers saved into non-adjacent words and P might be the low-order word's value. */ - p = (char *) ((uintptr_t) p + (uintptr_t) lispsym); - mark_maybe_pointer (p); + intptr_t ip; + INT_ADD_WRAPV ((intptr_t) p, (intptr_t) lispsym, &ip); + mark_maybe_pointer ((void *) ip); verify (alignof (Lisp_Object) % GC_POINTER_ALIGNMENT == 0); if (alignof (Lisp_Object) == GC_POINTER_ALIGNMENT -- cgit v1.2.1 From a1436544ff826b8c51242f4afb7c5d485c8e2e32 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Mon, 3 Aug 2020 15:21:59 -0700 Subject: Simplify use of __lsan_ignore_object * configure.ac: Use AC_CHECK_FUNCS_ONCE for __lsan_ignore_object. * src/buffer.c, src/data.c, src/emacs-module.c, src/regex-emacs.c: * src/search.c: Use __lsan_ignore_object unconditionally, and don’t include sanitizer/lsan_interface.h. * src/lisp.h (__lsan_ignore_object): Provide a dummy in the typical case where leak sanitization is not available. --- src/buffer.c | 6 ------ src/data.c | 6 ------ src/emacs-module.c | 8 -------- src/lisp.h | 11 +++++++++++ src/regex-emacs.c | 6 ------ src/search.c | 6 ------ 6 files changed, 11 insertions(+), 32 deletions(-) (limited to 'src') diff --git a/src/buffer.c b/src/buffer.c index e441499aeb0..241f2d43a93 100644 --- a/src/buffer.c +++ b/src/buffer.c @@ -28,10 +28,6 @@ along with GNU Emacs. If not, see . */ #include #include -#ifdef HAVE_SANITIZER_LSAN_INTERFACE_H -#include -#endif - #include #include "lisp.h" @@ -5087,9 +5083,7 @@ enlarge_buffer_text (struct buffer *b, ptrdiff_t delta) #else p = xrealloc (b->text->beg, new_nbytes); #endif -#ifdef HAVE___LSAN_IGNORE_OBJECT __lsan_ignore_object (p); -#endif if (p == NULL) { diff --git a/src/data.c b/src/data.c index 5fff52d24c2..59d148166fe 100644 --- a/src/data.c +++ b/src/data.c @@ -23,10 +23,6 @@ along with GNU Emacs. If not, see . */ #include #include -#ifdef HAVE_SANITIZER_LSAN_INTERFACE_H -#include -#endif - #include #include #include @@ -1788,9 +1784,7 @@ make_blv (struct Lisp_Symbol *sym, bool forwarded, set_blv_defcell (blv, tem); set_blv_valcell (blv, tem); set_blv_found (blv, false); -#ifdef HAVE___LSAN_IGNORE_OBJECT __lsan_ignore_object (blv); -#endif return blv; } diff --git a/src/emacs-module.c b/src/emacs-module.c index f57101946b3..a0bab118019 100644 --- a/src/emacs-module.c +++ b/src/emacs-module.c @@ -84,10 +84,6 @@ To add a new module function, proceed as follows: #include #include -#ifdef HAVE_SANITIZER_LSAN_INTERFACE_H -#include -#endif - #include "lisp.h" #include "bignum.h" #include "dynlib.h" @@ -1103,9 +1099,7 @@ DEFUN ("module-load", Fmodule_load, Smodule_load, 1, 1, 0, if (module_assertions) { rt = xmalloc (sizeof *rt); -#ifdef HAVE___LSAN_IGNORE_OBJECT __lsan_ignore_object (rt); -#endif } else rt = &rt_pub; @@ -1426,9 +1420,7 @@ initialize_environment (emacs_env *env, struct emacs_env_private *priv) if (module_assertions) { env = xmalloc (sizeof *env); -#ifdef HAVE___LSAN_IGNORE_OBJECT __lsan_ignore_object (env); -#endif } priv->pending_non_local_exit = emacs_funcall_exit_return; diff --git a/src/lisp.h b/src/lisp.h index fdf69ab7368..22ddf3e5faf 100644 --- a/src/lisp.h +++ b/src/lisp.h @@ -4789,6 +4789,17 @@ lispstpcpy (char *dest, Lisp_Object string) return dest + len; } +#if (defined HAVE___LSAN_IGNORE_OBJECT \ + && defined HAVE_SANITIZER_LSAN_INTERFACE_H) +# include +#else +/* Treat *P as a non-leak. */ +INLINE void +__lsan_ignore_object (void const *p) +{ +} +#endif + extern void xputenv (const char *); extern char *egetenv_internal (const char *, ptrdiff_t); diff --git a/src/regex-emacs.c b/src/regex-emacs.c index 1ecbc74b96c..c44cce9f787 100644 --- a/src/regex-emacs.c +++ b/src/regex-emacs.c @@ -29,10 +29,6 @@ #include -#ifdef HAVE_SANITIZER_LSAN_INTERFACE_H -#include -#endif - #include "character.h" #include "buffer.h" #include "syntax.h" @@ -1761,9 +1757,7 @@ regex_compile (re_char *pattern, ptrdiff_t size, /* Initialize the compile stack. */ compile_stack.stack = xmalloc (INIT_COMPILE_STACK_SIZE * sizeof *compile_stack.stack); -#ifdef HAVE___LSAN_IGNORE_OBJECT __lsan_ignore_object (compile_stack.stack); -#endif compile_stack.size = INIT_COMPILE_STACK_SIZE; compile_stack.avail = 0; diff --git a/src/search.c b/src/search.c index 7b74ff91480..38c64caf7c0 100644 --- a/src/search.c +++ b/src/search.c @@ -21,10 +21,6 @@ along with GNU Emacs. If not, see . */ #include -#ifdef HAVE_SANITIZER_LSAN_INTERFACE_H -#include -#endif - #include "lisp.h" #include "character.h" #include "buffer.h" @@ -619,9 +615,7 @@ newline_cache_on_off (struct buffer *buf) if (base_buf->newline_cache == 0) { base_buf->newline_cache = new_region_cache (); -#ifdef HAVE___LSAN_IGNORE_OBJECT __lsan_ignore_object (base_buf->newline_cache); -#endif } } return base_buf->newline_cache; -- cgit v1.2.1 From fd50b3fc45d35549b842a3ac4889b10f7fcf574c Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Mon, 3 Aug 2020 15:21:59 -0700 Subject: Ignore another memory leak * src/pdumper.c (dump_mmap_contiguous_heap): Ignore the heap control block when checking for leaks. --- src/pdumper.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/pdumper.c b/src/pdumper.c index 865ceff6fff..63ee0fcb7f6 100644 --- a/src/pdumper.c +++ b/src/pdumper.c @@ -4680,15 +4680,15 @@ dump_mmap_contiguous_heap (struct dump_memory_map *maps, int nr_maps, Beware: the simple patch 2019-03-11T15:20:54Z!eggert@cs.ucla.edu is worse, as it sometimes frees this storage twice. */ struct dump_memory_map_heap_control_block *cb = calloc (1, sizeof (*cb)); - - char *mem; if (!cb) goto out; + __lsan_ignore_object (cb); + cb->refcount = 1; cb->mem = malloc (total_size); if (!cb->mem) goto out; - mem = cb->mem; + char *mem = cb->mem; for (int i = 0; i < nr_maps; ++i) { struct dump_memory_map *map = &maps[i]; -- cgit v1.2.1 From 19e76f6190c5c7b939bb15c8ab1137c5db2871c0 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Mon, 3 Aug 2020 15:21:59 -0700 Subject: Use void * for pointers in with_echo_area_buffer * src/xdisp.c (with_echo_area_buffer): Pass void * instead of ptrdiff_t, since the values are typically pointers and this ports better to (mostly-theoretical) hosts where ptrdiff_t is narrower than intptr_t. All uses changed. --- src/xdisp.c | 45 +++++++++++++++++++++------------------------ 1 file changed, 21 insertions(+), 24 deletions(-) (limited to 'src') diff --git a/src/xdisp.c b/src/xdisp.c index 9f07361d48b..4fe1c4288af 100644 --- a/src/xdisp.c +++ b/src/xdisp.c @@ -993,12 +993,12 @@ static void handle_line_prefix (struct it *); static void handle_stop_backwards (struct it *, ptrdiff_t); static void unwind_with_echo_area_buffer (Lisp_Object); static Lisp_Object with_echo_area_buffer_unwind_data (struct window *); -static bool current_message_1 (ptrdiff_t, Lisp_Object); -static bool truncate_message_1 (ptrdiff_t, Lisp_Object); +static bool current_message_1 (void *, Lisp_Object); +static bool truncate_message_1 (void *, Lisp_Object); static void set_message (Lisp_Object); -static bool set_message_1 (ptrdiff_t, Lisp_Object); -static bool display_echo_area_1 (ptrdiff_t, Lisp_Object); -static bool resize_mini_window_1 (ptrdiff_t, Lisp_Object); +static bool set_message_1 (void *, Lisp_Object); +static bool display_echo_area_1 (void *, Lisp_Object); +static bool resize_mini_window_1 (void *, Lisp_Object); static void unwind_redisplay (void); static void extend_face_to_end_of_line (struct it *); static intmax_t message_log_check_duplicate (ptrdiff_t, ptrdiff_t); @@ -11278,8 +11278,8 @@ ensure_echo_area_buffers (void) static bool with_echo_area_buffer (struct window *w, int which, - bool (*fn) (ptrdiff_t, Lisp_Object), - ptrdiff_t a1, Lisp_Object a2) + bool (*fn) (void *, Lisp_Object), + void *a1, Lisp_Object a2) { Lisp_Object buffer; bool this_one, the_other, clear_buffer_p, rc; @@ -11550,8 +11550,7 @@ display_echo_area (struct window *w) window_height_changed_p = with_echo_area_buffer (w, display_last_displayed_message_p, - display_echo_area_1, - (intptr_t) w, Qnil); + display_echo_area_1, w, Qnil); if (no_message_p) echo_area_buffer[i] = Qnil; @@ -11568,10 +11567,9 @@ display_echo_area (struct window *w) Value is true if height of W was changed. */ static bool -display_echo_area_1 (ptrdiff_t a1, Lisp_Object a2) +display_echo_area_1 (void *a1, Lisp_Object a2) { - intptr_t i1 = a1; - struct window *w = (struct window *) i1; + struct window *w = a1; Lisp_Object window; struct text_pos start; @@ -11612,7 +11610,7 @@ resize_echo_area_exactly (void) struct window *w = XWINDOW (echo_area_window); Lisp_Object resize_exactly = (minibuf_level == 0 ? Qt : Qnil); bool resized_p = with_echo_area_buffer (w, 0, resize_mini_window_1, - (intptr_t) w, resize_exactly); + w, resize_exactly); if (resized_p) { windows_or_buffers_changed = 42; @@ -11630,10 +11628,9 @@ resize_echo_area_exactly (void) returns. */ static bool -resize_mini_window_1 (ptrdiff_t a1, Lisp_Object exactly) +resize_mini_window_1 (void *a1, Lisp_Object exactly) { - intptr_t i1 = a1; - return resize_mini_window ((struct window *) i1, !NILP (exactly)); + return resize_mini_window (a1, !NILP (exactly)); } @@ -11769,8 +11766,7 @@ current_message (void) msg = Qnil; else { - with_echo_area_buffer (0, 0, current_message_1, - (intptr_t) &msg, Qnil); + with_echo_area_buffer (0, 0, current_message_1, &msg, Qnil); if (NILP (msg)) echo_area_buffer[0] = Qnil; } @@ -11780,10 +11776,9 @@ current_message (void) static bool -current_message_1 (ptrdiff_t a1, Lisp_Object a2) +current_message_1 (void *a1, Lisp_Object a2) { - intptr_t i1 = a1; - Lisp_Object *msg = (Lisp_Object *) i1; + Lisp_Object *msg = a1; if (Z > BEG) *msg = make_buffer_string (BEG, Z, true); @@ -11857,7 +11852,8 @@ truncate_echo_area (ptrdiff_t nchars) just an informative message; if the frame hasn't really been initialized yet, just toss it. */ if (sf->glyphs_initialized_p) - with_echo_area_buffer (0, 0, truncate_message_1, nchars, Qnil); + with_echo_area_buffer (0, 0, truncate_message_1, + (void *) (intptr_t) nchars, Qnil); } } @@ -11866,8 +11862,9 @@ truncate_echo_area (ptrdiff_t nchars) message to at most NCHARS characters. */ static bool -truncate_message_1 (ptrdiff_t nchars, Lisp_Object a2) +truncate_message_1 (void *a1, Lisp_Object a2) { + intptr_t nchars = (intptr_t) a1; if (BEG + nchars < Z) del_range (BEG + nchars, Z); if (Z == BEG) @@ -11919,7 +11916,7 @@ set_message (Lisp_Object string) This function is called with the echo area buffer being current. */ static bool -set_message_1 (ptrdiff_t a1, Lisp_Object string) +set_message_1 (void *a1, Lisp_Object string) { eassert (STRINGP (string)); -- cgit v1.2.1 From fe2649528b0b7637e6b6851c41e696a1016d8d53 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Tue, 4 Aug 2020 11:09:55 -0700 Subject: Drop support for -fcheck-pointer-bounds GCC has removed the -fcheck-pointer bounds option, and the Linux kernel has also removed support for Intel MPX, so there’s no point to keeping this debugging option within Emacs. * src/bytecode.c (BYTE_CODE_THREADED): * src/lisp.h (DEFINE_LISP_SYMBOL, XSYMBOL, make_lisp_symbol): Assume __CHKP__ is not defined. * src/ptr-bounds.h: Remove. All uses of ptr_bounds_clip, ptr_bounds_copy, ptr_bounds_init, ptr_bounds_set removed. --- src/alloc.c | 31 ++++++++-------------- src/bytecode.c | 11 +++----- src/callint.c | 4 --- src/dispnew.c | 7 ----- src/editfns.c | 3 --- src/emacs.c | 1 - src/frame.c | 6 ----- src/fringe.c | 5 +--- src/gmalloc.c | 16 +++--------- src/lisp.h | 31 +++++----------------- src/ptr-bounds.h | 79 -------------------------------------------------------- 11 files changed, 26 insertions(+), 168 deletions(-) delete mode 100644 src/ptr-bounds.h (limited to 'src') diff --git a/src/alloc.c b/src/alloc.c index 3a02ef3f8c4..b16b2f8b93e 100644 --- a/src/alloc.c +++ b/src/alloc.c @@ -34,7 +34,6 @@ along with GNU Emacs. If not, see . */ #include "bignum.h" #include "dispextern.h" #include "intervals.h" -#include "ptr-bounds.h" #include "puresize.h" #include "sheap.h" #include "sysstdio.h" @@ -1624,8 +1623,7 @@ static struct Lisp_String *string_free_list; a pointer to the `u.data' member of its sdata structure; the structure starts at a constant offset in front of that. */ -#define SDATA_OF_STRING(S) ((sdata *) ptr_bounds_init ((S)->u.s.data \ - - SDATA_DATA_OFFSET)) +#define SDATA_OF_STRING(S) ((sdata *) ((S)->u.s.data - SDATA_DATA_OFFSET)) #ifdef GC_CHECK_STRING_OVERRUN @@ -1799,7 +1797,7 @@ allocate_string (void) /* Every string on a free list should have NULL data pointer. */ s->u.s.data = NULL; NEXT_FREE_LISP_STRING (s) = string_free_list; - string_free_list = ptr_bounds_clip (s, sizeof *s); + string_free_list = s; } } @@ -1908,7 +1906,7 @@ allocate_string_data (struct Lisp_String *s, MALLOC_UNBLOCK_INPUT; - s->u.s.data = ptr_bounds_clip (SDATA_DATA (data), nbytes + 1); + s->u.s.data = SDATA_DATA (data); #ifdef GC_CHECK_STRING_BYTES SDATA_NBYTES (data) = nbytes; #endif @@ -2036,7 +2034,7 @@ sweep_strings (void) /* Put the string on the free-list. */ NEXT_FREE_LISP_STRING (s) = string_free_list; - string_free_list = ptr_bounds_clip (s, sizeof *s); + string_free_list = s; ++nfree; } } @@ -2044,7 +2042,7 @@ sweep_strings (void) { /* S was on the free-list before. Put it there again. */ NEXT_FREE_LISP_STRING (s) = string_free_list; - string_free_list = ptr_bounds_clip (s, sizeof *s); + string_free_list = s; ++nfree; } } @@ -2171,8 +2169,7 @@ compact_small_strings (void) { eassert (tb != b || to < from); memmove (to, from, size + GC_STRING_EXTRA); - to->string->u.s.data - = ptr_bounds_clip (SDATA_DATA (to), nbytes + 1); + to->string->u.s.data = SDATA_DATA (to); } /* Advance past the sdata we copied to. */ @@ -2959,7 +2956,6 @@ Lisp_Object zero_vector; static void setup_on_free_list (struct Lisp_Vector *v, ptrdiff_t nbytes) { - v = ptr_bounds_clip (v, nbytes); eassume (header_size <= nbytes); ptrdiff_t nwords = (nbytes - header_size) / word_size; XSETPVECTYPESIZE (v, PVEC_FREE, 0, nwords); @@ -3307,7 +3303,7 @@ allocate_vectorlike (ptrdiff_t len, bool clearit) MALLOC_UNBLOCK_INPUT; - return ptr_bounds_clip (p, nbytes); + return p; } @@ -4461,7 +4457,6 @@ live_string_holding (struct mem_node *m, void *p) must not be on the free-list. */ if (0 <= offset && offset < sizeof b->strings) { - cp = ptr_bounds_copy (cp, b); struct Lisp_String *s = p = cp -= offset % sizeof b->strings[0]; if (s->u.s.data) return s; @@ -4494,7 +4489,6 @@ live_cons_holding (struct mem_node *m, void *p) && (b != cons_block || offset / sizeof b->conses[0] < cons_block_index)) { - cp = ptr_bounds_copy (cp, b); struct Lisp_Cons *s = p = cp -= offset % sizeof b->conses[0]; if (!deadp (s->u.s.car)) return s; @@ -4528,7 +4522,6 @@ live_symbol_holding (struct mem_node *m, void *p) && (b != symbol_block || offset / sizeof b->symbols[0] < symbol_block_index)) { - cp = ptr_bounds_copy (cp, b); struct Lisp_Symbol *s = p = cp -= offset % sizeof b->symbols[0]; if (!deadp (s->u.s.function)) return s; @@ -5234,7 +5227,7 @@ pure_alloc (size_t size, int type) pure_bytes_used = pure_bytes_used_lisp + pure_bytes_used_non_lisp; if (pure_bytes_used <= pure_size) - return ptr_bounds_clip (result, size); + return result; /* Don't allocate a large amount here, because it might get mmap'd and then its address @@ -5325,7 +5318,7 @@ find_string_data_in_pure (const char *data, ptrdiff_t nbytes) /* Check the remaining characters. */ if (memcmp (data, non_lisp_beg + start, nbytes) == 0) /* Found. */ - return ptr_bounds_clip (non_lisp_beg + start, nbytes + 1); + return non_lisp_beg + start; start += last_char_skip; } @@ -6049,7 +6042,6 @@ garbage_collect (void) stack_copy = xrealloc (stack_copy, stack_size); stack_copy_size = stack_size; } - stack = ptr_bounds_set (stack, stack_size); no_sanitize_memcpy (stack_copy, stack, stack_size); } } @@ -6885,8 +6877,7 @@ sweep_conses (void) for (pos = start; pos < stop; pos++) { - struct Lisp_Cons *acons - = ptr_bounds_copy (&cblk->conses[pos], cblk); + struct Lisp_Cons *acons = &cblk->conses[pos]; if (!XCONS_MARKED_P (acons)) { this_free++; @@ -6939,7 +6930,7 @@ sweep_floats (void) int this_free = 0; for (int i = 0; i < lim; i++) { - struct Lisp_Float *afloat = ptr_bounds_copy (&fblk->floats[i], fblk); + struct Lisp_Float *afloat = &fblk->floats[i]; if (!XFLOAT_MARKED_P (afloat)) { this_free++; diff --git a/src/bytecode.c b/src/bytecode.c index 5ac30aa1010..1913a4812a0 100644 --- a/src/bytecode.c +++ b/src/bytecode.c @@ -24,7 +24,6 @@ along with GNU Emacs. If not, see . */ #include "character.h" #include "buffer.h" #include "keyboard.h" -#include "ptr-bounds.h" #include "syntax.h" #include "window.h" @@ -47,7 +46,7 @@ along with GNU Emacs. If not, see . */ indirect threaded, using GCC's computed goto extension. This code, as currently implemented, is incompatible with BYTE_CODE_SAFE and BYTE_CODE_METER. */ -#if (defined __GNUC__ && !defined __STRICT_ANSI__ && !defined __CHKP__ \ +#if (defined __GNUC__ && !defined __STRICT_ANSI__ \ && !BYTE_CODE_SAFE && !defined BYTE_CODE_METER) #define BYTE_CODE_THREADED #endif @@ -368,14 +367,12 @@ exec_byte_code (Lisp_Object bytestr, Lisp_Object vector, Lisp_Object maxdepth, USE_SAFE_ALLOCA; void *alloc; SAFE_ALLOCA_LISP_EXTRA (alloc, stack_items, bytestr_length); - ptrdiff_t item_bytes = stack_items * word_size; - Lisp_Object *stack_base = ptr_bounds_clip (alloc, item_bytes); + Lisp_Object *stack_base = alloc; Lisp_Object *top = stack_base; *top = vector; /* Ensure VECTOR survives GC (Bug#33014). */ Lisp_Object *stack_lim = stack_base + stack_items; - unsigned char *bytestr_data = alloc; - bytestr_data = ptr_bounds_clip (bytestr_data + item_bytes, bytestr_length); - memcpy (bytestr_data, SDATA (bytestr), bytestr_length); + unsigned char const *bytestr_data = memcpy (stack_lim, + SDATA (bytestr), bytestr_length); unsigned char const *pc = bytestr_data; ptrdiff_t count = SPECPDL_INDEX (); diff --git a/src/callint.c b/src/callint.c index eb916353a0c..f609c96a6fa 100644 --- a/src/callint.c +++ b/src/callint.c @@ -21,7 +21,6 @@ along with GNU Emacs. If not, see . */ #include #include "lisp.h" -#include "ptr-bounds.h" #include "character.h" #include "buffer.h" #include "keyboard.h" @@ -440,9 +439,6 @@ invoke it (via an `interactive' spec that contains, for instance, an signed char *varies = (signed char *) (visargs + nargs); memclear (args, nargs * (2 * word_size + 1)); - args = ptr_bounds_clip (args, nargs * sizeof *args); - visargs = ptr_bounds_clip (visargs, nargs * sizeof *visargs); - varies = ptr_bounds_clip (varies, nargs * sizeof *varies); if (!NILP (enable)) specbind (Qenable_recursive_minibuffers, Qt); diff --git a/src/dispnew.c b/src/dispnew.c index 1ae59e3ff2b..d318e26308e 100644 --- a/src/dispnew.c +++ b/src/dispnew.c @@ -25,7 +25,6 @@ along with GNU Emacs. If not, see . */ #include #include "lisp.h" -#include "ptr-bounds.h" #include "termchar.h" /* cm.h must come after dispextern.h on Windows. */ #include "dispextern.h" @@ -4891,12 +4890,6 @@ scrolling (struct frame *frame) unsigned *new_hash = old_hash + height; int *draw_cost = (int *) (new_hash + height); int *old_draw_cost = draw_cost + height; - old_hash = ptr_bounds_clip (old_hash, height * sizeof *old_hash); - new_hash = ptr_bounds_clip (new_hash, height * sizeof *new_hash); - draw_cost = ptr_bounds_clip (draw_cost, height * sizeof *draw_cost); - old_draw_cost = ptr_bounds_clip (old_draw_cost, - height * sizeof *old_draw_cost); - eassert (current_matrix); /* Compute hash codes of all the lines. Also calculate number of diff --git a/src/editfns.c b/src/editfns.c index 763d95bb8fa..cb09ea8a31a 100644 --- a/src/editfns.c +++ b/src/editfns.c @@ -46,7 +46,6 @@ along with GNU Emacs. If not, see . */ #include "composite.h" #include "intervals.h" -#include "ptr-bounds.h" #include "systime.h" #include "character.h" #include "buffer.h" @@ -3131,8 +3130,6 @@ styled_format (ptrdiff_t nargs, Lisp_Object *args, bool message) string was not copied into the output. It is 2 if byte I was not the first byte of its character. */ char *discarded = (char *) &info[nspec_bound]; - info = ptr_bounds_clip (info, info_size); - discarded = ptr_bounds_clip (discarded, formatlen); memset (discarded, 0, formatlen); /* Try to determine whether the result should be multibyte. diff --git a/src/emacs.c b/src/emacs.c index 8a6bb3ad228..8e5eaf5e43e 100644 --- a/src/emacs.c +++ b/src/emacs.c @@ -83,7 +83,6 @@ along with GNU Emacs. If not, see . */ #include "charset.h" #include "composite.h" #include "dispextern.h" -#include "ptr-bounds.h" #include "regex-emacs.h" #include "sheap.h" #include "syntax.h" diff --git a/src/frame.c b/src/frame.c index c871e4fd994..c21d4708f75 100644 --- a/src/frame.c +++ b/src/frame.c @@ -35,7 +35,6 @@ along with GNU Emacs. If not, see . */ #include "buffer.h" /* These help us bind and responding to switch-frame events. */ #include "keyboard.h" -#include "ptr-bounds.h" #include "frame.h" #include "blockinput.h" #include "termchar.h" @@ -5019,8 +5018,6 @@ gui_display_get_resource (Display_Info *dpyinfo, Lisp_Object attribute, USE_SAFE_ALLOCA; char *name_key = SAFE_ALLOCA (name_keysize + class_keysize); char *class_key = name_key + name_keysize; - name_key = ptr_bounds_clip (name_key, name_keysize); - class_key = ptr_bounds_clip (class_key, class_keysize); /* Start with emacs.FRAMENAME for the name (the specific one) and with `Emacs' for the class key (the general one). */ @@ -5091,9 +5088,6 @@ x_get_resource_string (const char *attribute, const char *class) ptrdiff_t class_keysize = sizeof (EMACS_CLASS) - 1 + strlen (class) + 2; char *name_key = SAFE_ALLOCA (name_keysize + class_keysize); char *class_key = name_key + name_keysize; - name_key = ptr_bounds_clip (name_key, name_keysize); - class_key = ptr_bounds_clip (class_key, class_keysize); - esprintf (name_key, "%s.%s", SSDATA (Vinvocation_name), attribute); sprintf (class_key, "%s.%s", EMACS_CLASS, class); diff --git a/src/fringe.c b/src/fringe.c index fc4c738dc2d..c3d64fefc82 100644 --- a/src/fringe.c +++ b/src/fringe.c @@ -23,7 +23,6 @@ along with GNU Emacs. If not, see . */ #include "lisp.h" #include "frame.h" -#include "ptr-bounds.h" #include "window.h" #include "dispextern.h" #include "buffer.h" @@ -1607,9 +1606,7 @@ If BITMAP already exists, the existing definition is replaced. */) fb.dynamic = true; xfb = xmalloc (sizeof fb + fb.height * BYTES_PER_BITMAP_ROW); - fb.bits = b = ((unsigned short *) - ptr_bounds_clip (xfb + 1, fb.height * BYTES_PER_BITMAP_ROW)); - xfb = ptr_bounds_clip (xfb, sizeof *xfb); + fb.bits = b = (unsigned short *) (xfb + 1); j = 0; while (j < fb.height) diff --git a/src/gmalloc.c b/src/gmalloc.c index 8450a639e77..3560c744539 100644 --- a/src/gmalloc.c +++ b/src/gmalloc.c @@ -38,8 +38,6 @@ License along with this library. If not, see . #include "lisp.h" -#include "ptr-bounds.h" - #ifdef HAVE_MALLOC_H # if GNUC_PREREQ (4, 2, 0) # pragma GCC diagnostic ignored "-Wdeprecated-declarations" @@ -200,8 +198,7 @@ extern size_t _bytes_free; /* Internal versions of `malloc', `realloc', and `free' used when these functions need to call each other. - They are the same but don't call the hooks - and don't bound the resulting pointers. */ + They are the same but don't call the hooks. */ extern void *_malloc_internal (size_t); extern void *_realloc_internal (void *, size_t); extern void _free_internal (void *); @@ -551,7 +548,7 @@ malloc_initialize_1 (void) _heapinfo[0].free.size = 0; _heapinfo[0].free.next = _heapinfo[0].free.prev = 0; _heapindex = 0; - _heapbase = (char *) ptr_bounds_init (_heapinfo); + _heapbase = (char *) _heapinfo; _heaplimit = BLOCK (_heapbase + heapsize * sizeof (malloc_info)); register_heapinfo (); @@ -912,8 +909,7 @@ malloc (size_t size) among multiple threads. We just leave it for compatibility with glibc malloc (i.e., assignments to gmalloc_hook) for now. */ hook = gmalloc_hook; - void *result = (hook ? hook : _malloc_internal) (size); - return ptr_bounds_clip (result, size); + return (hook ? hook : _malloc_internal) (size); } #if !(defined (_LIBC) || defined (HYBRID_MALLOC)) @@ -991,7 +987,6 @@ _free_internal_nolock (void *ptr) if (ptr == NULL) return; - ptr = ptr_bounds_init (ptr); PROTECT_MALLOC_STATE (0); @@ -1303,7 +1298,6 @@ _realloc_internal_nolock (void *ptr, size_t size) else if (ptr == NULL) return _malloc_internal_nolock (size); - ptr = ptr_bounds_init (ptr); block = BLOCK (ptr); PROTECT_MALLOC_STATE (0); @@ -1426,8 +1420,7 @@ realloc (void *ptr, size_t size) return NULL; hook = grealloc_hook; - void *result = (hook ? hook : _realloc_internal) (ptr, size); - return ptr_bounds_clip (result, size); + return (hook ? hook : _realloc_internal) (ptr, size); } /* Copyright (C) 1991, 1992, 1994 Free Software Foundation, Inc. @@ -1601,7 +1594,6 @@ aligned_alloc (size_t alignment, size_t size) { l->exact = result; result = l->aligned = (char *) result + adj; - result = ptr_bounds_clip (result, size); } UNLOCK_ALIGNED_BLOCKS (); if (l == NULL) diff --git a/src/lisp.h b/src/lisp.h index 22ddf3e5faf..17b92a04146 100644 --- a/src/lisp.h +++ b/src/lisp.h @@ -893,8 +893,8 @@ verify (GCALIGNED (struct Lisp_Symbol)); convert it to a Lisp_Word. */ #if LISP_WORDS_ARE_POINTERS /* untagged_ptr is a pointer so that the compiler knows that TAG_PTR - yields a pointer; this can help with gcc -fcheck-pointer-bounds. - It is char * so that adding a tag uses simple machine addition. */ + yields a pointer. It is char * so that adding a tag uses simple + machine addition. */ typedef char *untagged_ptr; typedef uintptr_t Lisp_Word_tag; #else @@ -922,13 +922,9 @@ typedef EMACS_UINT Lisp_Word_tag; when using a debugger like GDB, on older platforms where the debug format does not represent C macros. However, they are unbounded and would just be asking for trouble if checking pointer bounds. */ -#ifdef __CHKP__ -# define DEFINE_LISP_SYMBOL(name) -#else -# define DEFINE_LISP_SYMBOL(name) \ - DEFINE_GDB_SYMBOL_BEGIN (Lisp_Object, name) \ - DEFINE_GDB_SYMBOL_END (LISPSYM_INITIALLY (name)) -#endif +#define DEFINE_LISP_SYMBOL(name) \ + DEFINE_GDB_SYMBOL_BEGIN (Lisp_Object, name) \ + DEFINE_GDB_SYMBOL_END (LISPSYM_INITIALLY (name)) /* The index of the C-defined Lisp symbol SYM. This can be used in a static initializer. */ @@ -1002,30 +998,15 @@ XSYMBOL (Lisp_Object a) eassert (SYMBOLP (a)); intptr_t i = (intptr_t) XUNTAG (a, Lisp_Symbol, struct Lisp_Symbol); void *p = (char *) lispsym + i; -#ifdef __CHKP__ - /* Bypass pointer checking. Although this could be improved it is - probably not worth the trouble. */ - p = __builtin___bnd_set_ptr_bounds (p, sizeof (struct Lisp_Symbol)); -#endif return p; } INLINE Lisp_Object make_lisp_symbol (struct Lisp_Symbol *sym) { -#ifdef __CHKP__ - /* Although '__builtin___bnd_narrow_ptr_bounds (sym, sym, sizeof *sym)' - should be more efficient, it runs afoul of GCC bug 83251 - . - Also, attempting to call __builtin___bnd_chk_ptr_bounds (sym, sizeof *sym) - here seems to trigger a GCC bug, as yet undiagnosed. */ - char *addr = __builtin___bnd_set_ptr_bounds (sym, sizeof *sym); - char *symoffset = addr - (intptr_t) lispsym; -#else - /* If !__CHKP__, GCC 7 x86-64 generates faster code if lispsym is + /* GCC 7 x86-64 generates faster code if lispsym is cast to char * rather than to intptr_t. */ char *symoffset = (char *) ((char *) sym - (char *) lispsym); -#endif Lisp_Object a = TAG_PTR (Lisp_Symbol, symoffset); eassert (XSYMBOL (a) == sym); return a; diff --git a/src/ptr-bounds.h b/src/ptr-bounds.h deleted file mode 100644 index 22d49f25b6c..00000000000 --- a/src/ptr-bounds.h +++ /dev/null @@ -1,79 +0,0 @@ -/* Pointer bounds checking for GNU Emacs - -Copyright 2017-2020 Free Software Foundation, Inc. - -This file is part of GNU Emacs. - -GNU Emacs is free software: you can redistribute it and/or modify -it under the terms of the GNU General Public License as published by -the Free Software Foundation, either version 3 of the License, or (at -your option) any later version. - -GNU Emacs is distributed in the hope that it will be useful, -but WITHOUT ANY WARRANTY; without even the implied warranty of -MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -GNU General Public License for more details. - -You should have received a copy of the GNU General Public License -along with GNU Emacs. If not, see . */ - -/* Pointer bounds checking is a no-op unless running on hardware - supporting Intel MPX (Intel Skylake or better). Also, it requires - GCC 5 and Linux kernel 3.19, or later. Configure with - CFLAGS='-fcheck-pointer-bounds -mmpx', perhaps with - -fchkp-first-field-has-own-bounds thrown in. - - Although pointer bounds checking can help during debugging, it is - disabled by default because it hurts performance significantly. - The checking does not detect all pointer errors. For example, a - dumped Emacs might not detect a bounds violation of a pointer that - was created before Emacs was dumped. */ - -#ifndef PTR_BOUNDS_H -#define PTR_BOUNDS_H - -#include - -/* When not checking pointer bounds, the following macros simply - return their first argument. These macros return either void *, or - the same type as their first argument. */ - -INLINE_HEADER_BEGIN - -/* Return a copy of P, with bounds narrowed to [P, P + N). */ -#ifdef __CHKP__ -INLINE void * -ptr_bounds_clip (void const *p, size_t n) -{ - return __builtin___bnd_narrow_ptr_bounds (p, p, n); -} -#else -# define ptr_bounds_clip(p, n) ((void) (size_t) {n}, p) -#endif - -/* Return a copy of P, but with the bounds of Q. */ -#ifdef __CHKP__ -# define ptr_bounds_copy(p, q) __builtin___bnd_copy_ptr_bounds (p, q) -#else -# define ptr_bounds_copy(p, q) ((void) (void const *) {q}, p) -#endif - -/* Return a copy of P, but with infinite bounds. - This is a loophole in pointer bounds checking. */ -#ifdef __CHKP__ -# define ptr_bounds_init(p) __builtin___bnd_init_ptr_bounds (p) -#else -# define ptr_bounds_init(p) (p) -#endif - -/* Return a copy of P, but with bounds [P, P + N). - This is a loophole in pointer bounds checking. */ -#ifdef __CHKP__ -# define ptr_bounds_set(p, n) __builtin___bnd_set_ptr_bounds (p, n) -#else -# define ptr_bounds_set(p, n) ((void) (size_t) {n}, p) -#endif - -INLINE_HEADER_END - -#endif /* PTR_BOUNDS_H */ -- cgit v1.2.1 From 519a93e067f459ceddb57573261a52118086b73d Mon Sep 17 00:00:00 2001 From: Alan Third Date: Sun, 2 Aug 2020 20:43:56 +0100 Subject: Don't smooth images when scaling up (bug#38394) * src/image.c (image_set_transform [HAVE_XRENDER]): Use different filter when scaling up vs scaling down. * src/nsimage.m (ns_image_set_smoothing): ([EmacsImage setSmoothing:]): New functions. * src/nsterm.h: Add definitions. * src/nsterm.m (ns_dumpglyphs_image): Disable smoothing if requested. --- src/image.c | 20 +++++++++++++++++--- src/nsimage.m | 12 ++++++++++++ src/nsterm.h | 3 +++ src/nsterm.m | 12 ++++++++++++ 4 files changed, 44 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/image.c b/src/image.c index e7e0a93313b..e236b389210 100644 --- a/src/image.c +++ b/src/image.c @@ -259,6 +259,8 @@ cr_put_image_to_cr_data (struct image *img) cairo_matrix_t matrix; cairo_pattern_get_matrix (img->cr_data, &matrix); cairo_pattern_set_matrix (pattern, &matrix); + cairo_pattern_set_filter + (pattern, cairo_pattern_get_filter (img->cr_data)); cairo_pattern_destroy (img->cr_data); } cairo_surface_destroy (surface); @@ -2114,6 +2116,15 @@ image_set_transform (struct frame *f, struct image *img) double rotation = 0.0; compute_image_rotation (img, &rotation); +# if defined USE_CAIRO || defined HAVE_XRENDER || defined HAVE_NS + /* We want scale up operations to use a nearest neighbour filter to + show real pixels instead of munging them, but scale down + operations to use a blended filter, to avoid aliasing and the like. + + TODO: implement for Windows. */ + bool scale_down = (width < img->width) || (height < img->height); +# endif + /* Perform scale transformation. */ matrix3x3 matrix @@ -2225,11 +2236,14 @@ image_set_transform (struct frame *f, struct image *img) /* Under NS the transform is applied to the drawing surface at drawing time, so store it for later. */ ns_image_set_transform (img->pixmap, matrix); + ns_image_set_smoothing (img->pixmap, scale_down); # elif defined USE_CAIRO cairo_matrix_t cr_matrix = {matrix[0][0], matrix[0][1], matrix[1][0], matrix[1][1], matrix[2][0], matrix[2][1]}; cairo_pattern_t *pattern = cairo_pattern_create_rgb (0, 0, 0); cairo_pattern_set_matrix (pattern, &cr_matrix); + cairo_pattern_set_filter (pattern, scale_down + ? CAIRO_FILTER_BEST : CAIRO_FILTER_NEAREST); /* Dummy solid color pattern just to record pattern matrix. */ img->cr_data = pattern; # elif defined (HAVE_XRENDER) @@ -2246,14 +2260,14 @@ image_set_transform (struct frame *f, struct image *img) XDoubleToFixed (matrix[1][2]), XDoubleToFixed (matrix[2][2])}}}; - XRenderSetPictureFilter (FRAME_X_DISPLAY (f), img->picture, FilterBest, - 0, 0); + XRenderSetPictureFilter (FRAME_X_DISPLAY (f), img->picture, + scale_down ? FilterBest : FilterNearest, 0, 0); XRenderSetPictureTransform (FRAME_X_DISPLAY (f), img->picture, &tmat); if (img->mask_picture) { XRenderSetPictureFilter (FRAME_X_DISPLAY (f), img->mask_picture, - FilterBest, 0, 0); + scale_down ? FilterBest : FilterNearest, 0, 0); XRenderSetPictureTransform (FRAME_X_DISPLAY (f), img->mask_picture, &tmat); } diff --git a/src/nsimage.m b/src/nsimage.m index 07750de95fe..966e7044f12 100644 --- a/src/nsimage.m +++ b/src/nsimage.m @@ -199,6 +199,12 @@ ns_image_set_transform (void *img, double m[3][3]) [(EmacsImage *)img setTransform:m]; } +void +ns_image_set_smoothing (void *img, bool smooth) +{ + [(EmacsImage *)img setSmoothing:smooth]; +} + unsigned long ns_get_pixel (void *img, int x, int y) { @@ -591,4 +597,10 @@ ns_set_alpha (void *img, int x, int y, unsigned char a) [transform setTransformStruct:tm]; } +- (void)setSmoothing: (BOOL) s +{ + smoothing = s; +} + + @end diff --git a/src/nsterm.h b/src/nsterm.h index 8d5371c8f24..a511fef5b98 100644 --- a/src/nsterm.h +++ b/src/nsterm.h @@ -640,6 +640,7 @@ typedef id instancetype; unsigned long xbm_fg; @public NSAffineTransform *transform; + BOOL smoothing; } + (instancetype)allocInitFromFile: (Lisp_Object)file; - (void)dealloc; @@ -658,6 +659,7 @@ typedef id instancetype; - (Lisp_Object)getMetadata; - (BOOL)setFrame: (unsigned int) index; - (void)setTransform: (double[3][3]) m; +- (void)setSmoothing: (BOOL)s; @end @@ -1200,6 +1202,7 @@ extern int ns_image_width (void *img); extern int ns_image_height (void *img); extern void ns_image_set_size (void *img, int width, int height); extern void ns_image_set_transform (void *img, double m[3][3]); +extern void ns_image_set_smoothing (void *img, bool smooth); extern unsigned long ns_get_pixel (void *img, int x, int y); extern void ns_put_pixel (void *img, int x, int y, unsigned long argb); extern void ns_set_alpha (void *img, int x, int y, unsigned char a); diff --git a/src/nsterm.m b/src/nsterm.m index df7f716f51e..572b859a982 100644 --- a/src/nsterm.m +++ b/src/nsterm.m @@ -4043,10 +4043,22 @@ ns_dumpglyphs_image (struct glyph_string *s, NSRect r) [doTransform concat]; + /* Smoothing is the default, so if we don't want smoothing we + have to turn it off. */ + if (! img->smoothing) + [[NSGraphicsContext currentContext] + setImageInterpolation:NSImageInterpolationNone]; + [img drawInRect:ir fromRect:ir operation:NSCompositingOperationSourceOver fraction:1.0 respectFlipped:YES hints:nil]; + /* Apparently image interpolation is not reset with + restoreGraphicsState, so we have to manually reset it. */ + if (! img->smoothing) + [[NSGraphicsContext currentContext] + setImageInterpolation:NSImageInterpolationDefault]; + [[NSGraphicsContext currentContext] restoreGraphicsState]; } -- cgit v1.2.1 From 74606481c2859b843ebf3f744c215447458becc2 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Thu, 6 Aug 2020 19:11:58 -0700 Subject: Pacify gcc -Wunused-variable * src/frame.c (Fset_mouse_position, Fset_mouse_pixel_position) (Fset_frame_position): Always use xval, yval. Simplify #if nesting. --- src/frame.c | 33 +++++++++++++++------------------ 1 file changed, 15 insertions(+), 18 deletions(-) (limited to 'src') diff --git a/src/frame.c b/src/frame.c index c21d4708f75..c4dfc35a0c5 100644 --- a/src/frame.c +++ b/src/frame.c @@ -2565,21 +2565,18 @@ before calling this function on it, like this. if (FRAME_WINDOW_P (XFRAME (frame))) /* Warping the mouse will cause enternotify and focus events. */ frame_set_mouse_position (XFRAME (frame), xval, yval); -#else -#if defined (MSDOS) +#elif defined MSDOS if (FRAME_MSDOS_P (XFRAME (frame))) { Fselect_frame (frame, Qnil); mouse_moveto (xval, yval); } +#elif defined HAVE_GPM + Fselect_frame (frame, Qnil); + term_mouse_moveto (xval, yval); #else -#ifdef HAVE_GPM - { - Fselect_frame (frame, Qnil); - term_mouse_moveto (xval, yval); - } -#endif -#endif + (void) xval; + (void) yval; #endif return Qnil; @@ -2606,21 +2603,18 @@ before calling this function on it, like this. if (FRAME_WINDOW_P (XFRAME (frame))) /* Warping the mouse will cause enternotify and focus events. */ frame_set_mouse_pixel_position (XFRAME (frame), xval, yval); -#else -#if defined (MSDOS) +#elif defined MSDOS if (FRAME_MSDOS_P (XFRAME (frame))) { Fselect_frame (frame, Qnil); mouse_moveto (xval, yval); } +#elif defined HAVE_GPM + Fselect_frame (frame, Qnil); + term_mouse_moveto (xval, yval); #else -#ifdef HAVE_GPM - { - Fselect_frame (frame, Qnil); - term_mouse_moveto (xval, yval); - } -#endif -#endif + (void) xval; + (void) yval; #endif return Qnil; @@ -3657,6 +3651,9 @@ bottom edge of FRAME's display. */) #ifdef HAVE_WINDOW_SYSTEM if (FRAME_TERMINAL (f)->set_frame_offset_hook) FRAME_TERMINAL (f)->set_frame_offset_hook (f, xval, yval, 1); +#else + (void) xval; + (void) yval; #endif } -- cgit v1.2.1