[Tarantool-patches] [PATCH] test/uint: fiber

[Tarantool-patches] [PATCH] test/uint: fiber

[Cyrill Gorcunov]

When optimization level increased we've found that
memset calls might be optimized, moreover there
were no check if calls themselves would override
the stack.

Lets rework and use number of calls instead. Strictly
speaking the test is still a bit fragile and precise
testing would rather require asm level code.

Repored-by: "Alexander V. Tikhonov" <avtikhon@tarantool.org>
Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---

branch gorcunov/unit-fiber

 test/unit/fiber.cc | 52 ++++++++++++++++++++++++++++++++++++----------
 1 file changed, 41 insertions(+), 11 deletions(-)

diff --git a/test/unit/fiber.cc b/test/unit/fiber.cc
index b0dfc9b14..7e9416bbb 100644
--- a/test/unit/fiber.cc
+++ b/test/unit/fiber.cc
@@ -1,10 +1,16 @@
+#include <unistd.h>
+
 #include "memory.h"
 #include "fiber.h"
 #include "unit.h"
 #include "trivia/util.h"
 
+static size_t stack_expand_limit;
 static struct fiber_attr default_attr;
 
+static unsigned long page_size;
+#define PAGE_4K 4096
+
 static int
 noop_f(va_list ap)
 {
@@ -49,24 +55,41 @@ cancel_dead_f(va_list ap)
 	return 0;
 }
 
-static size_t stack_expand_limit;
-
 static void NOINLINE
-stack_expand(void *ptr)
+stack_expand(unsigned long *ret, unsigned long nr_calls)
 {
-	char buf[2048];
-	memset(buf, 0x45, 2048);
-	ptrdiff_t stack_diff = (buf - (char *)ptr);
-	stack_diff = stack_diff >= 0 ? stack_diff : -stack_diff;
-	if (stack_diff < (ptrdiff_t)stack_expand_limit)
-		stack_expand(ptr);
+	char volatile fill[PAGE_4K];
+	char volatile *p;
+
+	memset((void *)fill, (unsigned char)nr_calls, sizeof(fill));
+	p = fill;
+	p[PAGE_4K / 2] = (unsigned char)nr_calls;
+
+	if (nr_calls != 0) {
+		stack_expand(ret, nr_calls-1);
+	} else {
+		*ret = (unsigned long)&fill[0];
+	}
 }
 
 static int
 test_stack_f(va_list ap)
 {
-	char s;
-	stack_expand(&s);
+	unsigned long ret = 0;
+	unsigned long ret_addr = (unsigned long)&ret;
+
+	/*
+	 * We can't just dirty the stack in precise
+	 * way without using assembly. Thus lets do
+	 * the following trick:
+	 *  - assume 8K will be enough to carry all
+	 *    arguments passed for all calls, still
+	 *    we might need to adjust this value
+	 */
+	stack_expand(&ret, (stack_expand_limit - 2 * page_size) / page_size);
+	note("ret address %#lx last address %#lx covered %lu",
+	     ret_addr, ret, ret_addr > ret ?
+	     ret_addr - ret : ret - ret_addr);
 	return 0;
 }
 
@@ -139,6 +162,7 @@ fiber_stack_test()
 	 * Test a fiber with the default stack size.
 	 */
 	stack_expand_limit = default_attr.stack_size * 3 / 4;
+	note("stack_expand_limit %lu", stack_expand_limit);
 	fiber = fiber_new_xc("test_stack", test_stack_f);
 	fiber_wakeup(fiber);
 	fiber_sleep(0);
@@ -150,6 +174,7 @@ fiber_stack_test()
 	fiber_attr = fiber_attr_new();
 	fiber_attr_setstacksize(fiber_attr, default_attr.stack_size * 2);
 	stack_expand_limit = default_attr.stack_size * 3 / 2;
+	note("stack_expand_limit %lu", stack_expand_limit);
 	fiber = fiber_new_ex("test_stack", fiber_attr, test_stack_f);
 	fiber_attr_delete(fiber_attr);
 	if (fiber == NULL)
