)]}'
{
  "commit": "0b535498eae6b66da8b9534e7ca55a561248d90b",
  "tree": "3a87577b7172b000565805f11c69ce4f9095943d",
  "parents": [
    "00f4447bd57f7a312d160b28a4d3fbd0c8ae072c"
  ],
  "author": {
    "name": "David Benjamin",
    "email": "davidben@google.com",
    "time": "Thu May 15 18:23:29 2025 -0400"
  },
  "committer": {
    "name": "Boringssl LUCI CQ",
    "email": "boringssl-scoped@luci-project-accounts.iam.gserviceaccount.com",
    "time": "Fri May 16 11:03:59 2025 -0700"
  },
  "message": "Fix some theoretical missing earlyclobber markers in inline assembly\n\nGCC inline assembly, by default, assumes that, like most single\ninstructions, the assembly block reads all inputs before writing any\noutputs. This way it can use the same register for some input and some\noutput operand.\n\nWhen the assembly block contains multiple instructions, this may not be\nthe case. Then registers must be marked \u0027\u0026\u0027 for earlyclobber. OpenSSL\ndid, from what I can tell, correctly mark all \"\u003d\" output constraints as\nearlyclobber, but not \"+\".\n\nSee https://gcc.gnu.org/onlinedocs/gcc/Modifiers.html\n\nNaively, it would seem earlyclobber is redundant for \"+\" because\ninput/output constraints cannot alias with other inputs. *However*, if\nthe compiler can prove that, on input to inline asm, some \"+r\"\ninput/output constraint and some \"r\" input constraint have the same\nvalue, it might merge the registers. If the \"+r\" constraint is clobbered\nbefore the \"r\" is read, the code will then break.\n\nGCC seems to do exactly this: https://godbolt.org/z/7hEGoE66a\n\nI\u0027ve not been able to come up with any other scenario where the compiler\nis allowed to do this. If the values are not proven equal at the point\nthe inline assembly block is lowered, the compiler is obligated to\npresent different registers to carry the different values.\n\nThere were a few inline assembly blocks that were missing an \"\u0026\" based\non this requirement. These instances seem to all be theoretical:\n\n- In mul_add\u0027s second block, %0 (carry) might alias %3 (0) if carry is\n  known to be zero. But presumably any compiler would just bind %3 to a\n  $0 literal. To be sure it does that, I\u0027ve just removed it in favor of\n  $0 in the assembly string because that seems absurd.\n\n  Once that\u0027s removed, I do not believe \u0026 is needed because now we only\n  need to worry about clobbering %1 (high). That is an output operand\n  and I don\u0027t believe there\u0027s any scenario where the compiler is allowed\n  to alias two outputs. The GCC documentation also specifically says\n  \"input operands\" and %1 is an output operand that happens to have an\n  in/out constraint on it.\n\n- In mul_add\u0027s third block, %0 (r) might alias %3 (0) if r is known to\n  be zero. Ditto.\n\n- In mul\u0027s second block, %0 (carry) might alias %3 (0) if carry is known\n  to be zero. Ditto.\n\n- In bn_add_words, %1 (n) and %2 (i) might alias any of %3 (rp), %4\n  (ap), or %5 (r). The in/out params are numbers, while the inputs are\n  pointers, so they can only be proven equal by the compiler if the\n  numbers are zero and the pointers are null. (It is conceivably\n  possible for a buffer size and buffer address to overlap, but it will\n  never be statically true that this happened except in intentionally\n  contrived calling code.) The pointers can only be null if n is zero,\n  but the functions already check this.\n\n- In bn_sub_words, we have the same situation as bn_add_words.\n\n- In mul_add_c\u0027s second block, %0 (c0) and %1 (c1) might alias with %4\n  (t2) and %5 (0). See the above discussion for 0. t2 is the output of\n  another inline asm block, so it is impossible for the compiler to\n  prove anything about its value. Once the 0 is resolved, only %0 needs\n  an earlyclobber marker because %1 is only in danger of clobbering\n  another in/out output operand.\n\n- In sqr_add_c\u0027s second block, ditto.\n\n- In mul_add_c2\u0027s second and third blocks, ditto.\n\nNote +m does not seem to need \u0026. Clang complains when I even put one on,\nand m cannot alias r.\n\nChange-Id: I3edbc7e6347f0271b87c7fc4da5b2971c6861f5a\nReviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/79387\nCommit-Queue: David Benjamin \u003cdavidben@google.com\u003e\nReviewed-by: Adam Langley \u003cagl@google.com\u003e\nAuto-Submit: David Benjamin \u003cdavidben@google.com\u003e\n",
  "tree_diff": [
    {
      "type": "modify",
      "old_id": "c1f0fe9fe599ac379177b471a69ca23ecb4e0a46",
      "old_mode": 33188,
      "old_path": "crypto/fipsmodule/bn/asm/x86_64-gcc.cc.inc",
      "new_id": "cbf7107b02848df7c4d421dc18d6e1304fb2c184",
      "new_mode": 33188,
      "new_path": "crypto/fipsmodule/bn/asm/x86_64-gcc.cc.inc"
    }
  ]
}