diff mbox

[V4,1/6] oslib-posix: add helpers for stack alloc and free

Message ID 1468228082-7492-2-git-send-email-pl@kamp.de (mailing list archive)
State New, archived
Headers show

Commit Message

Peter Lieven July 11, 2016, 9:07 a.m. UTC
the allocated stack will be adjusted to the minimum supported stack size
by the OS and rounded up to be a multiple of the system pagesize.
Additionally an architecture dependent guard page is added to the stack
to catch stack overflows.

Signed-off-by: Peter Lieven <pl@kamp.de>
---
 include/sysemu/os-posix.h | 23 +++++++++++++++++++++++
 util/oslib-posix.c        | 44 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 67 insertions(+)

Comments

Richard Henderson July 11, 2016, 4:28 p.m. UTC | #1
On 07/11/2016 02:07 AM, Peter Lieven wrote:
> the allocated stack will be adjusted to the minimum supported stack size
> by the OS and rounded up to be a multiple of the system pagesize.
> Additionally an architecture dependent guard page is added to the stack
> to catch stack overflows.
>
> Signed-off-by: Peter Lieven <pl@kamp.de>
> ---
>  include/sysemu/os-posix.h | 23 +++++++++++++++++++++++
>  util/oslib-posix.c        | 44 ++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 67 insertions(+)

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~
Eric Blake July 11, 2016, 4:39 p.m. UTC | #2
On 07/11/2016 03:07 AM, Peter Lieven wrote:
> the allocated stack will be adjusted to the minimum supported stack size
> by the OS and rounded up to be a multiple of the system pagesize.
> Additionally an architecture dependent guard page is added to the stack
> to catch stack overflows.
> 
> Signed-off-by: Peter Lieven <pl@kamp.de>
> ---
>  include/sysemu/os-posix.h | 23 +++++++++++++++++++++++
>  util/oslib-posix.c        | 44 ++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 67 insertions(+)
> 

