I just came across the following MIPS code at line 328 of
tls_pstm_montgomery_reduce.c:
#define PROPCARRY \
asm( \
" lw $10,%1 \n\t" \
" addu $10,$10,%0 \n\t" \
" sw $10,%1 \n\t" \
" sltu %0,$10,%0 \n\t" \
:"=r"(cy),"=m"(_c[0])\
:"r"(cy),"r"(_c[0])\
:"$10");
Notice the lack of constraint forcing the output carry
to be in the same register as the input. This code
computes the output carry into %0, but the compiler
expects it in %2.
A second bug is the fact that the compiler provides
_c[0] in register %3, but the code fetches it from %1.
Nobody told the compiler to ensure that memory is up to
date! It might elide a store and pass the value to this
code instead.
Even if the above isn't a bug, it's still a waste of
code, as the compiler will load the value into a register
before the asm(), which will ignore it and fetch it itself.
A third problem, although not a bug, is the use of fixed
register $10 as a temporary. Far better to declare
a variable and let the compiler pick the register.
The correct way to write this is:
#define PROPCARRY \
asm( \
"addu %1,%1,%0" "\n\t" \
"sltu %2,%1,%0" \
:"=r"(cy), "+r"(_c[0]) \
:"r"(cy))
Or better yet, don't use asm() at all and just write:
#define PROPCARRY \
cy = (_c[0] += cy) < cy
The MIPS INNERMUL macro has the same problems; it's just
bigger and so messier to explain.
Sheesh. The correct way to write that is:
#define INNERMUL do { \
pstm_digit lo, hi; \
asm( \
"multu %2,%3" "\n\t" \
"mflo %0" "\n\t" \
"mfhi %1" \
:"=r"(lo), "=r"(hi), \
:"r"(mu), "r"(*tmpm++)); \
hi += (lo += cy) < cy; \
hi += (_c[0] += lo) < lo; \
cy = hi; \
} while (0)
I haven't yet checked if this is fixed in upstream
MatrixSSL.
_______________________________________________
busybox mailing list
[email protected]
https://lists.busybox.net/mailman/listinfo/busybox
