Closed theuni closed 6 months ago
Should fix #1511
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.
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.
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.
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.
utACK f7f0184ba1358cb8659607d2d0105628cb130e4f
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.
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.