Merge branch 'api-push-buffer-object-typing'

src/duk_api_public.h.in

@@ -443,7 +443,7 @@ DUK_EXTERNAL_DECL void *duk_push_buffer_raw(duk_context *ctx, duk_size_t size, d
 #define DUK_BUFOBJ_FLOAT32ARRAY        (11 | DUK_BUFOBJ_CREATE_ARRBUF)
 #define DUK_BUFOBJ_FLOAT64ARRAY        (12 | DUK_BUFOBJ_CREATE_ARRBUF)
 
-DUK_EXTERNAL_DECL void duk_push_buffer_object(duk_context *ctx, duk_idx_t idx_buffer, duk_uint_t byte_offset, duk_uint_t byte_length, duk_uint_t flags);
+DUK_EXTERNAL_DECL void duk_push_buffer_object(duk_context *ctx, duk_idx_t idx_buffer, duk_size_t byte_offset, duk_size_t byte_length, duk_uint_t flags);
 
 DUK_EXTERNAL_DECL duk_idx_t duk_push_heapptr(duk_context *ctx, void *ptr);
 

src/duk_api_stack.c

@@ -3858,7 +3858,7 @@ static const duk_uint32_t duk__bufobj_flags_lookup[] = {
 };
 #undef DUK__PACK_ARGS
 
-DUK_EXTERNAL void duk_push_buffer_object(duk_context *ctx, duk_idx_t idx_buffer, duk_uint_t byte_offset, duk_uint_t byte_length, duk_uint_t flags) {
+DUK_EXTERNAL void duk_push_buffer_object(duk_context *ctx, duk_idx_t idx_buffer, duk_size_t byte_offset, duk_size_t byte_length, duk_uint_t flags) {
 	duk_hthread *thr;
 	duk_hbufferobject *h_bufobj;
 	duk_hbuffer *h_val;
@@ -3866,16 +3866,33 @@ DUK_EXTERNAL void duk_push_buffer_object(duk_context *ctx, duk_idx_t idx_buffer,
 	duk_uint_t classnum;
 	duk_uint_t protobidx;
 	duk_uint_t lookupidx;
+	duk_uint_t uint_offset, uint_length, uint_added;
 
 	DUK_ASSERT_CTX_VALID(ctx);
 	thr = (duk_hthread *) ctx;
 	DUK_UNREF(thr);
 
+	/* The underlying types for offset/length in duk_hbufferobject is
+	 * duk_uint_t; make sure argument values fit and that offset + length
+	 * does not wrap.
+	 */
+	uint_offset = (duk_uint_t) byte_offset;
+	uint_length = (duk_uint_t) byte_length;
+	if (sizeof(duk_size_t) != sizeof(duk_uint_t)) {
+		if ((duk_size_t) uint_offset != byte_offset || (duk_size_t) uint_length != byte_length) {
+			goto range_error;
+		}
+	}
+	uint_added = uint_offset + uint_length;
+	if (uint_added < uint_offset) {
+		goto range_error;
+	}
+	DUK_ASSERT(uint_added >= uint_offset && uint_added >= uint_length);
+
 	DUK_ASSERT_DISABLE(flags >= 0);  /* flags is unsigned */
 	lookupidx = flags & 0x0f;  /* 4 low bits */
 	if (lookupidx >= sizeof(duk__bufobj_flags_lookup) / sizeof(duk_uint32_t)) {
-		DUK_ERROR(thr, DUK_ERR_TYPE_ERROR, DUK_STR_INVALID_CALL_ARGS);
-		return;  /* not reached */
+		goto arg_error;
 	}
 	tmp = duk__bufobj_flags_lookup[lookupidx];
 	classnum = tmp >> 24;
@@ -3893,11 +3910,12 @@ DUK_EXTERNAL void duk_push_buffer_object(duk_context *ctx, duk_idx_t idx_buffer,
 
 	h_bufobj->buf = h_val;
 	DUK_HBUFFER_INCREF(thr, h_val);
-	h_bufobj->offset = byte_offset;
-	h_bufobj->length = byte_length;
+	h_bufobj->offset = uint_offset;
+	h_bufobj->length = uint_length;
 	h_bufobj->shift = (tmp >> 4) & 0x0f;
 	h_bufobj->elem_type = (tmp >> 8) & 0xff;
 	h_bufobj->is_view = tmp & 0x0f;
+	DUK_ASSERT_HBUFFEROBJECT_VALID(h_bufobj);
 
 	/* TypedArray views need an automatic ArrayBuffer which must be
 	 * provided as .buffer property of the view.  Just create a new
@@ -3914,16 +3932,25 @@ DUK_EXTERNAL void duk_push_buffer_object(duk_context *ctx, duk_idx_t idx_buffer,
 
 		h_bufobj->buf = h_val;
 		DUK_HBUFFER_INCREF(thr, h_val);
-		h_bufobj->offset = byte_offset;
-		h_bufobj->length = byte_length;
+		h_bufobj->offset = uint_offset;
+		h_bufobj->length = uint_length;
 		DUK_ASSERT(h_bufobj->shift == 0);
 		h_bufobj->elem_type = DUK_HBUFFEROBJECT_ELEM_UINT8;
 		DUK_ASSERT(h_bufobj->is_view == 0);
+		DUK_ASSERT_HBUFFEROBJECT_VALID(h_bufobj);
 
 		duk_xdef_prop_stridx(ctx, -2, DUK_STRIDX_LC_BUFFER, DUK_PROPDESC_FLAGS_NONE);
 		duk_compact(ctx, -1);
 	}
+	return;
+
+ range_error:
+	DUK_ERROR(thr, DUK_ERR_RANGE_ERROR, DUK_STR_INVALID_CALL_ARGS);
+	return;  /* not reached */
 
+ arg_error:
+	DUK_ERROR(thr, DUK_ERR_TYPE_ERROR, DUK_STR_INVALID_CALL_ARGS);
+	return;  /* not reached */
 }
 
 DUK_EXTERNAL duk_idx_t duk_push_error_object_va_raw(duk_context *ctx, duk_errcode_t err_code, const char *filename, duk_int_t line, const char *fmt, va_list ap) {

src/duk_hbuffer_alloc.c

@@ -54,7 +54,9 @@ DUK_INTERNAL duk_hbuffer *duk_hbuffer_alloc(duk_heap *heap, duk_size_t size, duk
 #endif
 
 	if (flags & DUK_BUF_FLAG_EXTERNAL) {
-		duk_hbuffer_external *h = (duk_hbuffer_external *) res;
+		duk_hbuffer_external *h;
+		h = (duk_hbuffer_external *) res;
+		DUK_UNREF(h);
 		*out_bufdata = NULL;
 #if defined(DUK_USE_EXPLICIT_NULL_INIT)
 #if defined(DUK_USE_HEAPPTR16)

src/duk_hbufferobject.h

@@ -38,8 +38,9 @@
 		} else { \
 			/* No assertions for offset or length; in particular, \
 			 * it's OK for length to be longer than underlying \
-			 * buffer. \
+			 * buffer.  Just ensure they don't wrap when added. \
 			 */ \
+			DUK_ASSERT((h)->offset + (h)->length >= (h)->offset); \
 		} \
 	} while (0)
 

tests/api/test-push-buffer-object.c

@@ -398,6 +398,68 @@ static duk_ret_t test_invalid_flags3(duk_context *ctx) {
 	return 0;
 }
 
+/*===
+*** test_invalid_offlen_wrap1 (duk_safe_call)
+==> rc=1, result='RangeError: invalid call args'
+*** test_invalid_offlen_wrap2 (duk_safe_call)
+==> rc=1, result='RangeError: invalid call args'
+===*/
+
+/* Byte offset + byte length wrap. */
+static duk_ret_t test_invalid_offlen_wrap1(duk_context *ctx) {
+	duk_push_fixed_buffer(ctx, 256);
+	duk_push_buffer_object(ctx,
+	                       -1,
+	                       (duk_size_t) (duk_uint_t) -100,
+	                       1000,
+                               DUK_BUFOBJ_UINT8ARRAY);
+	printf("final top: %ld\n", (long) duk_get_top(ctx));
+	return 0;
+}
+
+/* Byte offset + byte length wrap, just barely */
+static duk_ret_t test_invalid_offlen_wrap2(duk_context *ctx) {
+	duk_push_fixed_buffer(ctx, 256);
+	duk_push_buffer_object(ctx,
+	                       -1,
+	                       (duk_size_t) (duk_uint_t) -100,
+	                       100,
+                               DUK_BUFOBJ_UINT8ARRAY);
+	printf("final top: %ld\n", (long) duk_get_top(ctx));
+	return 0;
+}
+
+/*===
+*** test_allowed_offlen_nowrap1 (duk_safe_call)
+object [object Uint8Array] 99 4294967196 99 1 object
+false false false false false true false false false false false false false -> Uint8Array
+false false false false false true false false false false false false false -> Uint8Array.prototype
+final top: 1
+==> rc=0, result='undefined'
+===*/
+
+/* Byte offset + byte length are just within limits of duk_uint_t and don't
+ * wrap.  This works and doesn't cause a ~4G allocation because the conceptual
+ * size (~4G) is unrelated to the underlying buffer size (256 bytes here).
+ */
+static duk_ret_t test_allowed_offlen_nowrap1(duk_context *ctx) {
+	duk_push_fixed_buffer(ctx, 256);
+	duk_push_buffer_object(ctx,
+	                       -1,
+	                       (duk_size_t) (duk_uint_t) -100,
+	                       99,
+                               DUK_BUFOBJ_UINT8ARRAY);
+	duk_eval_string(ctx, "dumpBufferInfo");
+	duk_dup(ctx, -2);
+	duk_call(ctx, 1);
+	duk_pop(ctx);
+
+	duk_pop(ctx);
+
+	printf("final top: %ld\n", (long) duk_get_top(ctx));
+	return 0;
+}
+
 /*
  *  Main
  */
@@ -418,5 +480,11 @@ void test(duk_context *ctx) {
 	TEST_SAFE_CALL(test_invalid_flags1);
 	TEST_SAFE_CALL(test_invalid_flags2);
 	TEST_SAFE_CALL(test_invalid_flags3);
-	/* XXX: could test for more errors */
+	TEST_SAFE_CALL(test_invalid_offlen_wrap1);
+	TEST_SAFE_CALL(test_invalid_offlen_wrap2);
+	TEST_SAFE_CALL(test_allowed_offlen_nowrap1);
+
+	/* XXX: testing for duk_size_t wrapping a duk_uint_t is only possible
+	 * if duk_size_t is a larger type.
+	 */
 }

website/api/duk_push_buffer_object.yaml

@@ -1,7 +1,7 @@
 name: duk_push_buffer
 
 proto: |
-  void duk_push_buffer_object(duk_context *ctx, duk_idx_t idx_buffer, duk_uint_t byte_offset, duk_uint_t byte_length, duk_uint_t flags);
+  void duk_push_buffer_object(duk_context *ctx, duk_idx_t idx_buffer, duk_size_t byte_offset, duk_size_t byte_length, duk_uint_t flags);
 
 stack: |
   [ ... buffer! ... ] -> [ ... buffer! ... bufobj! ]
@@ -31,13 +31,18 @@ summary: |
   <tr><td>DUK_BUFOBJ_FLOAT64ARRAY</td><td>Float64Array</td></tr>
   </table>
 
-  <p>The underlying plain buffer should cover the range indicated by the
-  <code>byte_offset</code> and <code>byte_length</code> arguments.  Even
-  if that is not the case, memory safety is guaranteed; e.g. attempt to
-  read values outside the underlying buffer will return zero.  The underlying
-  buffer size is intentionally not checked when creating a buffer object:
-  even if the buffer fully covered the byte range during creation, it might
-  be resized later.</p>
+  <p>When <code>DataView</code> or any of the typed array views
+  (<code>Uint8Array</code>, etc) is pushed, an <code>ArrayBuffer</code>
+  backing the view is automatically created.  It is accessible through
+  the <code>buffer</code> property of the view object.</p>
+
+  <p>The underlying plain buffer should normally cover the range indicated by
+  the <code>byte_offset</code> and <code>byte_length</code> arguments, but
+  memory safety is guaranteed even if that is not the case.  For example, an
+  attempt to read values outside the underlying buffer will return zero.  The
+  underlying buffer size is intentionally not checked when creating a buffer
+  object: even if the buffer fully covered the byte range during creation, it
+  might be resized later.</p>
 
 example: |
   /* Map byte range [100,150[ of plain buffer at idx_plain_buf into a