enable building dll's using makefile.msvc

[madebr]

• This pr enables building a dll with makefile.msvc
• the dll + static library are built
• the tests are not built for the shared libraries, because the tests use private symbols (that are not exported)
  See Build shared libraries using Visual Studio #488 (comment) for the link error.

@sjaeckel @nijtmans

[sjaeckel]

Did you read the comment in s_mp_rand_platform.c before the declaration of s_mp_rand_platform()? :)

[madebr]

s_mp_rand_platform

The reason I added these empty declarations is that when building a debug dll, the linker complains.
Running:

nmake -f makefile.msvc tommath.dll CFLAGS=-Zi LDFLAGS=-debug

causes this error to be thrown when creating the dll:

  Creating library tommath.dll.lib and object tommath.dll.exp
s_mp_rand_platform.obj : error LNK2019: unresolved external symbol s_read_arc4random referenced in function s_mp_rand_platform
s_mp_rand_platform.obj : error LNK2019: unresolved external symbol s_read_getrandom referenced in function s_mp_rand_platform
s_mp_rand_platform.obj : error LNK2019: unresolved external symbol s_read_urandom referenced in function s_mp_rand_platform

Adding these empty implementations, fixes that.

[madebr]

travis ci is failing because I have added some symbols to tommath.def, which are not automaticallly added by update.pl.
To fix this, I will have to create 5 files:

• mp_rand_source.c
• MP_MUL_KARATSUBA_CUTOFF.c
• MP_SQR_KARATSUBA_CUTOFF.c
• MP_MUL_TOOM_CUTOFF.c
• MP_SQR_TOOM_CUTOFF.c

What do you think?

[madebr]

Not adding implementations and depending on the compiler to optimize away impossible function calls, seems wrong to me.
I added a build script for libtommath at conan-center-index.
See conan-io/conan-center-index#2904 (comment)
It also fails for macos.

[sjaeckel]

Not adding implementations and depending on the compiler to optimize away impossible function calls, seems wrong to me.

I was also skeptical whether relying on this functionality is good, but nowadays I write code in the same style in my professional life. It has more advantages than disadvantages.

I added a build script for libtommath at conan-center-index.

cool, whatever this 'conan' is :)

It also fails for macos.

That's possible, as macos is not something we're testing regularly against.

How can I see a summary of what failed?

[madebr]

Not adding implementations and depending on the compiler to optimize away impossible function calls, seems wrong to me.

I was also skeptical whether relying on this functionality is good, but nowadays I write code in the same style in my professional life. It has more advantages than disadvantages.

It's easier to use and code, I understand.
But not so robust in the face of the variety of compilers.
For e.g. MSVC it requires at least -O1, -O0 would fail.

I added a build script for libtommath at conan-center-index.

cool, whatever this 'conan' is :)

conan is a package manager for c/c++, and conan-center-index is a library for useful comon libraries.
Think of it as a pypi for c/c++.

It also fails for macos.

That's possible, as macos is not something we're testing regularly against.

How can I see a summary of what failed?

I've fixed the la error.
See conan-io/conan-center-index#2904 (comment) for the latest CI output.
The CI is a wip, so no nice table (yet).
(Don't forget these have the patch applied to bn_s_mp_rand_platform.c)

[sjaeckel]

But not so robust in the face of the variety of compilers.
For e.g. MSVC it requires at least -O1, -O0 would fail.

TBH who really cares about MSVC? I don't ... support for it is a nice goodie, but to mess up code for this compiler - no way!

conan is a package manager for c/c++

nice!

(Don't forget these have the patch applied to bn_s_mp_rand_platform.c)

and sorry if our decision that MSVC support is only secondary makes additional work you :)

I guess if you have to use MSVC and this poses problems to you, you will have to maintain those patches.

IIRC @nijtmans builds Windows libraries with GCC, which then also have full 64bit support which MSVC is lacking!

[madebr]

TBH who really cares about MSVC? I don't ... support for it is a nice goodie, but to mess up code for this compiler - no way!

I understand your opinion, but as you see it's failing on clang too.
And it's always nice to have more test coverage.

I guess if you have to use MSVC and this poses problems to you, you will have to maintain those patches.

That's always a risk when packaging things 😄 .
(Please don't change too much in your next release 🤣 )

IIRC @nijtmans builds Windows libraries with GCC, which then also have full 64bit support which MSVC is lacking!

What missing 64-bit support are you referring to?

[sjaeckel]

I understand your opinion, but as you see it's failing on clang too.

hmm... I remember having seen this on clang as well, but I can't reproduce on my machine.

$ clang --version
clang version 10.0.1 
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin
$ CC=clang make -f makefile.shared -j9 IGNORE_SPEED=1                                                                                                                   makefile.shared:56: warning: ignoring prerequisites on suffix rule definition                                                                                                                                                                        
libtool --mode=compile --tag=CC clang -I./ -Wall -Wsign-compare -Wextra -Wshadow -Wdeclaration-after-statement -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wpointer-arith -Wsystem-headers -Wno-typedef-redefinition -Wno-tautological-compare -Wno-builtin-requires-header   -o mp_2expt.o -c mp_2expt.c     
...
libtool: link: ranlib .libs/libtommath.a
libtool: link: ( cd ".libs" && rm -f "libtommath.la" && ln -s "../libtommath.la" "libtommath.la" )

What missing 64-bit support are you referring to?

The fact that MSVC has no 128bit data type for mp_word ... which leads to libtommath with MSVC getting compiled as 32bit for 64bit Windows. If you want the full (64/128bit) experience you have to use a proper compiler ;)

[madebr]

hmm... I remember having seen this on clang as well, but I can't reproduce on my machine.

It also doesn't occur on my machine, running clang 9.
I'm not running ubuntu, but 20.04 has clang-6.0.

The fact that MSVC has no 128bit data type for mp_word ... which leads to libtommath with MSVC getting compiled as 32bit for 64bit Windows. If you want the full (64/128bit) experience you have to use a proper compiler ;)

I don't mind some friendly competition 😄

[MasterDuke17]

FWIW, MoarVM carries MoarVM@628cf84 around on top of libtommath because we use some debug builds in our CI testing.

[sjaeckel]

It also doesn't occur on my machine, running clang 9.
I'm not running ubuntu, but 20.04 has clang-6.0.

so it's clear that it's a compiler bug that has since been fixed in clang and the only compiler that is still buggy is MSVC.

[madebr]

@MasterDuke17

FWIW, MoarVM carries MoarVM@628cf84 around on top of libtommath because we use some debug builds in our CI testing.

Your patch is a lot easier. Can I borrow it for the cci patch? With attribution of course.

so it's clear that it's a compiler bug that has since been fixed in clang and the only compiler that is still buggy is MSVC.

Some things never change.

[MasterDuke17]

Your patch is a lot easier. Can I borrow it for the cci patch? With attribution of course.

Sure. Don't worry about attribution, and now that you mention it, I think I cribbed it from an earlier version of libtommath itself.

[sjaeckel]

Can this be closed in favor of #490 ?

[madebr]

Of course, your pr is building on top of this one.