GNU bug report logs - #41936
28.0.50; AREF: assert that the index is inside bounds

Previous Next

Full log

View this message in rfc822 format

From: help-debbugs <at> gnu.org (GNU bug Tracking System)
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: tracker <at> debbugs.gnu.org
Subject: bug#41936: closed (28.0.50; AREF: assert that the index is inside
 bounds)
Date: Thu, 18 Jun 2020 21:07:01 +0000

[Message part 1 (text/plain, inline)]

Your message dated Thu, 18 Jun 2020 14:06:31 -0700
with message-id <9b502c26-1406-9a38-fb25-177e59fc6388 <at> cs.ucla.edu>
and subject line Re: bug#41936: 28.0.50; AREF: assert that the index is inside bounds
has caused the debbugs.gnu.org bug report #41936,
regarding 28.0.50; AREF: assert that the index is inside bounds
to be marked as done.

(If you believe you have received this mail in error, please contact
help-debbugs <at> gnu.org.)


-- 
41936: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=41936
GNU Bug Tracking System
Contact help-debbugs <at> gnu.org with problems

[Message part 2 (message/rfc822, inline)]

From: Tino Calancha <tino.calancha <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 28.0.50; AREF: assert that the index is inside bounds
Date: Thu, 18 Jun 2020 22:12:20 +0200

Severity: wishlist,patch
X-Debbugs-Cc: Paul Eggert <eggert <at> cs.ucla.edu>, Eli Zaretskii <eliz <at> gnu.org>, <uyennhi.qm <at> gmail.com>


I was bitten by an out-of-bounds index at AREF while working
in a new feature.

A similar assert as we do in ASET would have allowed me
to diagnostic the bug in minutes; instead, it took me
few days to realize the bug.

Is it OK for you to add the following patch?

--8<-----------------------------cut here---------------start------------->8---
commit 8d904d41fcb8ef29ac8205761077a11f900916bc
Author: Tino Calancha <tino.calancha <at> gmail.com>
Date:   Thu Jun 18 22:01:07 2020 +0200

    AREF: assert that the index is inside bounds
    
    * src/lisp.h (gc_asize): Move before first use.
      (AREF): Assert the index is inside its bounds.
    * test/manual/etags/c-src/emacs/src/lisp.h (AREF):
      Same.

diff --git a/src/lisp.h b/src/lisp.h
index 3442699088..21722e4a78 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,6 +1860,7 @@ 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];
 }
 
@@ -1862,13 +1870,6 @@ aref_addr (Lisp_Object array, ptrdiff_t idx)
   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)
 {
diff --git a/test/manual/etags/c-src/emacs/src/lisp.h b/test/manual/etags/c-src/emacs/src/lisp.h
index eceef4c00d..b2e32554c3 100644
--- a/test/manual/etags/c-src/emacs/src/lisp.h
+++ b/test/manual/etags/c-src/emacs/src/lisp.h
@@ -1478,6 +1478,7 @@ enum
 INLINE Lisp_Object
 AREF (Lisp_Object array, ptrdiff_t idx)
 {
+  eassert (0 <= idx && idx < gc_asize (array));
   return XVECTOR (array)->contents[idx];
 }
 

--8<-----------------------------cut here---------------end--------------->8---

In GNU Emacs 28.0.50 (build 3, x86_64-pc-linux-gnu, GTK+ Version 3.24.5, cairo version 1.16.0)
 of 2020-06-18 built on calancha-pc.dy.bbexcite.jp
Repository revision: ba450b6f462e278fcd3bc96c88f154fce219f5fc
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12004000
System Description: Debian GNU/Linux 10 (buster)

[Message part 3 (message/rfc822, inline)]

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Tino Calancha <tino.calancha <at> gmail.com>
Cc: 41936-done <at> debbugs.gnu.org, eli zaretskii <eliz <at> gnu.org>,
 uyennhi.qm <at> gmail.com
Subject: Re: bug#41936: 28.0.50; AREF: assert that the index is inside bounds
Date: Thu, 18 Jun 2020 14:06:31 -0700

[Message part 4 (text/plain, inline)]

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.

[0001-Check-AREF-and-aref_addr-subscripts.patch (text/x-patch, attachment)]

Previous Next

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