[acc] Add clobbered-register annotation checker#231
Conversation
mkannwischer
force-pushed
the
mjk/check-clobbered-regs
branch
from
23ddbbe to
334e46e
Compare
mkannwischer
force-pushed
the
mjk/check-clobbered-regs
branch
2 times, most recently
from
0b88b43 to
593a66c
Compare
Add a static checker that verifies clobbered-register docstring annotations against the actual registers written, as determined by the existing information-flow analysis. The check currently only covers a part of the routines (those that are currently supported by the information-flow analysis script). Signed-off-by: Matthias J. Kannwischer <matthias@zerorisc.com>
mkannwischer
force-pushed
the
mjk/check-clobbered-regs
branch
from
593a66c to
c161310
Compare
|
@pqcfox, @jadephilipoom, do you think something like this would be useful? Or am I overengineering this? |
jadephilipoom
left a comment
jadephilipoom
left a comment
There was a problem hiding this comment.
Thanks Matthias, looks good to me! I think this would be nice to have.
| # N-dependent RSA/bignum helpers, routines taking a WDR index in a GPR, | ||
| # and sha3_shake (no acc_binary) are omitted. | ||
| # | ||
| # TODO: investigate why these take 30s-200s; tagged "manual" until fixed. |
There was a problem hiding this comment.
I think this is probably because for loopi instructions the information-flow backend unrolls loops, and these ones all include the P384 scalarmult loop repeated 448 times. If so, we could fix it by making the backend treat large loops the same way it treats loops with a non-constant number of iterations, but at the cost of potentially missing some details in large loops that depend on a specific number of iterations, and perhaps making the behavior a little less predictable.
There was a problem hiding this comment.
As of #247 this issue is fixed; all the consttime checks take less than 5s.
| subroutine = sub, | ||
| tags = ["manual"] if (elf, sub) in _SLOW_CLOBBERED_REGS_SUBS else [], | ||
| deps = ["//sw/acc/crypto:" + elf], | ||
| ) for sub, src, elf in [ |
There was a problem hiding this comment.
Hmm, rather than this big loop it would be nice to run this almost as a linter. Could we put it under the top-level quality maybe with the other linters, and automatically run it on everything that's not specifically excluded?
There was a problem hiding this comment.
I will try this when i find some time.
2 participants
Add a static checker that verifies clobbered-register docstring
annotations against the actual registers written, as determined by
the existing information-flow analysis.
The check currently only covers a part of the routines
(those that are currently supported by the information-flow
analysis script).
Also adjusted the docstrings for the covered routines - most of those were not aligned with the output of the information-flow script, but some of those differences are just cosmetic (like changing
x2, x3tox2 to x3)