Re: [PATCH] altstack: stack alignment and accounting tweaks

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1.1: Type: text/plain, Size: 7422 bytes --]

On Mon, Feb 01, 2016 at 02:43:16AM +0000, Dan Good wrote:
> 
> * add altstack_remn, returns amount of stack remaining
> * increase mapping by 1 page to handle abutment case
> * capture rsp earlier
> * align stack to 16 bytes
> 
> Signed-off-by: Dan Good <dan@dancancode.com>

Bit of a messy bitser, but doesn't look it will make anything
worse. so go ahead and push.

Unfortunately, altstack is breaking travis builds at the moment (see,
e.g. https://travis-ci.org/dgibson/ccan/jobs/106661592) - but not
local builds for me which is making it difficult to debug.

Some investigation showed that the test prog was getting a SIGBUS, but
I didn't get further than that.

altstack also breaks "make all" on any non-x86_64 platform (including
32-bit x86) which is pretty horrid.  For that I think we need to
adjust the makefiles to look at rusty's new "ported" stuff from _info.


> ---
>  ccan/altstack/altstack.c | 17 ++++++++++++++---
>  ccan/altstack/altstack.h | 14 ++++++++++++++
>  ccan/altstack/test/run.c | 20 +++++++++++++-------
>  3 files changed, 41 insertions(+), 10 deletions(-)
> 
> diff --git a/ccan/altstack/altstack.c b/ccan/altstack/altstack.c
> index 6faf38f..b71c64f 100644
> --- a/ccan/altstack/altstack.c
> +++ b/ccan/altstack/altstack.c
> @@ -8,6 +8,7 @@
>  #include <signal.h>
>  #include <stdio.h>
>  #include <string.h>
> +#include <unistd.h>
>  #include <sys/mman.h>
>  
>  static __thread char ebuf[ALTSTACK_ERR_MAXLEN];
> @@ -37,6 +38,11 @@ static void segvjmp(int signum)
>  }
>  
>  static __thread void *rsp_save_[2];
> +static __thread rlim_t max_;
> +
> +rlim_t altstack_max(void) {
> +	return max_;
> +}
>  
>  static ptrdiff_t rsp_save(unsigned i) {
>  	assert(i < 2);
> @@ -57,6 +63,7 @@ static __thread void *arg_, *out_;
>  
>  int altstack(rlim_t max, void *(*fn)(void *), void *arg, void **out)
>  {
> +	long pgsz = sysconf(_SC_PAGESIZE);
>  	int ret = -1, undo = 0;
>  	char *m;
>  	struct rlimit rl_save;
> @@ -69,11 +76,16 @@ int altstack(rlim_t max, void *(*fn)(void *), void *arg, void **out)
>  	fn_  = fn;
>  	arg_ = arg;
>  	out_ = 0;
> +	max_ = max;
>  	ebuf[elen = 0] = '\0';
>  	if (out) *out = 0;
>  
> +	// if the first page below the mapping is in use, we get max-pgsz usable bytes
> +	// add pgsz to max to guarantee at least max usable bytes
> +	max += pgsz;
> +
>  	ok(getrlimit(RLIMIT_STACK, &rl_save), 1);
> -	ok(setrlimit(RLIMIT_STACK, &(struct rlimit) { max, rl_save.rlim_max }), 1);
> +	ok(setrlimit(RLIMIT_STACK, &(struct rlimit) { max_, rl_save.rlim_max }), 1);
>  	undo++;
>  
>  	ok(m = mmap(0, max, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS|MAP_GROWSDOWN|MAP_NORESERVE, -1, 0), 1);
> @@ -91,8 +103,7 @@ int altstack(rlim_t max, void *(*fn)(void *), void *arg, void **out)
>  		ok(sigaction(SIGSEGV, &sa, &sa_save), 1);
>  		undo++;
>  
> -		asm volatile ("movq %%rsp, %%r10\nmov %0, %%rsp\npush %%r10" : : "g" (m + max) : "r10");
> -		rsp_save(0);
> +		asm volatile ("movq %%rsp, %%r10\nmov %1, %%rsp\nmov %%rsp, %0\nsub $8, %%rsp\npush %%r10" : "=g" (rsp_save_[0]) : "g" (m + max) : "r10");
>  		out_ = fn_(arg_);
>  		asm volatile ("pop %rsp");
>  		ret = 0;
> diff --git a/ccan/altstack/altstack.h b/ccan/altstack/altstack.h
> index 5570e7b..4445a2a 100644
> --- a/ccan/altstack/altstack.h
> +++ b/ccan/altstack/altstack.h
> @@ -104,6 +104,20 @@ char *altstack_geterr(void);
>  ptrdiff_t altstack_used(void);
>  
>  /**
> + * altstack_max - return usable stack size
> + *
> + * Returns: max value from altstack() call
> + */
> +rlim_t altstack_max(void);
> +
> +/**
> + * altstack_remn - return amount of stack remaining
> + *
> + * Returns: altstack_max() minus altstack_used()
> + */
> +#define altstack_remn() (altstack_max() - altstack_used())
> +
> +/**
>   * altstack_rsp_save - set initial rsp value
>   *
>   * Capture the current value of rsp for future altstack_used()
> diff --git a/ccan/altstack/test/run.c b/ccan/altstack/test/run.c
> index adc1020..d0b8d28 100644
> --- a/ccan/altstack/test/run.c
> +++ b/ccan/altstack/test/run.c
> @@ -4,6 +4,7 @@
>  #include <setjmp.h>
>  #include <signal.h>
>  #include <string.h>
> +#include <unistd.h>
>  #include <sys/mman.h>
>  #include <ccan/tap/tap.h>
>  #include <ccan/altstack/altstack.h>
> @@ -20,13 +21,13 @@ enum {
>  };
>  int fail, call1, call2;
>  char *m_;
> -rlim_t max_;
> +rlim_t msz_;
>  #define e(x) (900+(x))
>  #define seterr(x) (errno = e(x))
>  #define setcall(x) ((call1 |= !errno ? (x) : 0), (call2 |= errno || out_ ? (x) : 0))
>  #define getrlimit(...)		(fail&getrlimit_	? (seterr(getrlimit_),		-1) : (setcall(getrlimit_),	getrlimit(__VA_ARGS__)))
>  #define mmap(...)		(fail&mmap_		? (seterr(mmap_),	(void *)-1) : (setcall(mmap_),		mmap(__VA_ARGS__)))
> -#define munmap(a, b)		(fail&munmap_		? (seterr(munmap_),		-1) : (setcall(munmap_),	munmap(m_=(a), max_=(b))))
> +#define munmap(a, b)		(fail&munmap_		? (seterr(munmap_),		-1) : (setcall(munmap_),	munmap(m_=(a), msz_=(b))))
>  #define setrlimit(...)		(fail&setrlimit_	? (seterr(setrlimit_),		-1) : (setcall(setrlimit_),	setrlimit(__VA_ARGS__)))
>  #define sigaltstack(...)	(fail&sigaltstack_	? (seterr(sigaltstack_),	-1) : (setcall(sigaltstack_),	sigaltstack(__VA_ARGS__)))
>  #define sigaction(...)		(fail&sigaction_	? (seterr(sigaction_),		-1) : (setcall(sigaction_),	sigaction(__VA_ARGS__)))
> @@ -58,7 +59,9 @@ static void *wrap(void *i)
>  
>  int main(void)
>  {
> -	plan_tests(16);
> +	long pgsz = sysconf(_SC_PAGESIZE);
> +
> +	plan_tests(17);
>  
>  #define chkfail(x, y, z, c1, c2) (call1 = 0, call2 = 0, errno = 0, ok1((fail = x) && (y) && errno == (z) && call1 == (c1) && call2 == (c2)));
>  #define   chkok(   y, z, c1, c2) (call1 = 0, call2 = 0, errno = 0, fail = 0,     ok1((y) && errno == (z) && call1 == (c1) && call2 == (c2)));
> @@ -86,7 +89,7 @@ int main(void)
>  	chkfail(munmap_,	altstack(8*MiB, wrap, 0, 0) ==  1, e(munmap_),
>  		getrlimit_|setrlimit_|mmap_|sigaltstack_|sigaction_,
>  		setrlimit_|sigaltstack_|sigaction_);
> -	if (fail = 0, munmap(m_, max_) == -1)
> +	if (fail = 0, munmap(m_, msz_) == -1)
>  		err(1, "munmap");
>  
>  	chkok(			altstack(1*MiB, wrap, (void *) 1000000, 0) == -1, EOVERFLOW,
> @@ -102,10 +105,12 @@ int main(void)
>  	chkfail(munmap_,	altstack(1*MiB, wrap, (void *) 1000000, 0) == -1, EOVERFLOW,
>  		getrlimit_|setrlimit_|mmap_|sigaltstack_|sigaction_,
>  		setrlimit_|sigaltstack_|sigaction_);
> -	if (fail = 0, munmap(m_, max_) == -1)
> +	if (fail = 0, munmap(m_, msz_) == -1)
>  		err(1, "munmap");
>  
> -	ok1(used > 1*MiB-1*KiB && used < 1*MiB);
> +	ok1(altstack_max() == 1*MiB);
> +	diag("used: %lu", used);
> +	ok1(used >= 1*MiB && used <= 1*MiB + pgsz);
>  
>  	char *p;
>  	for(p = altstack_geterr(); *p; p++)
> @@ -128,7 +133,8 @@ int main(void)
>  		getrlimit_|setrlimit_|mmap_|sigaltstack_|sigaction_,
>  		setrlimit_|munmap_|sigaltstack_|sigaction_);
>  
> -	ok1(used > 8*MiB-8*KiB && used < 8*MiB);
> +	diag("used: %lu", used);
> +	ok1(used >= 8*MiB && used <= 8*MiB + pgsz);
>  
>  	used = 0;
>  	chkok(			altstack(8*MiB, wrap, (void *) 100000, 0) == 0, 0,

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan