bug#46256: [feature/native-comp] AOT eln files ignored if run from build tree

[Pip Cet <pipcet@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1166 bytes --]

On Mon, Mar 8, 2021 at 5:54 AM Pip Cet <pipcet@gmail.com> wrote:
> Note that this might not always work because of conservative GC.

If it doesn't work, can you simply retry a few times? Eventually there
shouldn't be references to the stale native_comp_unit on the stack.

However, I think I've worked out why dynlib_close doesn't do its job:

Fnative_elisp_load creates a comp unit, but, if the shared library has
already been initialized, it doesn't set that comp unit's comp_unit
variable to point to the new comp unit; instead, it will continue
pointing to the first comp unit which still has it open.

Then, the original comp unit is unloaded but not the new one created
by Fnative_elisp_load. We call dynlib_close() once, but we called it
twice before, leaving the shared library open and initialized.

Then, we try to load the comp unit again, and follow the stale
comp_unit variable pointing to the original comp unit.

Fix should be as attached. Note the fix is, at worst, harmless (unless
I messed up), so we should apply it anyway just because it's good not
to leave stale pointers lying around even if we hope that the OS
unmaps them at some point.

Pip

[-- Attachment #2: 0001-Fix-stale-pointers-in-comp-units-causing-crashes-bug.patch --]
[-- Type: text/x-patch, Size: 2001 bytes --]

From a2ab9701e48e5443809664d50c924b9d83062b4e Mon Sep 17 00:00:00 2001
From: Pip Cet <pipcet@gmail.com>
Date: Sun, 7 Mar 2021 21:26:29 +0000
Subject: [PATCH] Fix stale pointers in comp units causing crashes  (bug#46256)

* src/alloc.c (cleanup_vector): Call unload_comp_unit.
* src/comp.c (unload_comp_unit): New function.
---
 src/alloc.c |  3 +--
 src/comp.c  | 14 ++++++++++++++
 src/comp.h  |  2 ++
 3 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/src/alloc.c b/src/alloc.c
index af08336177070..fee8cc08aa483 100644
--- a/src/alloc.c
+++ b/src/alloc.c
@@ -3157,8 +3157,7 @@ cleanup_vector (struct Lisp_Vector *vector)
     {
       struct Lisp_Native_Comp_Unit *cu =
 	PSEUDOVEC_STRUCT (vector, Lisp_Native_Comp_Unit);
-      eassert (cu->handle);
-      dynlib_close (cu->handle);
+      unload_comp_unit (cu);
     }
   else if (NATIVE_COMP_FLAG
 	   && PSEUDOVECTOR_TYPEP (&vector->header, PVEC_SUBR))
diff --git a/src/comp.c b/src/comp.c
index b68adf31d68bd..c9e068b90aa2c 100644
--- a/src/comp.c
+++ b/src/comp.c
@@ -4936,6 +4936,20 @@ load_comp_unit (struct Lisp_Native_Comp_Unit *comp_u, bool loading_dump,
   return res;
 }
 
+void
+unload_comp_unit (struct Lisp_Native_Comp_Unit *cu)
+{
+  if (cu->handle == NULL)
+    return;
+
+  Lisp_Object *saved_cu = dynlib_sym (cu->handle, COMP_UNIT_SYM);
+  Lisp_Object this_cu;
+  XSETNATIVE_COMP_UNIT (this_cu, cu);
+  if (EQ (this_cu, *saved_cu))
+    *saved_cu = Qnil;
+  dynlib_close (cu->handle);
+}
+
 Lisp_Object
 native_function_doc (Lisp_Object function)
 {
diff --git a/src/comp.h b/src/comp.h
index f7d17f398c75d..d01bc17565d7d 100644
--- a/src/comp.h
+++ b/src/comp.h
@@ -78,6 +78,8 @@ XNATIVE_COMP_UNIT (Lisp_Object a)
 extern Lisp_Object load_comp_unit (struct Lisp_Native_Comp_Unit *comp_u,
 				   bool loading_dump, bool late_load);
 
+extern void unload_comp_unit (struct Lisp_Native_Comp_Unit *);
+
 extern Lisp_Object native_function_doc (Lisp_Object function);
 
 extern void syms_of_comp (void);
-- 
2.30.1