master 9f01ce6: Make checking for liveness of global values more precise.

[Philipp Stephani]

branch: master
commit 9f01ce6327af886f26399924a9aadf16cdd4fd9f
Author: Philipp Stephani <phst@google.com>
Commit: Philipp Stephani <phst@google.com>

    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.
    
    * test/data/emacs-module/mod-test.c (Fmod_test_globref_invalid_free):
    New test module function.
    (emacs_module_init): Export it.
    
    * test/src/emacs-module-tests.el
    (module--test-assertions--globref-invalid-free): New unit test.
---
 src/emacs-module.c                | 13 ++++++++-----
 test/data/emacs-module/mod-test.c | 14 ++++++++++++++
 test/src/emacs-module-tests.el    | 11 +++++++++++
 3 files changed, 33 insertions(+), 5 deletions(-)

diff --git a/src/emacs-module.c b/src/emacs-module.c
index 02563a4..e4e7da0 100644
--- a/src/emacs-module.c
+++ b/src/emacs-module.c
@@ -468,6 +468,14 @@ module_free_global_ref (emacs_env *env, emacs_value 
global_value)
   Lisp_Object obj = value_to_lisp (global_value);
   ptrdiff_t i = hash_lookup (h, obj, NULL);
 
+  if (module_assertions)
+    {
+      ptrdiff_t n = 0;
+      if (! module_global_reference_p (global_value, &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);
@@ -476,11 +484,6 @@ module_free_global_ref (emacs_env *env, emacs_value 
global_value)
       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
diff --git a/test/data/emacs-module/mod-test.c 
b/test/data/emacs-module/mod-test.c
index 437faae..ed289d7 100644
--- a/test/data/emacs-module/mod-test.c
+++ b/test/data/emacs-module/mod-test.c
@@ -201,7 +201,19 @@ Fmod_test_globref_free (emacs_env *env, ptrdiff_t nargs, 
emacs_value args[],
   return env->intern (env, "ok");
 }
 
+/* Treat a local reference as global and free it.  Module assertions
+   should detect this case even if a global reference representing the
+   same object also exists.  */
 
+static emacs_value
+Fmod_test_globref_invalid_free (emacs_env *env, ptrdiff_t nargs,
+                                emacs_value *args, void *data)
+{
+  emacs_value local = env->make_integer (env, 9876);
+  env->make_global_ref (env, local);
+  env->free_global_ref (env, local);  /* Not allowed. */
+  return env->intern (env, "nil");
+}
 
 /* Return a copy of the argument string where every 'a' is replaced
    with 'b'.  */
@@ -694,6 +706,8 @@ emacs_module_init (struct emacs_runtime *ert)
         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-globref-invalid-free", Fmod_test_globref_invalid_free, 0, 0,
+         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 7ed9ecf..8465fd0 100644
--- a/test/src/emacs-module-tests.el
+++ b/test/src/emacs-module-tests.el
@@ -301,6 +301,17 @@ during garbage collection."
     (mod-test-invalid-finalizer)
     (garbage-collect)))
 
+(ert-deftest module--test-assertions--globref-invalid-free ()
+  "Check that -module-assertions detects invalid freeing of a
+local reference."
+    (skip-unless (or (file-executable-p mod-test-emacs)
+                   (and (eq system-type 'windows-nt)
+                        (file-executable-p (concat mod-test-emacs ".exe")))))
+  (module--test-assertion
+      (rx "Global value was not found in list of " (+ digit) " globals")
+    (mod-test-globref-invalid-free)
+    (garbage-collect)))
+
 (ert-deftest module/describe-function-1 ()
   "Check that Bug#30163 is fixed."
   (with-temp-buffer