Subject: Re: bug#41936: 28.0.50; AREF: assert that the index is inside bounds
From: Paul Eggert <[email-masked]>
To: Tino Calancha <[email-masked]>
Cc: [email-masked], eli zaretskii <[email-masked]>, [email-masked]
Date: Thu, 18 Jun 2020 14:06:31 -0700
List-Id: <debbugs-submit.debbugs.gnu.org>

On 6/18/20 1:12 PM, Tino Calancha wrote:
> Is it OK for you to add the following patch?

Yes, good idea. I wondered a while ago (to myself) why AREF doesn't check
subscripts when Emacs is configured with --enable-checking. Now that I think
about it more, it's most likely because AREF was a macro and didn't want to
evaluate its index argument multiple times. We don't need to worry about that
any more.

aref_addr should have a similar check (off by one since one can address one past
the end of an array).

There's no need to change test/manual/etags/c-src/emacs/src/lisp.h as that's
just a data file (and changes can be harmful there as they can mess up the tests).

I installed the attached.
From e14eec7cd4a4217a0908a35415610e0fdb8604f0 Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email-masked]>
Date: Thu, 18 Jun 2020 14:01:56 -0700
Subject: [PATCH] Check AREF and aref_addr subscripts

* src/lisp.h (gc_asize): Move before first use.
(AREF, aref_addr): Check subscripts.
Co-authored-by: Tino Calancha <[email-masked]>
---
 src/lisp.h | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/src/lisp.h b/src/lisp.h
index 3442699088..7b4f484b9b 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -1671,6 +1671,13 @@ ASIZE (Lisp_Object array)
   return size;
 }
 
+INLINE ptrdiff_t
+gc_asize (Lisp_Object array)
+{
+  /* Like ASIZE, but also can be used in the garbage collector.  */
+  return XVECTOR (array)->header.size & ~ARRAY_MARK_FLAG;
+}
+
 INLINE ptrdiff_t
 PVSIZE (Lisp_Object pv)
 {
@@ -1853,22 +1860,17 @@ bool_vector_set (Lisp_Object a, EMACS_INT i, bool b)
 INLINE Lisp_Object
 AREF (Lisp_Object array, ptrdiff_t idx)
 {
+  eassert (0 <= idx && idx < gc_asize (array));
   return XVECTOR (array)->contents[idx];
 }
 
 INLINE Lisp_Object *
 aref_addr (Lisp_Object array, ptrdiff_t idx)
 {
+  eassert (0 <= idx && idx <= gc_asize (array));
   return & XVECTOR (array)->contents[idx];
 }
 
-INLINE ptrdiff_t
-gc_asize (Lisp_Object array)
-{
-  /* Like ASIZE, but also can be used in the garbage collector.  */
-  return XVECTOR (array)->header.size & ~ARRAY_MARK_FLAG;
-}
-
 INLINE void
 ASET (Lisp_Object array, ptrdiff_t idx, Lisp_Object val)
 {
-- 
2.17.1