Subject: Re: Missing assertions to detect access to GC-freed strings

From: Pip Cet <[email-masked]>
Date: Thu, 23 Jan 2025 17:42:16 +0000

Pip Cet <[email-masked]> writes:

> Even with this patch, we would fail to recognize impossible strings
> automatically if even a single bit in the data pointer is set.  I don't
> have a better solution yet, though, but the patch (to follow as soon as
> this has a bug number) would have detected this particular problem a
> little sooner.

Patch follows, I've confirmed that it would have detected this problem
very slightly sooner.

>From 5ddd7c3cb24fe1d0b2f53a44500f11db69a09831 Mon Sep 17 00:00:00 2001
From: Pip Cet <[email-masked]>
Subject: [PATCH] Add debug assertions that we never access a free'd string
 (bug#75768)

* src/lisp.h (LIVE_STRING_P): New function.
(SDATA):
(SCHARS):
(STRING_BYTES):
(string_immovable_p): Use it in assertions.
---
 src/lisp.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/src/lisp.h b/src/lisp.h
index 28fa4c8037e..498aa2b98af 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -1617,6 +1617,13 @@ STRINGP (Lisp_Object x)
   return TAGGEDP (x, Lisp_String);
 }
 
+/* /\* Should always return true. */  */
+INLINE bool
+LIVE_STRING_P (Lisp_Object string)
+{
+  return XSTRING (string)->u.s.data != NULL;
+}
+
 INLINE void
 CHECK_STRING (Lisp_Object x)
 {
@@ -1685,6 +1692,7 @@ STRING_SET_MULTIBYTE (Lisp_Object str)
 INLINE unsigned char *
 SDATA (Lisp_Object string)
 {
+  eassert (LIVE_STRING_P (string));
   return XSTRING (string)->u.s.data;
 }
 INLINE char *
@@ -1706,6 +1714,7 @@ SSET (Lisp_Object string, ptrdiff_t index, unsigned char new)
 INLINE ptrdiff_t
 SCHARS (Lisp_Object string)
 {
+  eassert (LIVE_STRING_P (string));
   ptrdiff_t nchars = XSTRING (string)->u.s.size;
   eassume (0 <= nchars);
   return nchars;
@@ -1717,6 +1726,7 @@ SCHARS (Lisp_Object string)
 INLINE ptrdiff_t
 STRING_BYTES (struct Lisp_String *s)
 {
+  eassert (LIVE_STRING_P (make_lisp_ptr (s, Lisp_String)));
 #ifdef GC_CHECK_STRING_BYTES
   ptrdiff_t nbytes = string_bytes (s);
 #else
@@ -1753,6 +1763,7 @@ CHECK_STRING_NULL_BYTES (Lisp_Object string)
 INLINE bool
 string_immovable_p (Lisp_Object str)
 {
+  eassert (LIVE_STRING_P (str));
   return XSTRING (str)->u.s.size_byte == -3;
 }
 
-- 
2.47.1