bug#34655: 26.1.92; Segfault in module with --module-assertions

[Basil L. Contovounesios]

gdb.txt
Description: Text document

[CCing Philipp as the author of module assertions.]
       
Eli Zaretskii <eliz@gnu.org> writes:

>> From: "Basil L. Contovounesios" <contovob@tcd.ie>
>> Date: Mon, 25 Feb 2019 21:00:41 +0000
>> 
>> Starting program: /home/blc/.local/src/emacs26/src/emacs -Q 
>> --module-assertions
>> [Thread debugging using libthread_db enabled]
>> Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
>> [New Thread 0x7ffff01cb700 (LWP 8299)]
>> [New Thread 0x7fffef9ac700 (LWP 8300)]
>> [New Thread 0x7fffef1ab700 (LWP 8301)]
>> 
>> Thread 1 "emacs" received signal SIGSEGV, Segmentation fault.
>> re_search_2 (bufp=0xbf5d00 <searchbufs+384>, str1=0x0, size1=0, str2=0x0, 
>> size2=18, startpos=0, 
>>     range=18, regs=0x0, stop=18) at regex.c:4354
>> 4354                         buf_ch = STRING_CHAR_AND_LENGTH (d, 
>> buf_charlen);
>> #0  0x0000000000608594 in re_search_2
>>     (bufp=0xbf5d00 <searchbufs+384>, str1=0x0, size1=0, str2=0x0, size2=18, 
>> startpos=0, range=18, regs=0x0, stop=18) at regex.c:4354
>>         buf_charlen = 0
>>         irange = 18
>>         lim = 0
>>         d = 0x0
>>         buf_ch = 18
>>         val = 691541629
>>         string1 = 0x0
>>         string2 = 0x0
>>         fastmap = 0xbf5d38 <searchbufs+440> ""
>>         translate = make_number(0)
>>         total_size = 18
>>         endpos = 18
>>         anchored_start = 0 '\000'
>>         multibyte = 1 '\001'
>> #1  0x0000000000607f91 in re_search
>>     (bufp=0xbf5d00 <searchbufs+384>, string=0x0, size=18, startpos=0, 
>> range=18, regs=0x0)
>>     at regex.c:4181
>> #2  0x00000000005f3fd0 in fast_string_match_internal
>>     (regexp=XIL(0x8c761c), string=XIL(0x3036ec4), table=XIL(0)) at 
>> search.c:485
>>         val = 140737488336288
>>         bufp = 0xbf5d00 <searchbufs+384>
>
> Here's your problem: fast_string_match_internal got a Lisp
> string=XIL(0x3036ec4), but its data passed to re_search as the 2nd arg
> is a NULL pointer.  You need to find out how this happens, e.g. by
> setting a watchpoint on string's data inside Ffile_name_as_directory.
> Or maybe the string is already corrupted there?

The file name string is already corrupted when Ffile_name_as_directory
is called; see below.

First, I rewrote the dynamic module[1] to add nonlocal exit checks after
almost every call to the module API.

[1]: https://gitlab.com/basil-conto/realpath

I also added the following assertions to src/emacs-module.c:

diff --git a/src/emacs-module.c b/src/emacs-module.c
index 0abfd3f6f1..4f2b0ecd4b 100644
--- a/src/emacs-module.c
+++ b/src/emacs-module.c
@@ -458,6 +458,8 @@ module_make_function (emacs_env *env, ptrdiff_t min_arity, 
ptrdiff_t max_arity,
   return lisp_to_value (env, result);
 }
 
+static bool my_check = false;
+
 static emacs_value
 module_funcall (emacs_env *env, emacs_value fun, ptrdiff_t nargs,
                emacs_value args[])
@@ -473,6 +475,7 @@ module_funcall (emacs_env *env, emacs_value fun, ptrdiff_t 
nargs,
     xsignal0 (Qoverflow_error);
   SAFE_ALLOCA_LISP (newargs, nargs1);
   newargs[0] = value_to_lisp (fun);
+  my_check = EQ (newargs[0], Qfile_name_as_directory);
   for (ptrdiff_t i = 0; i < nargs; i++)
     newargs[1 + i] = value_to_lisp (args[i]);
   emacs_value result = lisp_to_value (env, Ffuncall (nargs1, newargs));
@@ -581,8 +584,10 @@ module_make_string (emacs_env *env, const char *str, 
ptrdiff_t length)
   /* FIXME: AUTO_STRING_WITH_LEN requires STR to be null-terminated,
      but we shouldn't require that.  */
   AUTO_STRING_WITH_LEN (lstr, str, length);
-  return lisp_to_value (env,
-                        code_convert_string_norecord (lstr, Qutf_8, false));
+  Lisp_Object lisp = code_convert_string_norecord (lstr, Qutf_8, false);
+  eassert (STRINGP (lisp) && SSDATA (lisp));
+  my_check = true;
+  return lisp_to_value (env, lisp);
 }
 
 static emacs_value
@@ -955,6 +960,8 @@ value_to_lisp_bits (emacs_value v)
 static Lisp_Object
 value_to_lisp (emacs_value v)
 {
+  bool b = my_check;
+  my_check = false;
   if (module_assertions)
     {
       /* Check the liveness of the value by iterating over all live
@@ -972,7 +979,11 @@ value_to_lisp (emacs_value v)
             {
               Lisp_Object *p = XSAVE_POINTER (XCAR (values), 0);
               if (p == optr)
-                return *p;
+                {
+                  if (b)
+                    eassert (STRINGP (*p) && SSDATA (*p));
+                  return *p;
+                }
               ++num_values;
             }
           ++num_environments;
@@ -1008,6 +1019,8 @@ lisp_to_value_bits (Lisp_Object o)
 static emacs_value
 lisp_to_value (emacs_env *env, Lisp_Object o)
 {
+  bool b = my_check;
+  my_check = false;
   if (module_assertions)
     {
       /* Add the new value to the list of values allocated from this
@@ -1020,6 +1033,11 @@ lisp_to_value (emacs_env *env, Lisp_Object o)
       ATTRIBUTE_MAY_ALIAS emacs_value ret = vptr;
       struct emacs_env_private *priv = env->private_members;
       priv->values = Fcons (make_save_ptr (ret), priv->values);
+      if (b)
+        {
+          eassert (STRINGP (o) && SSDATA (o));
+          eassert (STRINGP (*optr) && SSDATA (*optr));
+        }
       return ret;
     }
 

These reveal that value_to_lisp eventually returns a corrupted string,
but I don't know why.  I've seen comments in src/fileio.c referring to
string-relocation during GC; could this be at play here?  Either way, do
you have any suggestions on how to proceed?

Here's the full recipe again, now with debugging symbols for the module:

0. git clone https://gitlab.com/basil-conto/realpath.git
1. cd realpath
2. make DEBUG=1
3. cd /path/to/emacs/src
4. gdb emacs
5. set logging on
6. r -Q --module-assertions
7. (progn (module-load "/path/to/realpath.so")
          (dotimes (_ 1000)
            (realpath-truename user-emacs-directory)))
   C-j

   [The loop usually trips the assertions on its first run, but if it
    doesn't, just try again once or twice.]

8. bt full
9. f 2
10. p p
11. pr [#<INVALID_LISP_OBJECT 0x03059c90>]
12. xpr

I attach the resulting GDB log.

Thanks,

-- 
Basil