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

[Pip Cet <pipcet@gmail.com>]

On Mon, Mar 8, 2021 at 2:39 PM Eli Zaretskii <eliz@gnu.org> wrote:
> > From: Pip Cet <pipcet@gmail.com>
> > Date: Mon, 8 Mar 2021 13:52:49 +0000
> > Cc: Andrea Corallo <akrl@sdf.org>, 46256@debbugs.gnu.org, andrewjmoreton@gmail.com
> >
> > > > Wait, I thought this was on Windows?
> > >
> > > Yes, and...?
> >
> > No dlclose() on Windows.
>
> Why does this matter in this case?  (And I do have dlclose in a
> standard library that comes with MinGW, btw.  Not that it's relevant.)

We don't use dlclose() on Windows. FreeLibrary() is documented not to
unload the library in certain cases, and to return a failure code.

> > FreeLibrary() is documented to behave differently from dlclose()
>
> It is?  In what way?

Libraries can be pinned, and it can fail without a clear list of
potential failure reasons in the documentation. Not that dlclose() is
better according to the documentation, but there, the source is
available...

> > and I don't have a Windows system to test whether it's actually
> > different in practice.
>
> Well, how about explaining the details in terms that are simple enough
> that I could understand and do the testing?

Excellent idea. I'll try!

> Until now, you and Andrea
> have been talking Chinese as far as I'm concerned.  Please be aware
> that I don't know half the details you two do about native-comp
> internals, and will never be able to know that: too many other things
> on my plate and too little time.  Can you perhaps explain the issue
> without alluding to CU_1, CU_2, Fnative_elisp_load etc., and without
> assuming that their interactions are common knowledge?

Native-comp uses a pseudo-vector type representing a dlopen()ed handle.

In addition to the handle being stored in the pseudo-vector, a pointer
to the pseudo-vector is stored in the data space belonging to the
handle. I'll refer to that as the "reverse pointer" because I can't
think of a better term right now.

When we cleanup the pseudo-vector, we don't reset the reverse pointer
to NULL, or Qnil.

That is because we assume that the dlclose() we perform on cleanup
will unmap the data space belonging to the handle, anyway.

That assumption is wrong in certain specific circumstances.

In those circumstances, the reverse pointer is dereferenced after the
vector has been deallocated. It points to a random different vector
now.

> > > Why would any of the above assumptions be violated?
> >
> > I have several suspicions, including "because the second compilation
> > unit referring to the same handle hasn't been collected". Because that
> > is definitely a bug, and one we should fix, and then we can debug this
> > issue more if and when it reappears.
>
> More Chinese.

(Upon rereading, I agree. My bad.)

One of the circumstances in which the assumption (that the reverse
pointer won't be used) becomes invalid is when two pseudo-vectors
share a handle (and, thus, a reverse pointer). But the reverse pointer
can only point to one of them, and it might be the wrong one.

My patch, thus, resets the reverse pointer to Qnil when cleanup is
performed. In addition, it does so only if the reverse pointer
actually pointed to the pseudo-vector being cleaned-up, rather than to
a different one, to handle a corner case in the code.

Pip