bug#33014: 26.1.50; 27.0.50; Fatal error after re-evaluating a thread's function

[Stefan Monnier]

>> > After thinking about this a bit, I don't really agree with the last
>> > one: the compiler could indeed stop tracking 'vector', but not
>> > XVECTOR (vector)->contents, and we are interested in the latter.
>> If the compiler stops tracking 'vector', and the garbage collector frees
>> it, doesn't that cause XVECTOR (vector)->contents to be overwritten?
> Hmmm... could be.

Indeed, the conservative GC doesn't try to handle "pointers into the
middle of objects", so a pointer to `XVECTOR (vector)->contents` won't
be sufficient to keep `vector` alive.

> From: Gemini Lasswell <gazally@runbox.com>
> Date: Sun, 14 Oct 2018 12:12:04 -0700
> Subject: [PATCH] * src/eval.c (Ffuncall): Make local variable 'fun' volatile
>  (bug#33014)

Shouldn't we do that in exec_byte_code instead (it probably doesn't
matter that much in the end, but I think conceptually that would be the
more correct place)?

E.g. if you change your test to

    (defun eval-tests-33014-redefine ()
      "Remove the Lisp reference to the byte-compiled object."
      (aset (symbol-function #'eval-tests-33014-func) 1 nil)
      (aset (symbol-function #'eval-tests-33014-func) 2 nil))

you won't get a crash but only because these `aset` will fail (bytecode
objects are luckily read-only).  Moving the volatile thingies to
exec_byte_code should let our code work correctly against the above test
even if we changed aset to allow modifying bytecode objects.


        Stefan