add template specializations to avoid DATAPTR use

ChangeLog

@@ -1,3 +1,10 @@
+2024-06-11  Kevin Ushey  <[email protected]>
+
+	* inst/include/Rcpp/internal/r_vector.h: Use template specializations to
+	avoid DATAPTR usage
+	* inst/include/Rcpp/vector/traits.h: Implement bounds checks in
+	r_vector_cache access
+
 2024-06-02  Dirk Eddelbuettel  <[email protected]>
 
 	* inst/include/Rcpp/internal/export.h: More R_xlen_t switching

inst/NEWS.Rd

@@ -9,9 +9,14 @@
     \itemize{
       \item Set R_NO_REMAP if not already defined (Dirk in \ghpr{1296})
       \item Add variadic templates to be used instead of generated code
-      (Andrew Johnson in \ghpr{1303}
+      (Andrew Johnson in \ghpr{1303})
       \item Count variables were switches to \code{size_t} to avoid warnings
       about conversion-narrowing (Dirk in \ghpr{1307})
+      \item Rcpp now avoids the usage of the (non-API) DATAPTR function when
+      accessing the contents of Rcpp Vector objects where possible. (Kevin in
+      \ghpr{1310})
+      \item Rcpp now throws an R error on out-of-bounds Vector accesses. (Kevin
+      in \ghpr{1310})
     }
     \item Changes in Rcpp Deployment:
     \itemize{

inst/include/Rcpp/internal/r_vector.h

@@ -32,6 +32,21 @@ typename Rcpp::traits::storage_type<RTYPE>::type* r_vector_start(SEXP x) {
     return reinterpret_cast<pointer>(dataptr(x));
 }
 
+// add specializations to avoid use of dataptr
+#define RCPP_VECTOR_START_IMPL(__RTYPE__, __ACCESSOR__)                                              \
+    template <>                                                                                      \
+    inline typename Rcpp::traits::storage_type<__RTYPE__>::type* r_vector_start<__RTYPE__>(SEXP x) { \
+        return __ACCESSOR__(x);                                                                      \
+    }
+
+RCPP_VECTOR_START_IMPL(LGLSXP,  LOGICAL);
+RCPP_VECTOR_START_IMPL(INTSXP,  INTEGER);
+RCPP_VECTOR_START_IMPL(RAWSXP,  RAW);
+RCPP_VECTOR_START_IMPL(CPLXSXP, COMPLEX);
+RCPP_VECTOR_START_IMPL(REALSXP, REAL);
+
+#undef RCPP_VECTOR_START_IMPL
+
 /**
  * The value 0 statically casted to the appropriate type for
  * the given SEXP type

inst/include/Rcpp/vector/traits.h

@@ -35,22 +35,36 @@ namespace traits{
 		typedef typename r_vector_const_proxy<RTYPE>::type const_proxy ;
 		typedef typename storage_type<RTYPE>::type storage_type ;
 
-		r_vector_cache() : start(0){} ;
+		r_vector_cache() : start(0), size(0) {} ;
+
 		inline void update( const VECTOR& v ) {
-		    start = ::Rcpp::internal::r_vector_start<RTYPE>(v) ;
+			start = ::Rcpp::internal::r_vector_start<RTYPE>(v) ;
+			size = v.size();
 		}
+
 		inline iterator get() const { return start; }
 		inline const_iterator get_const() const { return start; }
 
-		inline proxy ref() { return *start ;}
-		inline proxy ref(R_xlen_t i) { return start[i] ; }
+		inline proxy ref() { check_index(0); return start[0] ;}
+		inline proxy ref(R_xlen_t i) { check_index(i); return start[i] ; }
+
+		inline proxy ref() const { check_index(0); return start[0] ;}
+		inline proxy ref(R_xlen_t i) const { check_index(i); return start[i] ; }
 
-		inline proxy ref() const { return *start ;}
-		inline proxy ref(R_xlen_t i) const { return start[i] ; }
+	private:
 
-		private:
-			iterator start ;
+		void check_index(R_xlen_t i) const {
+#ifndef RCPP_NO_BOUNDS_CHECK
+			if (i >= size) {
+				stop("subscript out of bounds (index %s >= vector size %s)", i, size);
+			}
+#endif
+		}
+
+		iterator start ;
+		R_xlen_t size ;
 	} ;
+
 	template <int RTYPE, template <class> class StoragePolicy = PreserveStorage>
 	class proxy_cache{
 	public:
@@ -66,17 +80,24 @@ namespace traits{
 			p = const_cast<VECTOR*>(&v) ;
 		}
 		inline iterator get() const { return iterator( proxy(*p, 0 ) ) ;}
-		// inline const_iterator get_const() const { return const_iterator( *p ) ;}
 		inline const_iterator get_const() const { return const_iterator( const_proxy(*p, 0) ) ; }
 
-		inline proxy ref() { return proxy(*p,0) ; }
-		inline proxy ref(R_xlen_t i) { return proxy(*p,i);}
+		inline proxy ref() { check_index(0); return proxy(*p,0) ; }
+		inline proxy ref(R_xlen_t i) { check_index(i); return proxy(*p,i);}
 
-		inline const_proxy ref() const { return const_proxy(*p,0) ; }
-		inline const_proxy ref(R_xlen_t i) const { return const_proxy(*p,i);}
+		inline const_proxy ref() const { check_index(0); return const_proxy(*p,0) ; }
+		inline const_proxy ref(R_xlen_t i) const { check_index(i); return const_proxy(*p,i);}
 
 	private:
 		VECTOR* p ;
+
+		void check_index(R_xlen_t i) const {
+#ifndef RCPP_NO_BOUNDS_CHECK
+			if (i >= p->size()) {
+				stop("subscript out of bounds (index %s >= vector size %s)", i, p->size());
+			}
+#endif
+		}
 	} ;
 
 	// regular types for INTSXP, REALSXP, ...

inst/tinytest/cpp/Vector.cpp

@@ -886,3 +886,15 @@ bool CharacterVector_test_equality_crosspolicy(CharacterVector x, Vector<STRSXP,
 
     return std::equal(x.begin(), x.end(), y.begin());
 }
+
+// [[Rcpp::export]]
+double NumericVector_test_out_of_bounds_read() {
+    NumericVector v(3);
+    return v[5];
+}
+
+// [[Rcpp::export]]
+SEXP CharacterVector_test_out_of_bounds_read() {
+    CharacterVector v(3);
+    return v[5];
+}

inst/tinytest/test_vector.R

@@ -694,3 +694,7 @@ expect_equal(data, data2)
 #    test.CharacterVector_test_equality <- function(){
 expect_true( !CharacterVector_test_equality("foo", "bar") )
 expect_true( !CharacterVector_test_equality_crosspolicy("foo", "bar") )
+
+
+expect_error(NumericVector_test_out_of_bounds_read())
+expect_error(CharacterVector_test_out_of_bounds_read())

tests/tinytest.R

@@ -43,5 +43,5 @@ if (requireNamespace("tinytest", quietly=TRUE)) {
     ## there are several more granular ways to test files in a tinytest directory,
     ## see its package vignette; tests can also run once the package is installed
     ## using the same command `test_package(pkgName)`, or by director or file
-    tinytest::test_package("Rcpp", ncpu=getOption("Ncpus", 1))
+    tinytest::test_package("Rcpp", ncpu=getOption("Ncpus"))
 }