SECP256K1_BUILD sanity checking and USE_FIELD_INV_BUILTIN/USE_SCALAR_INV

gmaxwell commented at 7:34 pm on August 15, 2020: contributor

If secp256k1.c is built without SECP256K1_BUILD the non-null argument tests get compiled out and then the tests crash.

One way to address that would be to simply have test.c and secp256k1.c set the macro before importing the headers. I think this would be pretty fool-proof. If there is some reason that can’t be done secp256k1.c compile could fail if SECP256K1_BUILD isn’t set. I prefer the foolproof way because demanding the build system set some define is more trouble for users that aren’t using the build system.

Related, USE_FIELD_INV_BUILTIN/USE_SCALAR_INV_BUILTIN should get automatically defaulted. (maybe these go away with safegcd?) right now a non-buildsystem user needs to set them.

Perhaps for the rest too… I think now that there aren’t defines needed for the fe/scalar type it should be possible to build with zero defines.

sipa commented at 7:58 pm on August 15, 2020: contributor

One way to address that would be to simply have test.c and secp256k1.c set the macro before importing the headers.

I don’t see why that wouldn’t work.

Related, USE_FIELD_INV_BUILTIN/USE_SCALAR_INV_BUILTIN should get automatically defaulted. (maybe these go away with safegcd?) right now a non-buildsystem user needs to set them.

Yeah, I was expecting them to go away after safegcd (and we’d drop gmp/num support entirely; it can always be brought back from history if there is really a need).

Perhaps for the rest too… I think now that there aren’t defines needed for the fe/scalar type it should be possible to build with zero defines.

The remaining things are:

GMP or not (for now) (field inv/scalar inv could be decided automatically based on the presence of GMP, but GMP’s availability in the first place needs to be detected/configured through the build system or manual defines)
tweaks for ecmult / ecmult_gen (changed with #693) - there could be an automatic default if there is no override, though.
viability of using x86_64 asm can probably be made automatic (it’s restricted to x86_64, and to non-ancient gcc and cland anyway, which are pretty uniform)
ARM asm is harder/impossible, as it’s a separate file that needs to be externally assembled
Endomorphism or not (will also go away :D)
Static precomputation or not

gmaxwell commented at 10:25 pm on August 15, 2020: contributor

but GMP’s availability in the first place needs to be detected/configured through the build system or manual defines)

Yes but if you don’t use a build system, default to it off?

ASM could default on x86_64 … and on ARM it could emit a warning if it’s not used?

Static precompute could check for a define set by the header? … though maybe consider shipping the static table to avoid having to generate it?

real-or-random commented at 0:26 am on August 25, 2020: contributor

I don’t quite understand what the purpose of SECP256K1_BUILD is, can someone explain?

sipa commented at 0:37 am on August 25, 2020: contributor

@real-or-random It’s used to indicate that we’re building the library itself, and not an interface.

It matters because secp256k1.h is included both when building the library itself, and by applications linking with it. For the former, we don’t want the “nonnull” attributes, as they would make the compiler optimize away our explicit nonnull checks. We only want them as part of the external interface.

It’s also not set when compiling gen_context or the benchmarks. It’s less to clear to whether that matters.

real-or-random commented at 10:29 am on July 6, 2022: contributor

SECP256K1_BUILD : duplicate of #927 solved by #928
making manual builds easier: tracked in #929

real-or-random closed this on Jul 6, 2022

SECP256K1_BUILD sanity checking and USE_FIELD_INV_BUILTIN/USE_SCALAR_INV_BUILTIN simplification. #800