<div dir="ltr">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<br><br><div class="gmail_quote"><div dir="ltr">On Tue, Feb 16, 2016 at 5:04 AM David Gibson <<a href="mailto:david@gibson.dropbear.id.au">david@gibson.dropbear.id.au</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">altstack includes a couple of inline asm blocks with x86 push and pop<br>
instructions.  These instructions will access memory (the stack), but<br>
that's not declared in inline asm statement.  We seem to be getting away<br>
with it, but in theory that could allow the compiler to re-order accesses<br>
to local variables across the asm block.  Since those blocks change the<br>
location of the stack, that could be very bad.<br>
<br>
Adding a "memory" clobber should prevent this (effectively making the asm<br>
blocks a compiler memory barrier).<br>
<br>
Signed-off-by: David Gibson <<a href="mailto:david@gibson.dropbear.id.au" target="_blank">david@gibson.dropbear.id.au</a>><br>
---<br>
 ccan/altstack/altstack.c | 5 +++--<br>
 1 file changed, 3 insertions(+), 2 deletions(-)<br>
<br>
diff --git a/ccan/altstack/altstack.c b/ccan/altstack/altstack.c<br>
index 640344d..6351293 100644<br>
--- a/ccan/altstack/altstack.c<br>
+++ b/ccan/altstack/altstack.c<br>
@@ -108,9 +108,10 @@ int altstack(rlim_t max, void *(*fn)(void *), void *arg, void **out)<br>
                        "mov %1, %%rsp\n\t"<br>
                        "sub $8, %%rsp\n\t"<br>
                        "push %%r10"<br>
-                       : "=r" (rsp_save_[0]) : "0" (m + max) : "r10");<br>
+                       : "=r" (rsp_save_[0]) : "0" (m + max) : "r10", "memory");<br>
                out_ = fn_(arg_);<br>
-               asm volatile ("pop %rsp");<br>
+               asm volatile ("pop %%rsp"<br>
+                             : : : "memory");<br>
                ret = 0;<br>
                if (out) *out = out_;<br>
        }<br>
--<br>
2.5.0<br>
<br>
</blockquote></div></div>