Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-Id: <20190925033015.9905-1-palmer@sifive.com>
Date: Tue, 24 Sep 2019 20:30:15 -0700
From: Palmer Dabbelt <palmer@...ive.com>
To: musl@...ts.openwall.com
Cc: Alistair Francis <Alistair.Francis@....com>, Palmer Dabbelt <palmer@...ive.com>
Subject: [PATCH] correct the operand specifiers in the riscv64 CAS routines

The operand sepcifiers in a_cas and a_casp for riscv64 were incorrect:
there's a backwards branch in the routine, so despite tmp being written
at the end of the assembly fragment it cannot be allocated in one of the
input registers because the input values may be needed for another trip
around the loop.

For code that follows the guarnteed forward progress requirements, he
backwards branch is rarely taken: SiFive's hardware only fails a store
conditional on execptional cases (ie, instruction cache misses inside
the loop), and until recently a bug in QEMU allowed back-to-back
store conditionals to succeed.  The bug has been fixed in the latest
QEMU release, but it turns out that the fix caused this latent bug in
musl to manifest.

Full disclosure: I haven't actually even compiled musl.  I just guessed
this would fix a bug introducted by the new QEMU behavior, Alistair
(CC'd) actually checked it fixes the problem.  The rest is just
conjecture.
---
 arch/riscv64/atomic_arch.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/riscv64/atomic_arch.h b/arch/riscv64/atomic_arch.h
index c976534284aa..41ad4d04907c 100644
--- a/arch/riscv64/atomic_arch.h
+++ b/arch/riscv64/atomic_arch.h
@@ -14,7 +14,7 @@ static inline int a_cas(volatile int *p, int t, int s)
 		"	sc.w.aqrl %1, %4, (%2)\n"
 		"	bnez %1, 1b\n"
 		"1:"
-		: "=&r"(old), "=r"(tmp)
+		: "=&r"(old), "=&r"(tmp)
 		: "r"(p), "r"(t), "r"(s)
 		: "memory");
 	return old;
@@ -31,7 +31,7 @@ static inline void *a_cas_p(volatile void *p, void *t, void *s)
 		"	sc.d.aqrl %1, %4, (%2)\n"
 		"	bnez %1, 1b\n"
 		"1:"
-		: "=&r"(old), "=r"(tmp)
+		: "=&r"(old), "=&r"(tmp)
 		: "r"(p), "r"(t), "r"(s)
 		: "memory");
 	return old;
-- 
2.21.0

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.