From: Sergey Vinokurov <serg.foo@gmail.com>
To: 32479@debbugs.gnu.org
Subject: bug#32479: Fix tracking of freed emacs_values with enabled --module-assertions
Date: Mon, 20 Aug 2018 09:27:46 +0100 [thread overview]
Message-ID: <05d6bad8-a739-8fcc-bd32-1291423c85a5@gmail.com> (raw)
[-- Attachment #1: Type: text/plain, Size: 592 bytes --]
Hi,
I was developing a dynamic module when I noticed that when
'--module-assertions' are enabled I got strange complaints regarding
value to be freed. Namely, Emacs was producing an error message like the
following one:
Emacs module assertion: Global value was not found in list of 10 globals
I tracked it down to a bug in module_free_global_ref. It's traversal of
the linked list of all allocated global values was flawed and only
considered the head of the list. Please find attached my attempt at
fixing it and a test in Emacs test suite that reproduces the issue.
Regards,
Sergey
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-detection-of-freed-emacs_values.patch --]
[-- Type: text/x-patch; name="0001-Fix-detection-of-freed-emacs_values.patch", Size: 3806 bytes --]
From a432465c57f14157f444540af3cea73261580dd6 Mon Sep 17 00:00:00 2001
From: Sergey Vinokurov <serg.foo@gmail.com>
Date: Sun, 19 Aug 2018 21:31:01 +0100
Subject: [PATCH] Fix detection of freed emacs_values
Compare a value to be freed with all entries of the list
* src/emacs-module.c: Fix traversal of the list that contains all
emacs_values allocated by a dynamic module
---
src/emacs-module.c | 8 ++++----
test/data/emacs-module/mod-test.c | 19 +++++++++++++++++++
test/src/emacs-module-tests.el | 3 +++
3 files changed, 26 insertions(+), 4 deletions(-)
diff --git a/src/emacs-module.c b/src/emacs-module.c
index e7ba17426b..f2844c40d0 100644
--- a/src/emacs-module.c
+++ b/src/emacs-module.c
@@ -344,20 +344,20 @@ module_free_global_ref (emacs_env *env, emacs_value ref)
Lisp_Object globals = global_env_private.values;
Lisp_Object prev = Qnil;
ptrdiff_t count = 0;
- for (Lisp_Object tail = global_env_private.values; CONSP (tail);
+ for (Lisp_Object tail = globals; CONSP (tail);
tail = XCDR (tail))
{
- emacs_value global = xmint_pointer (XCAR (globals));
+ emacs_value global = xmint_pointer (XCAR (tail));
if (global == ref)
{
if (NILP (prev))
global_env_private.values = XCDR (globals);
else
- XSETCDR (prev, XCDR (globals));
+ XSETCDR (prev, XCDR (tail));
return;
}
++count;
- prev = globals;
+ prev = tail;
}
module_abort ("Global value was not found in list of %"pD"d globals",
count);
diff --git a/test/data/emacs-module/mod-test.c b/test/data/emacs-module/mod-test.c
index db05e90bc4..a9b459b4cc 100644
--- a/test/data/emacs-module/mod-test.c
+++ b/test/data/emacs-module/mod-test.c
@@ -156,6 +156,24 @@ Fmod_test_globref_make (emacs_env *env, ptrdiff_t nargs, emacs_value args[],
return env->make_global_ref (env, lisp_str);
}
+/* Create a few global references from arguments and free them. */
+static emacs_value
+Fmod_test_globref_free (emacs_env *env, ptrdiff_t nargs, emacs_value args[],
+ void *data)
+{
+ emacs_value refs[10];
+ for (int i = 0; i < 10; i++)
+ {
+ refs[i] = env->make_global_ref (env, args[i % nargs]);
+ }
+ for (int i = 0; i < 10; i++)
+ {
+ env->free_global_ref (env, refs[i]);
+ }
+ return env->intern (env, "ok");
+}
+
+
/* Return a copy of the argument string where every 'a' is replaced
with 'b'. */
@@ -339,6 +357,7 @@ emacs_module_init (struct emacs_runtime *ert)
DEFUN ("mod-test-non-local-exit-funcall", Fmod_test_non_local_exit_funcall,
1, 1, NULL, NULL);
DEFUN ("mod-test-globref-make", Fmod_test_globref_make, 0, 0, NULL, NULL);
+ DEFUN ("mod-test-globref-free", Fmod_test_globref_free, 4, 4, NULL, NULL);
DEFUN ("mod-test-string-a-to-b", Fmod_test_string_a_to_b, 1, 1, NULL, NULL);
DEFUN ("mod-test-userptr-make", Fmod_test_userptr_make, 1, 1, NULL, NULL);
DEFUN ("mod-test-userptr-get", Fmod_test_userptr_get, 1, 1, NULL, NULL);
diff --git a/test/src/emacs-module-tests.el b/test/src/emacs-module-tests.el
index 90cd37a98a..c67190be5c 100644
--- a/test/src/emacs-module-tests.el
+++ b/test/src/emacs-module-tests.el
@@ -148,6 +148,9 @@ multiply-string
(garbage-collect) ;; XXX: not enough to really test but it's something..
(should (string= ref-str mod-str))))
+(ert-deftest mod-test-globref-free-test ()
+ (should (eq (mod-test-globref-free 1 'a "test" 'b) 'ok)))
+
(ert-deftest mod-test-string-a-to-b-test ()
(should (string= (mod-test-string-a-to-b "aaa") "bbb")))
--
2.18.0
next reply other threads:[~2018-08-20 8:27 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-08-20 8:27 Sergey Vinokurov [this message]
2018-08-25 23:03 ` bug#32479: Fix tracking of freed emacs_values with enabled --module-assertions Noam Postavsky
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=05d6bad8-a739-8fcc-bd32-1291423c85a5@gmail.com \
--to=serg.foo@gmail.com \
--cc=32479@debbugs.gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.