)]}'
{
  "commit": "23e1a1f2d3c4ca0121eb93eb7d210501f9702317",
  "tree": "836a53c86b989ef30371810a60b7e89cbb31b464",
  "parents": [
    "ab578adf44b3e70255686e3a4009d7d35e6f1231"
  ],
  "author": {
    "name": "David Benjamin",
    "email": "davidben@google.com",
    "time": "Mon Jan 28 04:06:57 2019 +0000"
  },
  "committer": {
    "name": "CQ bot account: commit-bot@chromium.org",
    "email": "commit-bot@chromium.org",
    "time": "Mon Jan 28 21:09:40 2019 +0000"
  },
  "message": "Test and fix an ABI issue with small parameters.\n\nCalling conventions must specify how to handle arguments smaller than a\nmachine word. Should the caller pad them up to a machine word size with\npredictable values (zero/sign-extended), or should the callee tolerate\nan arbitrary bit pattern?\n\nAnnoyingly, I found no text in either SysV or Win64 ABI documentation\ndescribing any of this and resorted to experiment. The short answer is\nthat callees must tolerate an arbitrary bit pattern on x86_64, which\nmeans we must test this. See the comment in abi_test::internal::ToWord\nfor the long answer.\n\nCHECK_ABI now, if the type of the parameter is smaller than\ncrypto_word_t, fills the remaining bytes with 0xaa. This is so the\nnumber is out of bounds for code expecting either zero or sign\nextension. (Not that crypto assembly has any business seeing negative\nnumbers.)\n\nDoing so reveals a bug in ecp_nistz256_ord_sqr_mont. The rep parameter\nis typed int, but the code expected uint64_t. In practice, the compiler\nwill always compile this correctly because:\n\n- On both Win64 and SysV, rep is a register parameter.\n\n- The rep parameter is always a constant, so the compiler has no reason\n  to leave garbage in the upper half.\n\nHowever, I was indeed able to get a bug out of GCC via:\n\n  uint64_t foo \u003d (1ull \u003c\u003c 63) | 2;  // Some global the compiler can\u0027t\n                                    // prove constant.\n  ecp_nistz256_ord_sqr_mont(res, a, foo \u003e\u003e 1);\n\nWere ecp_nistz256_ord_sqr_mont a true int-taking function, this would\nact like ecp_nistz256_ord_sqr_mont(res, a, 1). Instead, it hung. Fix\nthis by having it take a full-width word.\n\nThis mess has several consequences:\n\n- ABI testing now ideally needs a functional testing component to fully cover\n  this case. A bad input might merely produce the wrong answer. Still,\n  this is fairly effective as it will cause most code to either segfault\n  or loop forever. (Not the enc parameter to AES however...)\n\n- We cannot freely change the type of assembly function prototypes. If the\n  prototype says int or unsigned, it must be ignoring the upper half and\n  thus \"fixing\" it to size_t cannot have handled the full range. (Unless\n  it was simply wrong of the parameter is already bounded.) If the\n  prototype says size_t, switching to int or unsigned will hit this type\n  of bug. The former is a safer failure mode though.\n\n- The simplest path out of this mess: new assembly code should *only*\n  ever take word-sized parameters. This is not a tall order as the bad\n  parameters are usually ints that should have been size_t.\n\nCalling conventions are hard.\n\nChange-Id: If8254aff8953844679fbce4bd3e345e5e2fa5213\nReviewed-on: https://boringssl-review.googlesource.com/c/34627\nCommit-Queue: David Benjamin \u003cdavidben@google.com\u003e\nReviewed-by: Adam Langley \u003cagl@google.com\u003e\n",
  "tree_diff": [
    {
      "type": "modify",
      "old_id": "2f5ae86a31e21aac0fc6a5d0edd0639a3819f755",
      "old_mode": 33261,
      "old_path": "crypto/fipsmodule/ec/asm/p256-x86_64-asm.pl",
      "new_id": "54028856e08e46cbfdb139d11b02fac4c836133a",
      "new_mode": 33261,
      "new_path": "crypto/fipsmodule/ec/asm/p256-x86_64-asm.pl"
    },
    {
      "type": "modify",
      "old_id": "2d70ca78b961a3b97d453a64f7b69dfb7a13b172",
      "old_mode": 33188,
      "old_path": "crypto/fipsmodule/ec/p256-x86_64.h",
      "new_id": "5deb81a3fec94e2e9ad1758c57d2caa9f99359e8",
      "new_mode": 33188,
      "new_path": "crypto/fipsmodule/ec/p256-x86_64.h"
    },
    {
      "type": "modify",
      "old_id": "4ff42d1a7bcc3f14b10ea06f0068b802224e02b2",
      "old_mode": 33188,
      "old_path": "crypto/test/abi_test.h",
      "new_id": "91b2d425f1aed72e13df60c9b6bcc72af9f7e3a1",
      "new_mode": 33188,
      "new_path": "crypto/test/abi_test.h"
    }
  ]
}