commit 0b535498eae6b66da8b9534e7ca55a561248d90b
author David Benjamin <davidben@google.com> Thu May 15 18:23:29 2025 -0400
committer Boringssl LUCI CQ <boringssl-scoped@luci-project-accounts.iam.gserviceaccount.com> Fri May 16 11:03:59 2025 -0700
tree 3a87577b7172b000565805f11c69ce4f9095943d
parent 00f4447bd57f7a312d160b28a4d3fbd0c8ae072c

Fix some theoretical missing earlyclobber markers in inline assembly

GCC inline assembly, by default, assumes that, like most single
instructions, the assembly block reads all inputs before writing any
outputs. This way it can use the same register for some input and some
output operand.

When the assembly block contains multiple instructions, this may not be
the case. Then registers must be marked '&' for earlyclobber. OpenSSL
did, from what I can tell, correctly mark all "=" output constraints as
earlyclobber, but not "+".

See https://gcc.gnu.org/onlinedocs/gcc/Modifiers.html

Naively, it would seem earlyclobber is redundant for "+" because
input/output constraints cannot alias with other inputs. *However*, if
the compiler can prove that, on input to inline asm, some "+r"
input/output constraint and some "r" input constraint have the same
value, it might merge the registers. If the "+r" constraint is clobbered
before the "r" is read, the code will then break.

GCC seems to do exactly this: https://godbolt.org/z/7hEGoE66a

I've not been able to come up with any other scenario where the compiler
is allowed to do this. If the values are not proven equal at the point
the inline assembly block is lowered, the compiler is obligated to
present different registers to carry the different values.

There were a few inline assembly blocks that were missing an "&" based
on this requirement. These instances seem to all be theoretical:

- In mul_add's second block, %0 (carry) might alias %3 (0) if carry is
  known to be zero. But presumably any compiler would just bind %3 to a
  $0 literal. To be sure it does that, I've just removed it in favor of
  $0 in the assembly string because that seems absurd.

  Once that's removed, I do not believe & is needed because now we only
  need to worry about clobbering %1 (high). That is an output operand
  and I don't believe there's any scenario where the compiler is allowed
  to alias two outputs. The GCC documentation also specifically says
  "input operands" and %1 is an output operand that happens to have an
  in/out constraint on it.

- In mul_add's third block, %0 (r) might alias %3 (0) if r is known to
  be zero. Ditto.

- In mul's second block, %0 (carry) might alias %3 (0) if carry is known
  to be zero. Ditto.

- In bn_add_words, %1 (n) and %2 (i) might alias any of %3 (rp), %4
  (ap), or %5 (r). The in/out params are numbers, while the inputs are
  pointers, so they can only be proven equal by the compiler if the
  numbers are zero and the pointers are null. (It is conceivably
  possible for a buffer size and buffer address to overlap, but it will
  never be statically true that this happened except in intentionally
  contrived calling code.) The pointers can only be null if n is zero,
  but the functions already check this.

- In bn_sub_words, we have the same situation as bn_add_words.

- In mul_add_c's second block, %0 (c0) and %1 (c1) might alias with %4
  (t2) and %5 (0). See the above discussion for 0. t2 is the output of
  another inline asm block, so it is impossible for the compiler to
  prove anything about its value. Once the 0 is resolved, only %0 needs
  an earlyclobber marker because %1 is only in danger of clobbering
  another in/out output operand.

- In sqr_add_c's second block, ditto.

- In mul_add_c2's second and third blocks, ditto.

Note +m does not seem to need &. Clang complains when I even put one on,
and m cannot alias r.

Change-Id: I3edbc7e6347f0271b87c7fc4da5b2971c6861f5a
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/79387
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Auto-Submit: David Benjamin <davidben@google.com>