Thank you for the fixes and improvements. The clobber change is the only that gives me pause. I think the volatile keyword on both is sufficient to prevent re-ordering. Are you sure we need the memory clobber? Thanks. -Dan
On Tue, Feb 16, 2016 at 5:04 AM David Gibson <da...@gibson.dropbear.id.au> wrote: > altstack includes a couple of inline asm blocks with x86 push and pop > instructions. These instructions will access memory (the stack), but > that's not declared in inline asm statement. We seem to be getting away > with it, but in theory that could allow the compiler to re-order accesses > to local variables across the asm block. Since those blocks change the > location of the stack, that could be very bad. > > Adding a "memory" clobber should prevent this (effectively making the asm > blocks a compiler memory barrier). > > Signed-off-by: David Gibson <da...@gibson.dropbear.id.au> > --- > ccan/altstack/altstack.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/ccan/altstack/altstack.c b/ccan/altstack/altstack.c > index 640344d..6351293 100644 > --- a/ccan/altstack/altstack.c > +++ b/ccan/altstack/altstack.c > @@ -108,9 +108,10 @@ int altstack(rlim_t max, void *(*fn)(void *), void > *arg, void **out) > "mov %1, %%rsp\n\t" > "sub $8, %%rsp\n\t" > "push %%r10" > - : "=r" (rsp_save_[0]) : "0" (m + max) : "r10"); > + : "=r" (rsp_save_[0]) : "0" (m + max) : "r10", > "memory"); > out_ = fn_(arg_); > - asm volatile ("pop %rsp"); > + asm volatile ("pop %%rsp" > + : : : "memory"); > ret = 0; > if (out) *out = out_; > } > -- > 2.5.0 > >
_______________________________________________ ccan mailing list ccan@lists.ozlabs.org https://lists.ozlabs.org/listinfo/ccan