[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: MPS: staticpro everything

From: Eli Zaretskii
Subject: Re: MPS: staticpro everything
Date: Thu, 02 May 2024 19:09:34 +0300

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: luangruo@yahoo.com,  eller.helmut@gmail.com,  emacs-devel@gnu.org
> Date: Thu, 02 May 2024 17:37:05 +0200
> Eli Zaretskii <eliz@gnu.org> writes:
> >> bidi_cache is a global variable pointing to a malloc'd array of bidi_it.
> >> So, by itself, nothing is protected. MPS doesn't know anything about
> >> malloc'd memory.
> >
> > That's true.  But the string Po Lu is alluding to is stored in the
> > cache as a Lisp object, not as a 'char *' pointer to the string's
> > data.  So as long as the string is alive (i.e. referenced from other
> > places), it should not be moved, right?
> That's not the case. MPS doesn't know anything about the malloc'd block,
> so it doesn't know anything about what it contains, it will never see
> the char *, and nothing keeps it alive or prevents moving.

I'm confused: what 'char *'?  There's no 'char *' in bidi_it;
instead, there's a Lisp_Object:

  struct bidi_string_data {
    Lisp_Object lstring;        /* Lisp string to reorder, or nil */
    const unsigned char *s;     /* string data, or NULL if reordering buffer */
    ptrdiff_t schars;           /* the number of characters in the string,
                                   excluding the terminating null */
    ptrdiff_t bufpos;           /* buffer position of lstring, or 0 if N/A */
    bool_bf from_disp_str : 1;  /* True means the string comes from a
                                   display property */
    bool_bf unibyte : 1;        /* True means the string is unibyte */

  struct bidi_it {
    struct bidi_string_data string;     /* string to reorder */


  . bidi_it has a sub-struct bidi_string_data
  . bidi_string_data has 2 fields: lstring and s
    - either one or the other of these two can be non-NULL/nil, but
      not both
    - if lstring is non-nil, it is a Lisp string which is being
      iterated by the caller, and is the same as it->string in the
    - if s is non-NULL, it is a C string being iterated by the caller,
      and is the same as it->s in the caller

(For those who forgot or didn't know what "iterating a C string" means
here: the Emacs display can display a C string, defined with the
normal C 'char *' way.  We use this, for example, for displaying
certain parts of the mode line, such as lots_of_dashes[].)

So, if you thought that 'char *s" above can ever point to the data of
lstring, that is not so.  When the caller iterates over a Lisp string,
bidi.c accesses lstring like so (where 'string' is a pointer to
bidi_it->string, i.e. a pointer to 'struct bidi_string_data'):

      else if (STRINGP (string->lstring))
          if (!string->unibyte)
              int len;
              ch = string_char_and_length (SDATA (string->lstring) + bytepos,
              *ch_len = len;
              ch = UNIBYTE_TO_CHAR (SREF (string->lstring, bytepos));
              *ch_len = 1;

IOW, we maintain the string position and access the string using that

(It was you, Gerd, years ago who explained to me why bidi.c cannot use
a 'char *' to the data of a Lisp string, because GC can relocate
string data, rendering the pointer invalid.  That's why the code gets
passed the Lisp string, not a pointer to its data, and that's why it
accesses it like above.)

When I said that the Lisp string "should not be moved", I meant the
string referenced in bidi_it->string.lstring.

The bidi_cache holds instances of 'struct bidi_it', and that includes
the Lisp string in bidi_it->string.lstring.  But since the same Lisp
string is referenced from it->string, where it is on the stack, AFAIU
the string pointed by it->string should not be moved by MPS.  And as
long as it is not moved all its instances in bidi_cache are okay.
Right?  Or did I misunderstand something?
> >> For some reason, this works with the old GC, without the need for a
> >> special mark function, analogous to mark_specpdl, say. Why that is I
> >> have no idea. Something else apparently keeps everything in the cache
> >> alive.
> >
> > That string is not just an arbitrary string, it's a string that the
> > display iterator is iterating.  So its reference is also in
> > it->string, which is on the C stack, and that reference keeps it alive
> > for as long as the string is being processed.  Once we are done
> > iterating the string, the parts of the cache that hold the states
> > related to that iteration are thrown away, because they are no longer
> > needed.
> That would the "other references" I mention above that can have the same
> effect of making things immovable, keeping things alive.

Right.  But I see nothing except those "other references" in this
story.  Or what did I miss?

reply via email to

[Prev in Thread] Current Thread [Next in Thread]