msan: notate more variable assignments from assembly code

[theuni]

This was missed in 31ba40494428dcbf2eb5eb6f2328eca91b0b0746 because older versions of clang did not complain about it. But clang-17, at least, does.

The array-as-a-param makes this annoying because sizeof(l) is not helpful. I'd be happy to change the size calculation if there are any better suggestions or strong preferences.

[theuni]

Should fix #1511

[fanquake]

Nice. This fixes #1511 for me, under Clang 17 (Ubuntu clang version 17.0.6 (5build1)) and 18 (Ubuntu clang version 18.1.0 (rc2-4)). I've also cherry-picked it into https://github.com/bitcoin/bitcoin/pull/29742 to run through our CI.

[real-or-random]

The array-as-a-param makes this annoying because sizeof(l) is not helpful.

Hm, yeah, idk, the uint64_t l[8] parameter declaration is a bit strange, or at least unusual for this code base. I'd ACK a patch that changes this a pointer declaration, which we usually do in the codebase. If you do, maybe also rename to l8.

For others: The issue is that, despite the declaration, l will still decay to a pointer, i.e., l won't be of array type, but of type uint64_t*. And the [8] has no significance. That means that sizeof(l) == sizeof(uint64_t*), which, for maximum confusion, happens to be 8 coincidentally.

https://github.com/bitcoin-core/secp256k1/blob/05bfab69aef3622f77f754cfb01220108a109c91/src/scalar_4x64_impl.h#L684

[theuni]

TIL of the weird-looking

void func(uint64_t l[static 8])

syntax in c99: https://stackoverflow.com/questions/3430315/what-is-the-purpose-of-static-keyword-in-array-parameter-of-function-like-char

Sadly it doesn't do anything to help us here, just thought I'd share. Yet another totally unrelated thing that static can mean in c/c++ code :)

I'll add a commit to change this to a pointer as suggested.

[real-or-random]

void func(uint64_t l[static 8])

Haha okay, I totally wasn't aware of this, even though I happen to look at the standards text from time to time... And yeah, static is a bit cursed.

[sipa]

utACK f7f0184ba1358cb8659607d2d0105628cb130e4f

[real-or-random]

For posterity, the reason why clang 15 was happy with just the annotation in https://github.com/bitcoin-core/secp256k1/commit/31ba40494428dcbf2eb5eb6f2328eca91b0b0746 is this:

The default of what is considered a "use" of uninitialized memory was changed in clang 16. Returning an uninitialized variables from a function, or passing uninitialized values to a function as a parameter is now considered, and MSan will report it by default. See the Clang 16.0.09 Release Notes:

-fsanitize-memory-param-retval is turned on by default. With -fsanitize=memory, passing uninitialized variables to functions and returning uninitialized variables from functions is more aggressively reported. -fno-sanitize-memory-param-retval restores the previous behavior.