Avoid optimizing out a verify_check

[elichai]

Before that even on debug the compiler could've assumed a isn't null and optimized VERIFY_CHECK(a != NULL); out.
This put the dereference after the check
Resolves #643

[real-or-random]

ACK, I read the code

[jonasnick]

ACK mod nit

[real-or-random]

ACK read the code

[jonasnick]

ACK read the code

[gmaxwell]

This description is one which would incorrectly cause a overblown reaction. I would have described it as "prevent the compiler from optimizing out an interface invariant assertion" or similar. "checking null pointer before dereferencing" is a description people use when fixing serious vulnerabilities.

[elichai]

When I named this I tried to think of a name that won't sound alarming but still describe what's the point.
I didn't try to make this sound like a "vulnerability". it's not.
It's so nothing I wasn't even sure if it's worth a PR and that's why I opened an issue first #643

If anyone looking at this PR thought it is a vulnerability it's my fault. suggestions for a title?

[gmaxwell]

No need to apologize, you did nothing wrong. We want people to treat serious issues seriously, but an unfortunate side effect is that a little extra effort needs to go into not falsely triggering serious issue detection. My comment had a suggestion, "avoid optimizing out a verify_check" which was actually your goal! :) Generally it's most informative if changes are described "why" first either describe the "how" later (or just leave it to the code itself to describe the "how").

To add some additional colour which might be useful to you in the future there is actually a fair amount of machinery in the codebase already just to avoid optimizing out VERIFY_CHECKs (and even some ARG_CHECKs): Most of the external interface is annotated with nonnull in order to cause the compiler to warn at compile time on the caller's code when it can prove that an argument will be null. Unfortunately that annotation is treated like a dereference when compiling the function and will cause null checks to be treated like dead code. As a result, when compiling the library itself those annotations are left out (triggered by SECP256K1_BUILD define).

[elichai]

Yeah i'm still trying to figure out all the autotools and macro configurations in this library :)

[sipa]

ACK 94ae7cb