GNU bug report logs - #75754
styled_format stack usage/GC protection

Previous Next

Full log

View this message in rfc822 format

From: Eli Zaretskii <eliz <at> gnu.org>
To: Pip Cet <pipcet <at> protonmail.com>
Cc: eggert <at> cs.ucla.edu, 75754 <at> debbugs.gnu.org
Subject: bug#75754: styled_format stack usage/GC protection
Date: Fri, 24 Jan 2025 10:16:28 +0200

> Date: Thu, 23 Jan 2025 23:58:41 +0000
> From: Pip Cet <pipcet <at> protonmail.com>
> Cc: eggert <at> cs.ucla.edu, 75754 <at> debbugs.gnu.org
> 
> 1. Reload format_start and format (and end, and format0) after every
> call which might have GC'd.  If you think we should do this, please
> tell me whether lisp_string_width can GC.

It can, if called with the last argument 'true' (because
find_automatic_composition calls into Lisp).  Currently, we call it
with 'false', so it cannot.

> More importantly, assuming it doesn't, document this in every
> function in the call tree starting at lisp_string_width so we don't
> accidentally change it.
> 
> 2. memcpy the format string.  Two-liner, more likely to fix the bug for
> good than (1), wastes more memory (since sa_avail has been negative
> since we entered the function, this is xmalloc'd memory).
> 
> 3. replace format by a ptrdiff_t and all instances of *format by
> SREF (args[0], index).  Faster than 2, but many changes hurting
> readability.

I prefer (2), I think.  Assuming it indeed fixes the problem.

> > +  format_start = SSDATA (args[0]);
> >    p = buf;
> >    nchars = 0;
> >  
> >
> > should fix it.
> 
> Just be clear here: that fixes the specific test.  It does not fix the
> bug.

Can we have a test for "the rest" of the bug, which is not fixed by
the above?

> Here's (2), which seems most doable and, I think, may fix the bug,
> unless there's a subtlety I missed.
> 
> diff --git a/src/editfns.c b/src/editfns.c
> index 4ba356d627c..72cbbb51c40 100644
> --- a/src/editfns.c
> +++ b/src/editfns.c
> @@ -3442,9 +3442,10 @@ styled_format (ptrdiff_t nargs, Lisp_Object *args, bool message)
>    } *info;
>  
>    CHECK_STRING (args[0]);
> -  char *format_start = SSDATA (args[0]);
>    bool multibyte_format = STRING_MULTIBYTE (args[0]);
>    ptrdiff_t formatlen = SBYTES (args[0]);
> +  char *format_start = SAFE_ALLOCA (formatlen);
> +  memcpy (format_start, SSDATA (args[0]), formatlen);
>    bool fmt_props = !!string_intervals (args[0]);

Does this assume anything about formatlen, like that it doesn't exceed
the alloca limit?  If so, should we have an assertion about that?

> feature/igc avoids all of these problems by keeping SDATA pointers
> valid, even when the string is resized.

Unless the string is GC'ed, right?

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.