Merge pull request #1536 from svaarala/fix-check-require-stack-top

Fix duk_{check,require}_stack_top() reserve calculation

RELEASES.rst

@@ -2885,6 +2885,11 @@ Planned
   calls to bound functions with a lot of bound arguments to fail with a value
   stack limit error (GH-1504)
 
+* Fix duk_check_stack_top() and duk_require_stack_top() internal value stack
+  reserve calculation which failed to take into account call stack entries
+  below the current call, leading potentially to a smaller reserve than
+  requested (GH-1536)
+
 * Fix duk_hbufobj assert in shared slice() handling (GH-1506)
 
 * Fix internal debug print opcode name list which was out of sync with previous

src-input/duk_api_stack.c

@@ -859,9 +859,11 @@ DUK_EXTERNAL void duk_require_stack(duk_context *ctx, duk_idx_t extra) {
 }
 
 DUK_EXTERNAL duk_bool_t duk_check_stack_top(duk_context *ctx, duk_idx_t top) {
+	duk_hthread *thr;
 	duk_size_t min_new_size;
 
 	DUK_ASSERT_CTX_VALID(ctx);
+	thr = (duk_hthread *) ctx;
 
 	if (DUK_UNLIKELY(top < 0)) {
 		/* Clamping to zero makes the API more robust to calling code
@@ -870,7 +872,7 @@ DUK_EXTERNAL duk_bool_t duk_check_stack_top(duk_context *ctx, duk_idx_t top) {
 		top = 0;
 	}
 
-	min_new_size = top + DUK_VALSTACK_INTERNAL_EXTRA;
+	min_new_size = (thr->valstack_bottom - thr->valstack) + top + DUK_VALSTACK_INTERNAL_EXTRA;
 	return duk_valstack_resize_raw(ctx,
 	                               min_new_size,  /* min_new_size */
 	                               0 /* no shrink */ |   /* flags */
@@ -879,9 +881,11 @@ DUK_EXTERNAL duk_bool_t duk_check_stack_top(duk_context *ctx, duk_idx_t top) {
 }
 
 DUK_EXTERNAL void duk_require_stack_top(duk_context *ctx, duk_idx_t top) {
+	duk_hthread *thr;
 	duk_size_t min_new_size;
 
 	DUK_ASSERT_CTX_VALID(ctx);
+	thr = (duk_hthread *) ctx;
 
 	if (DUK_UNLIKELY(top < 0)) {
 		/* Clamping to zero makes the API more robust to calling code
@@ -890,7 +894,7 @@ DUK_EXTERNAL void duk_require_stack_top(duk_context *ctx, duk_idx_t top) {
 		top = 0;
 	}
 
-	min_new_size = top + DUK_VALSTACK_INTERNAL_EXTRA;
+	min_new_size = (thr->valstack_bottom - thr->valstack) + top + DUK_VALSTACK_INTERNAL_EXTRA;
 	(void) duk_valstack_resize_raw(ctx,
 	                               min_new_size,  /* min_new_size */
 	                               0 /* no shrink */ |   /* flags */

tests/api/test-check-require-stack-top.c

@@ -39,12 +39,10 @@ static duk_ret_t test_1(duk_context *ctx, void *udata) {
 /* demonstrate how using one large duk_check_stack_top() before such
  * a loop works.
  */
-static duk_ret_t check_1(duk_context *ctx, void *udata) {
+static duk_ret_t check_1_inner(duk_context *ctx) {
 	int i;
 	duk_ret_t rc;
 
-	(void) udata;
-
 	rc = duk_check_stack_top(ctx, 1000);
 	printf("rc=%d\n", (int) rc);
 
@@ -55,6 +53,22 @@ static duk_ret_t check_1(duk_context *ctx, void *udata) {
 	printf("final top: %ld\n", (long) duk_get_top(ctx));
 	return 0;
 }
+static duk_ret_t check_1(duk_context *ctx, void *udata) {
+	int i;
+
+	(void) udata;
+
+	/* dummy filler */
+	for (i = 0; i < 10000; i++) {
+		duk_require_stack(ctx, 1);
+		duk_push_uint(ctx, 123);
+	}
+
+	duk_push_c_function(ctx, check_1_inner, 0);
+	duk_call(ctx, 0);
+
+	return 0;
+}
 
 /* same test but with one element checks, once per loop */
 static duk_ret_t check_2(duk_context *ctx, void *udata) {
@@ -89,11 +103,9 @@ static duk_ret_t check_3(duk_context *ctx, void *udata) {
 }
 
 /* same as check_1 but with duk_require_stack_top() */
-static duk_ret_t require_1(duk_context *ctx, void *udata) {
+static duk_ret_t require_1_inner(duk_context *ctx) {
 	int i;
 
-	(void) udata;
-
 	duk_require_stack_top(ctx, 1000);
 
 	for (i = 0; i < 1000; i++) {
@@ -103,6 +115,23 @@ static duk_ret_t require_1(duk_context *ctx, void *udata) {
 	printf("final top: %ld\n", (long) duk_get_top(ctx));
 	return 0;
 }
+static duk_ret_t require_1(duk_context *ctx, void *udata) {
+	int i;
+	duk_ret_t rc;
+
+	(void) udata;
+
+	/* dummy filler */
+	for (i = 0; i < 10000; i++) {
+		duk_require_stack(ctx, 1);
+		duk_push_uint(ctx, 123);
+	}
+
+	duk_push_c_function(ctx, require_1_inner, 0);
+	duk_call(ctx, 0);
+
+	return 0;
+}
 
 /* same as check_2 but with duk_require_stack_top() */
 static duk_ret_t require_2(duk_context *ctx, void *udata) {

tests/api/test-check-require-stack.c

@@ -39,12 +39,10 @@ static duk_ret_t test_1(duk_context *ctx, void *udata) {
 /* demonstrate how using one large duk_check_stack() before such
  * a loop works.
  */
-static duk_ret_t check_1(duk_context *ctx, void *udata) {
+static duk_ret_t check_1_inner(duk_context *ctx) {
 	int i;
 	duk_ret_t rc;
 
-	(void) udata;
-
 	rc = duk_check_stack(ctx, 1000);
 	printf("rc=%d\n", (int) rc);
 
@@ -55,6 +53,22 @@ static duk_ret_t check_1(duk_context *ctx, void *udata) {
 	printf("final top: %ld\n", (long) duk_get_top(ctx));
 	return 0;
 }
+static duk_ret_t check_1(duk_context *ctx, void *udata) {
+	int i;
+
+	(void) udata;
+
+	/* dummy filler */
+	for (i = 0; i < 10000; i++) {
+		duk_require_stack(ctx, 1);
+		duk_push_uint(ctx, 123);
+	}
+
+	duk_push_c_function(ctx, check_1_inner, 0);
+	duk_call(ctx, 0);
+
+	return 0;
+}
 
 /* same test but with one element checks, once per loop */
 static duk_ret_t check_2(duk_context *ctx, void *udata) {
@@ -89,11 +103,9 @@ static duk_ret_t check_3(duk_context *ctx, void *udata) {
 }
 
 /* same as check_1 but with duk_require_stack() */
-static duk_ret_t require_1(duk_context *ctx, void *udata) {
+static duk_ret_t require_1_inner(duk_context *ctx) {
 	int i;
 
-	(void) udata;
-
 	duk_require_stack(ctx, 1000);
 
 	for (i = 0; i < 1000; i++) {
@@ -103,6 +115,22 @@ static duk_ret_t require_1(duk_context *ctx, void *udata) {
 	printf("final top: %ld\n", (long) duk_get_top(ctx));
 	return 0;
 }
+static duk_ret_t require_1(duk_context *ctx, void *udata) {
+	int i;
+
+	(void) udata;
+
+	/* dummy filler */
+	for (i = 0; i < 10000; i++) {
+		duk_require_stack(ctx, 1);
+		duk_push_uint(ctx, 123);
+	}
+
+	duk_push_c_function(ctx, require_1_inner, 0);
+	duk_call(ctx, 0);
+
+	return 0;
+}
 
 /* same as check_2 but with duk_require_stack() */
 static duk_ret_t require_2(duk_context *ctx, void *udata) {