@@ -193,6 +218,11 @@ main_f(va_list ap)
 
 int main()
 {
+	page_size = sysconf(_SC_PAGESIZE);
+
+	/* Page should be at least 4K */
+	assert(page_size >= PAGE_4K);
+
 	memory_init();
 	fiber_init(fiber_cxx_invoke);
 	fiber_attr_create(&default_attr);
-- 
2.26.2

Re: [Tarantool-patches] [PATCH] test/uint: fiber

[Cyrill Gorcunov]

On Thu, Oct 15, 2020 at 05:28:27PM +0300, Cyrill Gorcunov wrote:
> When optimization level increased we've found that
> memset calls might be optimized, moreover there
> were no check if calls themselves would override
> the stack.
> 
> Lets rework and use number of calls instead. Strictly
> speaking the test is still a bit fragile and precise
> testing would rather require asm level code.
> 
> Repored-by: "Alexander V. Tikhonov" <avtikhon@tarantool.org>
> Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
> ---
> 
> branch gorcunov/unit-fiber

I've force update the patch to match results file.
---
From: Cyrill Gorcunov <gorcunov@gmail.com>
Date: Thu, 15 Oct 2020 17:25:55 +0300
Subject: [PATCH] test/uint: fiber

When optimization level increased we've found that
memset calls might be optimized, moreover there
were no check if calls themselves would override
the stack.

Lets rework and use number of calls instead. Strictly
speaking the test is still a bit fragile and precise
testing would rather require asm level code.

Repored-by: "Alexander V. Tikhonov" <avtikhon@tarantool.org>
Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
 test/unit/fiber.cc | 47 +++++++++++++++++++++++++++++++++++-----------
 1 file changed, 36 insertions(+), 11 deletions(-)

diff --git a/test/unit/fiber.cc b/test/unit/fiber.cc
index b0dfc9b14..9c1a23bdd 100644
--- a/test/unit/fiber.cc
+++ b/test/unit/fiber.cc
@@ -1,10 +1,16 @@
+#include <unistd.h>
+
 #include "memory.h"
 #include "fiber.h"
 #include "unit.h"
 #include "trivia/util.h"
 
+static size_t stack_expand_limit;
 static struct fiber_attr default_attr;
 
+static unsigned long page_size;
+#define PAGE_4K 4096
+
 static int
 noop_f(va_list ap)
 {
@@ -49,24 +55,38 @@ cancel_dead_f(va_list ap)
 	return 0;
 }
 
-static size_t stack_expand_limit;
-
 static void NOINLINE
-stack_expand(void *ptr)
+stack_expand(unsigned long *ret, unsigned long nr_calls)
 {
-	char buf[2048];
-	memset(buf, 0x45, 2048);
-	ptrdiff_t stack_diff = (buf - (char *)ptr);
-	stack_diff = stack_diff >= 0 ? stack_diff : -stack_diff;
-	if (stack_diff < (ptrdiff_t)stack_expand_limit)
-		stack_expand(ptr);
+	char volatile fill[PAGE_4K];
+	char volatile *p;
+
+	memset((void *)fill, (unsigned char)nr_calls, sizeof(fill));
+	p = fill;
+	p[PAGE_4K / 2] = (unsigned char)nr_calls;
+
+	if (nr_calls != 0) {
+		stack_expand(ret, nr_calls-1);
+	} else {
+		*ret = (unsigned long)&fill[0];
+	}
 }
 
 static int
 test_stack_f(va_list ap)
 {
-	char s;
-	stack_expand(&s);
+	unsigned long ret = 0;
+	unsigned long ret_addr = (unsigned long)&ret;
+
+	/*
+	 * We can't just dirty the stack in precise
+	 * way without using assembly. Thus lets do
+	 * the following trick:
+	 *  - assume 8K will be enough to carry all
+	 *    arguments passed for all calls, still
+	 *    we might need to adjust this value
+	 */
+	stack_expand(&ret, (stack_expand_limit - 2 * page_size) / page_size);
 	return 0;
 }
 
@@ -193,6 +213,11 @@ main_f(va_list ap)
 
 int main()
 {
+	page_size = sysconf(_SC_PAGESIZE);
+
+	/* Page should be at least 4K */
+	assert(page_size >= PAGE_4K);
+
 	memory_init();
 	fiber_init(fiber_cxx_invoke);
 	fiber_attr_create(&default_attr);
-- 
2.26.2