Re: Dead stores in src/common/sha2.c

Michael Paquier <michael(at)paquier(dot)xyz> writes:
> On Wed, May 29, 2019 at 01:24:19PM +0000, Hamlin, Garick L wrote:
>> I ran clang checker and noticed these. It looks like the
>> sha2 implementation is trying to zero out state on exit, but
>> clang checker finds at least 'a' is a dead store.
>>
>> Should we fix this?
>> Is something like the attached sensible?
>> Is there a common/better approach to zero-out in PG ?

> This code comes from the SHA-2 implementation of OpenBSD, so it is not
> adapted to directly touch it. What's the current state of this code
> in upstream? Should we perhaps try to sync with the upstream
> implementation instead?
> After a quick search I am not seeing that this area has actually
> changed:
> http://fxr.watson.org/fxr/source/crypto/sha2.c?v=OPENBSD

Hm ... plastering "volatile"s all over it isn't good for readability
or for quality of the generated code. (In particular, I'm worried
about this patch causing all those variables to be forced into memory
instead of registers.)

At the same time, I'm not sure if we should just write this off as an
ignorable warning. If the C compiler concludes these are dead stores
it'll probably optimize them away, leading to not accomplishing the
goal of wiping the values.

On the third hand, that goal may not be worth much, particularly not
if the variables do get kept in registers.

regards, tom lane