> +
> +static size_t adjust_stack_size(size_t sz)
> +{
> +    /* avoid stacks smaller than _SC_THREAD_STACK_MIN */
> +    sz = MAX(sz, sysconf(_SC_THREAD_STACK_MIN));

sz is unsigned, but sysconf() is signed.  Furthermore, sysconf() is
permitted to return -1 if there is no such minimum.  MAX() would then
operate on the common integral promotion between the two arguments,
which may treat (unsigned)(-1) as the larger of the two values, and give
you the wrong results.

I think it is theoretical (all platforms that we compile on have a
working sysconf(_SC_THREAD_STACK_MIN), right?), but still may be worth
being sure that sysconf() returned a positive value before computing MAX().
Peter Lieven July 12, 2016, 2:36 p.m. UTC | #3
Am 11.07.2016 um 18:39 schrieb Eric Blake:
> On 07/11/2016 03:07 AM, Peter Lieven wrote:
>> the allocated stack will be adjusted to the minimum supported stack size
>> by the OS and rounded up to be a multiple of the system pagesize.
>> Additionally an architecture dependent guard page is added to the stack
>> to catch stack overflows.
>>
>> Signed-off-by: Peter Lieven <pl@kamp.de>
>> ---
>>  include/sysemu/os-posix.h | 23 +++++++++++++++++++++++
>>  util/oslib-posix.c        | 44 ++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 67 insertions(+)
>>
>> +
>> +static size_t adjust_stack_size(size_t sz)
>> +{
>> +    /* avoid stacks smaller than _SC_THREAD_STACK_MIN */
>> +    sz = MAX(sz, sysconf(_SC_THREAD_STACK_MIN));
> sz is unsigned, but sysconf() is signed.  Furthermore, sysconf() is
> permitted to return -1 if there is no such minimum.  MAX() would then
> operate on the common integral promotion between the two arguments,
> which may treat (unsigned)(-1) as the larger of the two values, and give
> you the wrong results.
>
> I think it is theoretical (all platforms that we compile on have a
> working sysconf(_SC_THREAD_STACK_MIN), right?), but still may be worth
> being sure that sysconf() returned a positive value before computing MAX().
>

If you feel more comfortable I can surround it by a

if (sysconf(_SC_THREAD_STACK_MIN) > 0) { }

I wonder if the _SC_THREAD_STACK_MIN constant exists if there is no minimum?

Peter
Peter Lieven July 12, 2016, 2:41 p.m. UTC | #4
Am 12.07.2016 um 16:36 schrieb Peter Lieven:
> Am 11.07.2016 um 18:39 schrieb Eric Blake:
>> On 07/11/2016 03:07 AM, Peter Lieven wrote:
>>> the allocated stack will be adjusted to the minimum supported stack size
>>> by the OS and rounded up to be a multiple of the system pagesize.
>>> Additionally an architecture dependent guard page is added to the stack
>>> to catch stack overflows.
>>>
>>> Signed-off-by: Peter Lieven <pl@kamp.de>
>>> ---
>>>  include/sysemu/os-posix.h | 23 +++++++++++++++++++++++
>>>  util/oslib-posix.c        | 44 ++++++++++++++++++++++++++++++++++++++++++++
>>>  2 files changed, 67 insertions(+)
>>>
>>> +
>>> +static size_t adjust_stack_size(size_t sz)
>>> +{
>>> +    /* avoid stacks smaller than _SC_THREAD_STACK_MIN */
>>> +    sz = MAX(sz, sysconf(_SC_THREAD_STACK_MIN));
>> sz is unsigned, but sysconf() is signed.  Furthermore, sysconf() is
>> permitted to return -1 if there is no such minimum.  MAX() would then
>> operate on the common integral promotion between the two arguments,
>> which may treat (unsigned)(-1) as the larger of the two values, and give
>> you the wrong results.
>>
>> I think it is theoretical (all platforms that we compile on have a
>> working sysconf(_SC_THREAD_STACK_MIN), right?), but still may be worth
>> being sure that sysconf() returned a positive value before computing MAX().
>>
> If you feel more comfortable I can surround it by a
>
> if (sysconf(_SC_THREAD_STACK_MIN) > 0) { }
>
> I wonder if the _SC_THREAD_STACK_MIN constant exists if there is no minimum?

Update:

glibc basically does the following:

static gulong g_thread_min_stack_size = 0;

#ifdef _SC_THREAD_STACK_MIN
g_thread_min_stack_size = MAX (sysconf (_SC_THREAD_STACK_MIN), 0);
#endif /* _SC_THREAD_STACK_MIN */

stack_size = MAX (g_thread_min_stack_size, stack_size);


So we should do sth similar, I think?!

Peter
Eric Blake July 12, 2016, 3:32 p.m. UTC | #5
On 07/12/2016 08:41 AM, Peter Lieven wrote:
>> I wonder if the _SC_THREAD_STACK_MIN constant exists if there is no minimum?
> 
> Update:
> 
> glibc basically does the following:
> 
> static gulong g_thread_min_stack_size = 0;
> 
> #ifdef _SC_THREAD_STACK_MIN
> g_thread_min_stack_size = MAX (sysconf (_SC_THREAD_STACK_MIN), 0);
> #endif /* _SC_THREAD_STACK_MIN */
> 
> stack_size = MAX (g_thread_min_stack_size, stack_size);
> 
> 
> So we should do sth similar, I think?!

Yes, that would be a good pattern to copy.
diff mbox

Patch

diff --git a/include/sysemu/os-posix.h b/include/sysemu/os-posix.h
index 9c7dfdf..7630665 100644
--- a/include/sysemu/os-posix.h
+++ b/include/sysemu/os-posix.h
@@ -60,4 +60,27 @@  int qemu_utimens(const char *path, const qemu_timespec *times);
 
 bool is_daemonized(void);
 
+/**
+ * qemu_alloc_stack:
+ * @sz: size of required stack in bytes
+ *
+ * Allocate memory that can be used as a stack, for instance for
+ * coroutines. If the memory cannot be allocated, this function
+ * will abort (like g_malloc()).
+ *
+ * The allocated stack must be freed with qemu_free_stack().
+ *
+ * Returns: pointer to (the lowest address of) the stack memory.
+ */
+void *qemu_alloc_stack(size_t sz);
+
+/**
+ * qemu_free_stack:
+ * @stack: stack to free
+ * @sz: size of stack in bytes
+ *
+ * Free a stack allocated via qemu_alloc_stack().
+ */
+void qemu_free_stack(void *stack, size_t sz);
+
 #endif
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index e2e1d4d..9e7bc65 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -497,3 +497,47 @@  pid_t qemu_fork(Error **errp)
     }
     return pid;
 }
+
+static size_t adjust_stack_size(size_t sz)
+{
+    /* avoid stacks smaller than _SC_THREAD_STACK_MIN */
+    sz = MAX(sz, sysconf(_SC_THREAD_STACK_MIN));
+    /* adjust stack size to a multiple of the page size */
+    sz = ROUND_UP(sz, getpagesize());
+    return sz;
+}
+
+void *qemu_alloc_stack(size_t sz)
+{
+    void *ptr, *guardpage;
+    size_t pagesz = getpagesize();
+    sz = adjust_stack_size(sz);
+
+    ptr = mmap(NULL, sz, PROT_READ | PROT_WRITE,
+               MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+    if (ptr == MAP_FAILED) {
+        abort();
+    }
+
+#if defined(HOST_IA64)
+    /* separate register stack */
+    guardpage = ptr + (((sz - pagesz) / 2) & ~pagesz);
+#elif defined(HOST_HPPA)
+    /* stack grows up */
+    guardpage = ptr + sz - pagesz;
+#else
+    /* stack grows down */
+    guardpage = ptr;
+#endif
+    if (mprotect(guardpage, pagesz, PROT_NONE) != 0) {
+        abort();
+    }
+
+    return ptr;
+}
+
+void qemu_free_stack(void *stack, size_t sz)
+{
+    sz = adjust_stack_size(sz);
+    munmap(stack, sz);
+}