Re: master d582356: * src/fns.c (Frandom): Handle bignum `limit`s

[Pip Cet <pipcet@gmail.com>]

On Sun, Mar 7, 2021 at 6:37 PM Stefan Monnier <monnier@iro.umontreal.ca> wrote:
> >> I don't think I understand how will we know which function says it
> >> never calls GC.
> > By tagging it in the source code?
>
> Random thoughts on this:
> - AFAIK in the current code, the places where we can't run GC are much
>   more rare than the cases where we can run GC, so we'd be better off
>   trying to annotate the places where it can't happen.

I think there's one exception: DEFUNs should be assumed to call GC
unless they're explicitly tagged as not doing so, and they should
assert they're allowed to call GC when run (again, unless explicitly
tagged as not requiring GC).

> - Those places are currently not annotated at all, by and large.
>   There are a few comments here and there stating that GC shouldn't
>   happen, but those comments shouldn't be trusted.

Indeed.

> - The trend is to reduce the amount of code where GC cannot take place
>   [ I think and I hope.  ]

There are several analogous problems, IMHO:
1. Code that can't GC
2. Code that can't quit
3. Code that leaves my X session in an unusable state if gdb interrupts it
4. Code that mustn't call Lisp (signal handlers)
5. Code that mustn't call malloc
6. Code that never signals an error

I think (5) is less of an issue now that reentrant libcs are a thing,
and I'll freely admit I don't understand (3) (or the X/Wayland
situation, at all). But my suspicion is critical sections of one kind
or another will keep coming up.

> - As you have noted some functions can be called in contexts where they
>   may GC and in other contexts where they may not GC.  So we don't have
>   a clear static partitioning of the code.  So maybe a dynamic-test
>   approach is the better option (it will rarely catch the unlikely
>   corner cases, but if it does catch them in their rare occurrences it'll
>   still help us diagnose those problems which tend to be very painful
>   to track down).

I'm currently leaning towards a dynamic approach together with running
GCC with -fanalyzer -flto. (IIUC, the analyzer differs from normal
warning passes in that it tries a hell of a lot harder to prove the
code is okay, but if it fails to do so, it outputs a warning rather
than erring on the side of caution. For example,
verify_interval_modification in textprop.c generates a warning; i and
prev are two variables which cannot simultaneously both be NULL (but
either one of them can be), and code dereferences them in the i ==
prev branch of an if. The code's correct, but it's definitely not
obviously correct.) So I hope the analyzer will be able to turn the
dynamic check into a static check given a few starting points...

And as I said, I think DEFUNs are a good starting point, because it's
easy to redefine DEFUN() to define a wrapper around the actual
Fwhatever function instead of the Fwhatever function directly.

But what I would like to know is whether it's potentially worth it to
investigate this further. If there's a strong consensus (or a
maintainer veto implying) that we'd rather keep catching bugs by hand,
well, I'd rather know.

I'd also like to point out that this approach is not cheap. Running
the analyzer on an LTO'd Emacs (compiled with -O0; I don't know
whether that makes it slower or faster) takes 20 GB of RAM and 5
minutes of CPU time, and that's with the configurable parameters tuned
down quite a bit.

Pip